Re: drivers/net/can/usb/etas_es58x/es58x_devlink.c:201:55: warning: '%02u' directive output may be truncated writing between 2 and 3 bytes into a region of size between 1 and 3

[Vincent Mailhol <mailhol.vincent@wanadoo.fr>]

On 16/01/2025 at 03:49, kernel test robot wrote:
> Hi Vincent,
> 
> FYI, the error/warning still remains.
> 
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head:   619f0b6fad524f08d493a98d55bac9ab8895e3a6
> commit: 9f06631c3f1f0f298536443df85a6837ba4c5f5c can: etas_es58x: export product information through devlink_ops::info_get()
> date:   2 years, 1 month ago
> config: powerpc-buildonly-randconfig-r001-20230115 (https://download.01.org/0day-ci/archive/20250116/202501160236.YSb9NcEp-lkp@intel.com/config)
> compiler: powerpc-linux-gcc (GCC) 12.4.0
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20250116/202501160236.YSb9NcEp-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@intel.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202501160236.YSb9NcEp-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
>    drivers/net/can/usb/etas_es58x/es58x_devlink.c: In function 'es58x_devlink_info_get':
>>> drivers/net/can/usb/etas_es58x/es58x_devlink.c:201:55: warning: '%02u' directive output may be truncated writing between 2 and 3 bytes into a region of size between 1 and 3 [-Wformat-truncation=]
>      201 |                 snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
>          |                                                       ^~~~
>    drivers/net/can/usb/etas_es58x/es58x_devlink.c:201:44: note: directive argument in the range [0, 255]
>      201 |                 snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
>          |                                            ^~~~~~~~~~~~~~~~
>    drivers/net/can/usb/etas_es58x/es58x_devlink.c:201:17: note: 'snprintf' output between 9 and 12 bytes into a destination of size 9
>      201 |                 snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
>          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      202 |                          fw_ver->major, fw_ver->minor, fw_ver->revision);
>          |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/net/can/usb/etas_es58x/es58x_devlink.c:211:55: warning: '%02u' directive output may be truncated writing between 2 and 3 bytes into a region of size between 1 and 3 [-Wformat-truncation=]
>      211 |                 snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
>          |                                                       ^~~~
>    drivers/net/can/usb/etas_es58x/es58x_devlink.c:211:44: note: directive argument in the range [0, 255]
>      211 |                 snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
>          |                                            ^~~~~~~~~~~~~~~~
>    drivers/net/can/usb/etas_es58x/es58x_devlink.c:211:17: note: 'snprintf' output between 9 and 12 bytes into a destination of size 9
>      211 |                 snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
>          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      212 |                          bl_ver->major, bl_ver->minor, bl_ver->revision);
>          |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    drivers/net/can/usb/etas_es58x/es58x_devlink.c:221:52: warning: '%03u' directive output may be truncated writing between 3 and 5 bytes into a region of size between 2 and 4 [-Wformat-truncation=]
>      221 |                 snprintf(buf, sizeof(buf), "%c%03u/%03u",
>          |                                                    ^~~~
>    drivers/net/can/usb/etas_es58x/es58x_devlink.c:221:44: note: directive argument in the range [0, 65535]
>      221 |                 snprintf(buf, sizeof(buf), "%c%03u/%03u",
>          |                                            ^~~~~~~~~~~~~
>    drivers/net/can/usb/etas_es58x/es58x_devlink.c:221:17: note: 'snprintf' output between 9 and 13 bytes into a destination of size 9
>      221 |                 snprintf(buf, sizeof(buf), "%c%03u/%03u",
>          |                 ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>      222 |                          hw_rev->letter, hw_rev->major, hw_rev->minor);
>          |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

I do not have plan to fix this warning.

The range is already checked in es58x_sw_version_is_valid() and in
es58x_hw_revision_is_valid() and recent versions of GCC are able to see
that no actual truncation can occur.

I just tested with powerpc-unknown-linux-gnu-gcc version 14.2.1 and the
warning indeed does not show up:

  $ powerpc-unknown-linux-gnu-gcc --version
  powerpc-unknown-linux-gnu-gcc (Gentoo 14.2.1_p20241221 p7) 14.2.1 20241221
  (...)

  $ make -j8 W=1 ARCH=powerpc CROSS_COMPILE=powerpc-unknown-linux-gnu-
drivers/net/can/usb/etas_es58x/es58x_devlink.o
    CALL    scripts/checksyscalls.sh
    CC      drivers/net/can/usb/etas_es58x/es58x_devlink.o

The warning is specific to some old GCC versions (v12.4.0 in this
report) on which the static analyzer is not able to take into account
that the sanitization done in the es58x_sw_version_is_valid() and
es58x_hw_revision_is_valid() inlined functions.

> vim +201 drivers/net/can/usb/etas_es58x/es58x_devlink.c
> 
>    177	
>    178	/**
>    179	 * es58x_devlink_info_get() - Report the product information.
>    180	 * @devlink: Devlink.
>    181	 * @req: skb wrapper where to put requested information.
>    182	 * @extack: Unused.
>    183	 *
>    184	 * Report the firmware version, the bootloader version, the hardware
>    185	 * revision and the serial number through netlink.
>    186	 *
>    187	 * Return: zero on success, errno when any error occurs.
>    188	 */
>    189	static int es58x_devlink_info_get(struct devlink *devlink,
>    190					  struct devlink_info_req *req,
>    191					  struct netlink_ext_ack *extack)
>    192	{
>    193		struct es58x_device *es58x_dev = devlink_priv(devlink);
>    194		struct es58x_sw_version *fw_ver = &es58x_dev->firmware_version;
>    195		struct es58x_sw_version *bl_ver = &es58x_dev->bootloader_version;
>    196		struct es58x_hw_revision *hw_rev = &es58x_dev->hardware_revision;
>    197		char buf[max(sizeof("xx.xx.xx"), sizeof("axxx/xxx"))];
>    198		int ret = 0;
>    199	
>    200		if (es58x_sw_version_is_set(fw_ver)) {
>  > 201			snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
>    202				 fw_ver->major, fw_ver->minor, fw_ver->revision);
>    203			ret = devlink_info_version_running_put(req,
>    204							       DEVLINK_INFO_VERSION_GENERIC_FW,
>    205							       buf);
>    206			if (ret)
>    207				return ret;
>    208		}
>    209	
>    210		if (es58x_sw_version_is_set(bl_ver)) {
>    211			snprintf(buf, sizeof(buf), "%02u.%02u.%02u",
>    212				 bl_ver->major, bl_ver->minor, bl_ver->revision);
>    213			ret = devlink_info_version_running_put(req,
>    214							       DEVLINK_INFO_VERSION_GENERIC_FW_BOOTLOADER,
>    215							       buf);
>    216			if (ret)
>    217				return ret;
>    218		}
>    219	
>    220		if (es58x_hw_revision_is_set(hw_rev)) {
>    221			snprintf(buf, sizeof(buf), "%c%03u/%03u",
>    222				 hw_rev->letter, hw_rev->major, hw_rev->minor);
>    223			ret = devlink_info_version_fixed_put(req,
>    224							     DEVLINK_INFO_VERSION_GENERIC_BOARD_REV,
>    225							     buf);
>    226			if (ret)
>    227				return ret;
>    228		}
>    229	
>    230		return devlink_info_serial_number_put(req, es58x_dev->udev->serial);
>    231	}
>    232	
> 

Yours sincerely,
Vincent Mailhol