Re: [PATCH v2] usbip: tools: fix GCC8 warning for strncpy

["Liu, Changcheng" <changcheng.liu@intel.com>]

On 08:19 Thu 25 Jul, shuah wrote:
> On 7/25/19 7:22 AM, Liu, Changcheng wrote:
> > GCC8 started emitting warning about using strncpy with number of bytes
> > exactly equal destination size which could lead to non-zero terminated
> > string being copied. Use "SYSFS_PATH_MAX - 1" & "SYSFS_BUS_ID_SIZE - 1"
> > as number of bytes to ensure name is always zero-terminated.
> > 
> > Signed-off-by: Changcheng Liu <changcheng.liu@aliyun.com>
> > ---
> > v1 -> v2:
> >   * Correct array tail index
> > ---
> >   tools/usb/usbip/libsrc/usbip_common.c        | 6 ++++--
> >   tools/usb/usbip/libsrc/usbip_device_driver.c | 6 ++++--
> >   2 files changed, 8 insertions(+), 4 deletions(-)
> > 
> > diff --git a/tools/usb/usbip/libsrc/usbip_common.c b/tools/usb/usbip/libsrc/usbip_common.c
> > index bb424638d75b..b8d7d480595a 100644
> > --- a/tools/usb/usbip/libsrc/usbip_common.c
> > +++ b/tools/usb/usbip/libsrc/usbip_common.c
> > @@ -226,8 +226,10 @@ int read_usb_device(struct udev_device *sdev, struct usbip_usb_device *udev)
> >   	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);
> > +	strncpy(udev->path,  path,  SYSFS_PATH_MAX - 1);
> > +	udev->path[SYSFS_PATH_MAX - 1] = '\0';
> > +	strncpy(udev->busid, name, SYSFS_BUS_ID_SIZE - 1);
> > +	udev->busid[SYSFS_BUS_ID_SIZE - 1] = '\0';
> 
> strlcpy() would be better choice here. Any reason to not use that?
> 
@Shuah: linux tools link with libc which doesn't implment strlcpy yet.
So tools source code can't use strlcpy function like other kernel source
code.

> >   	sscanf(name, "%u-%u", &busnum, &devnum);
> >   	udev->busnum = busnum;
> > diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c
> > index 5a3726eb44ab..051d7d3f443b 100644
> > --- a/tools/usb/usbip/libsrc/usbip_device_driver.c
> > +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c
> > @@ -91,7 +91,8 @@ int read_usb_vudc_device(struct udev_device *sdev, struct usbip_usb_device *dev)
> >   	copy_descr_attr16(dev, &descr, idProduct);
> >   	copy_descr_attr16(dev, &descr, bcdDevice);
> > -	strncpy(dev->path, path, SYSFS_PATH_MAX);
> > +	strncpy(dev->path, path, SYSFS_PATH_MAX - 1);
> > +	dev->path[SYSFS_PATH_MAX - 1] = '\0';
> >   	dev->speed = USB_SPEED_UNKNOWN;
> >   	speed = udev_device_get_sysattr_value(sdev, "current_speed");
> > @@ -110,7 +111,8 @@ int read_usb_vudc_device(struct udev_device *sdev, struct usbip_usb_device *dev)
> >   	dev->busnum = 0;
> >   	name = udev_device_get_sysname(plat);
> > -	strncpy(dev->busid, name, SYSFS_BUS_ID_SIZE);
> > +	strncpy(dev->busid, name, SYSFS_BUS_ID_SIZE - 1);
> > +	dev->busid[SYSFS_BUS_ID_SIZE - 1] = '\0';
> 
> strlcpy() would be better choice here. Any reason to not use that?
> 
> >   	return 0;
> >   err:
> >   	fclose(fd);
> > 
> 
> thanks,
> -- Shuah