| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <Z4ijE/T0cTfyBhM9@rli9-mobl>
Date: Thu, 16 Jan 2025 14:11:31 +0800
From: Philip Li <philip.li@...el.com>
To: Vincent Mailhol <mailhol.vincent@...adoo.fr>
CC: kernel test robot <lkp@...el.com>, <oe-kbuild-all@...ts.linux.dev>,
<linux-kernel@...r.kernel.org>, Marc Kleine-Budde <mkl@...gutronix.de>,
Andrew Lunn <andrew@...n.ch>
Subject: 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
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@...el.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
>
>
Powered by blists - more mailing lists