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

From: Vincent Mailhol
Date: Wed Jan 15 2025 - 21:40:33 EST

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@xxxxxxxxx/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@xxxxxxxxx/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@xxxxxxxxx>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202501160236.YSb9NcEp-lkp@xxxxxxxxx/
>
> 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