Re: [PATCH] usbip: Fix misuse of strncpy()
On 07/26/2018 04:39 AM, Ben Hutchings wrote: > On Tue, 2018-07-24 at 11:04 -0600, Shuah Khan wrote: >> On 07/20/2018 08:12 PM, Ben Hutchings wrote: >>> gcc 8 reports: >>> >>> usbip_device_driver.c: In function ‘read_usb_vudc_device’: >>> usbip_device_driver.c:106:2: error: ‘strncpy’ specified bound 256 equals >>> destination size [-Werror=stringop-truncation] >>> strncpy(dev->path, path, SYSFS_PATH_MAX); >>> ^~~~ >>> usbip_device_driver.c:125:2: error: ‘strncpy’ specified bound 32 equals >>> destination size [-Werror=stringop-truncation] >>> strncpy(dev->busid, name, SYSFS_BUS_ID_SIZE); >>> ^~~~ >>> >>> I'm not convinced it makes sense to truncate the copied strings here, >>> but since we're already doing so let's ensure they're still null- >>> terminated. We can't easily use strlcpy() here, so use snprintf(). >>> >>> usbip_common.c has the same problem. >>> >>> Signed-off-by: Ben Hutchings >>> Cc: sta...@vger.kernel.org >>> --- >>> tools/usb/usbip/libsrc/usbip_common.c|4 ++-- >>> tools/usb/usbip/libsrc/usbip_device_driver.c |4 ++-- >>> 2 files changed, 4 insertions(+), 4 deletions(-) >>> >>> --- a/tools/usb/usbip/libsrc/usbip_common.c >>> +++ b/tools/usb/usbip/libsrc/usbip_common.c >>> @@ -226,8 +226,8 @@ int read_usb_device(struct udev_device * >>> path = udev_device_get_syspath(sdev); >>> name = udev_device_get_sysname(sdev); >>> >>> - strncpy(udev->path, path, SYSFS_PATH_MAX); >>> - strncpy(udev->busid, name, SYSFS_BUS_ID_SIZE); >>> + snprintf(udev->path, SYSFS_PATH_MAX, "%s", path); >>> + snprintf(udev->busid, SYSFS_BUS_ID_SIZE, "%s", name); >> >> I am okay with the change to use snprintf(). Please add check for >> return to avoid GCC 7 -Wformat-overflow wanrs on snprintf instances >> that don't check return. > [...] > > I've tried running: > > ./autogen.sh > CC=gcc-7 CFLAGS=-Wformat-overflow ./configure > make > > but this didn't produce any warnings. How did you get the warning? > > Ben. > Sorry for the delay. It has been a busy few weeks with travel both vacation and business. Okay. Please check the e5dfa3f902b9a642ae8c6997d57d7c41e384a90b for details. I didn't see this problem myself, likely because I was using older gcc. thanks, -- Shuah thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] usbip: Fix misuse of strncpy()
On Tue, 2018-07-24 at 11:04 -0600, Shuah Khan wrote: > On 07/20/2018 08:12 PM, Ben Hutchings wrote: > > gcc 8 reports: > > > > usbip_device_driver.c: In function ‘read_usb_vudc_device’: > > usbip_device_driver.c:106:2: error: ‘strncpy’ specified bound 256 equals > > destination size [-Werror=stringop-truncation] > > strncpy(dev->path, path, SYSFS_PATH_MAX); > > ^~~~ > > usbip_device_driver.c:125:2: error: ‘strncpy’ specified bound 32 equals > > destination size [-Werror=stringop-truncation] > > strncpy(dev->busid, name, SYSFS_BUS_ID_SIZE); > > ^~~~ > > > > I'm not convinced it makes sense to truncate the copied strings here, > > but since we're already doing so let's ensure they're still null- > > terminated. We can't easily use strlcpy() here, so use snprintf(). > > > > usbip_common.c has the same problem. > > > > Signed-off-by: Ben Hutchings > > Cc: sta...@vger.kernel.org > > --- > > tools/usb/usbip/libsrc/usbip_common.c|4 ++-- > > tools/usb/usbip/libsrc/usbip_device_driver.c |4 ++-- > > 2 files changed, 4 insertions(+), 4 deletions(-) > > > > --- a/tools/usb/usbip/libsrc/usbip_common.c > > +++ b/tools/usb/usbip/libsrc/usbip_common.c > > @@ -226,8 +226,8 @@ int read_usb_device(struct udev_device * > > path = udev_device_get_syspath(sdev); > > name = udev_device_get_sysname(sdev); > > > > - strncpy(udev->path, path, SYSFS_PATH_MAX); > > - strncpy(udev->busid, name, SYSFS_BUS_ID_SIZE); > > + snprintf(udev->path, SYSFS_PATH_MAX, "%s", path); > > + snprintf(udev->busid, SYSFS_BUS_ID_SIZE, "%s", name); > > I am okay with the change to use snprintf(). Please add check for > return to avoid GCC 7 -Wformat-overflow wanrs on snprintf instances > that don't check return. [...] I've tried running: ./autogen.sh CC=gcc-7 CFLAGS=-Wformat-overflow ./configure make but this didn't produce any warnings. How did you get the warning? Ben. -- Ben Hutchings Tomorrow will be cancelled due to lack of interest. signature.asc Description: This is a digitally signed message part
Re: [PATCH] usbip: Fix misuse of strncpy()
On 07/20/2018 08:12 PM, Ben Hutchings wrote: > gcc 8 reports: > > usbip_device_driver.c: In function ‘read_usb_vudc_device’: > usbip_device_driver.c:106:2: error: ‘strncpy’ specified bound 256 equals > destination size [-Werror=stringop-truncation] > strncpy(dev->path, path, SYSFS_PATH_MAX); > ^~~~ > usbip_device_driver.c:125:2: error: ‘strncpy’ specified bound 32 equals > destination size [-Werror=stringop-truncation] > strncpy(dev->busid, name, SYSFS_BUS_ID_SIZE); > ^~~~ > > I'm not convinced it makes sense to truncate the copied strings here, > but since we're already doing so let's ensure they're still null- > terminated. We can't easily use strlcpy() here, so use snprintf(). > > usbip_common.c has the same problem. > > Signed-off-by: Ben Hutchings > Cc: sta...@vger.kernel.org > --- > tools/usb/usbip/libsrc/usbip_common.c|4 ++-- > tools/usb/usbip/libsrc/usbip_device_driver.c |4 ++-- > 2 files changed, 4 insertions(+), 4 deletions(-) > > --- a/tools/usb/usbip/libsrc/usbip_common.c > +++ b/tools/usb/usbip/libsrc/usbip_common.c > @@ -226,8 +226,8 @@ int read_usb_device(struct udev_device * > path = udev_device_get_syspath(sdev); > name = udev_device_get_sysname(sdev); > > - strncpy(udev->path, path, SYSFS_PATH_MAX); > - strncpy(udev->busid, name, SYSFS_BUS_ID_SIZE); > + snprintf(udev->path, SYSFS_PATH_MAX, "%s", path); > + snprintf(udev->busid, SYSFS_BUS_ID_SIZE, "%s", name); I am okay with the change to use snprintf(). Please add check for return to avoid GCC 7 -Wformat-overflow wanrs on snprintf instances that don't check return. > > sscanf(name, "%u-%u", , ); > udev->busnum = busnum; > --- a/tools/usb/usbip/libsrc/usbip_device_driver.c > +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c > @@ -103,7 +103,7 @@ int read_usb_vudc_device(struct udev_dev > copy_descr_attr16(dev, , idProduct); > copy_descr_attr16(dev, , bcdDevice); > > - strncpy(dev->path, path, SYSFS_PATH_MAX); > + snprintf(dev->path, SYSFS_PATH_MAX, "%s", path); > > dev->speed = USB_SPEED_UNKNOWN; > speed = udev_device_get_sysattr_value(sdev, "current_speed"); > @@ -122,7 +122,7 @@ int read_usb_vudc_device(struct udev_dev > dev->busnum = 0; > > name = udev_device_get_sysname(plat); > - strncpy(dev->busid, name, SYSFS_BUS_ID_SIZE); > + snprintf(dev->busid, SYSFS_BUS_ID_SIZE, "%s", name); I am okay with the change to use snprintf(). Please add check for return to avoid GCC 7 -Wformat-overflow wanrs on snprintf instances that don't check return. thanks, -- Shuah -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] usbip: Fix misuse of strncpy()
gcc 8 reports: usbip_device_driver.c: In function ‘read_usb_vudc_device’: usbip_device_driver.c:106:2: error: ‘strncpy’ specified bound 256 equals destination size [-Werror=stringop-truncation] strncpy(dev->path, path, SYSFS_PATH_MAX); ^~~~ usbip_device_driver.c:125:2: error: ‘strncpy’ specified bound 32 equals destination size [-Werror=stringop-truncation] strncpy(dev->busid, name, SYSFS_BUS_ID_SIZE); ^~~~ I'm not convinced it makes sense to truncate the copied strings here, but since we're already doing so let's ensure they're still null- terminated. We can't easily use strlcpy() here, so use snprintf(). usbip_common.c has the same problem. Signed-off-by: Ben Hutchings Cc: sta...@vger.kernel.org --- tools/usb/usbip/libsrc/usbip_common.c|4 ++-- tools/usb/usbip/libsrc/usbip_device_driver.c |4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) --- a/tools/usb/usbip/libsrc/usbip_common.c +++ b/tools/usb/usbip/libsrc/usbip_common.c @@ -226,8 +226,8 @@ int read_usb_device(struct udev_device * path = udev_device_get_syspath(sdev); name = udev_device_get_sysname(sdev); - strncpy(udev->path, path, SYSFS_PATH_MAX); - strncpy(udev->busid, name, SYSFS_BUS_ID_SIZE); + snprintf(udev->path, SYSFS_PATH_MAX, "%s", path); + snprintf(udev->busid, SYSFS_BUS_ID_SIZE, "%s", name); sscanf(name, "%u-%u", , ); udev->busnum = busnum; --- a/tools/usb/usbip/libsrc/usbip_device_driver.c +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c @@ -103,7 +103,7 @@ int read_usb_vudc_device(struct udev_dev copy_descr_attr16(dev, , idProduct); copy_descr_attr16(dev, , bcdDevice); - strncpy(dev->path, path, SYSFS_PATH_MAX); + snprintf(dev->path, SYSFS_PATH_MAX, "%s", path); dev->speed = USB_SPEED_UNKNOWN; speed = udev_device_get_sysattr_value(sdev, "current_speed"); @@ -122,7 +122,7 @@ int read_usb_vudc_device(struct udev_dev dev->busnum = 0; name = udev_device_get_sysname(plat); - strncpy(dev->busid, name, SYSFS_BUS_ID_SIZE); + snprintf(dev->busid, SYSFS_BUS_ID_SIZE, "%s", name); return 0; err: fclose(fd); signature.asc Description: Digital signature
