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

[Philip Li <philip.li@intel.com>]

On Thu, Jan 16, 2025 at 11:40:06AM +0900, Vincent Mailhol wrote:
> 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.

Thanks a lot for the detail info, we will avoid reporting again such issue. Sorry
for the false report.

> 
> 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
> 
>