| Message ID | 20191015131437.525-1-gy741.kim@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | usbip: tools: Fix read_usb_vudc_device() error path handling | expand |
On 10/15/19 7:14 AM, GwanYeong Kim wrote: > cannot be less than 0 - fread() returns 0 on error. > > Signed-off-by: GwanYeong Kim <gy741.kim@gmail.com> > --- > tools/usb/usbip/libsrc/usbip_device_driver.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c > index 051d7d3f443b..49760b98aabc 100644 > --- a/tools/usb/usbip/libsrc/usbip_device_driver.c > +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c > @@ -79,7 +79,7 @@ int read_usb_vudc_device(struct udev_device *sdev, struct usbip_usb_device *dev) > if (!fd) > return -1; > ret = fread((char *) &descr, sizeof(descr), 1, fd); > - if (ret < 0) > + if (ret != sizeof(descr)) Are you sure this check is correct? fread() returns the number of elements read, # elements = 1 in this case. fread() returns 0 when size or # of elements is 0 and in other error cases it will return < # of elements. I would think you want to check ret != 1 here. thanks, -- Shuah
On Tue, 15 Oct 2019 17:14:32 -0600 shuah <shuah@kernel.org> wrote: > On 10/15/19 7:14 AM, GwanYeong Kim wrote: > > cannot be less than 0 - fread() returns 0 on error. > > > > Signed-off-by: GwanYeong Kim <gy741.kim@gmail.com> > > --- > > tools/usb/usbip/libsrc/usbip_device_driver.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c > > b/tools/usb/usbip/libsrc/usbip_device_driver.c index > > 051d7d3f443b..49760b98aabc 100644 --- > > a/tools/usb/usbip/libsrc/usbip_device_driver.c +++ > > b/tools/usb/usbip/libsrc/usbip_device_driver.c @@ -79,7 +79,7 @@ > > int read_usb_vudc_device(struct udev_device *sdev, struct > > usbip_usb_device *dev) if (!fd) return -1; > > ret = fread((char *) &descr, sizeof(descr), 1, fd); > > - if (ret < 0) > + if (ret != sizeof(descr)) > > Are you sure this check is correct? fread() returns the number > of elements read, # elements = 1 in this case. Thank you for pointing this out. Sorry for my mistake. > fread() returns 0 when size or # of elements is 0 and in other > error cases it will return < # of elements. I would think you > want to check ret != 1 here. You're right. This should be changed to "ret != 1". Should I send a new patch? Regards, GwanYeong Kim
On Wed, Oct 16, 2019 at 01:38:25PM +0900, GwanYeong Kim wrote: > On Tue, 15 Oct 2019 17:14:32 -0600 > shuah <shuah@kernel.org> wrote: > > > On 10/15/19 7:14 AM, GwanYeong Kim wrote: > > > cannot be less than 0 - fread() returns 0 on error. > > > > > > Signed-off-by: GwanYeong Kim <gy741.kim@gmail.com> > > > --- > > > tools/usb/usbip/libsrc/usbip_device_driver.c | 2 +- > > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > > > diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c > > > b/tools/usb/usbip/libsrc/usbip_device_driver.c index > > > 051d7d3f443b..49760b98aabc 100644 --- > > > a/tools/usb/usbip/libsrc/usbip_device_driver.c +++ > > > b/tools/usb/usbip/libsrc/usbip_device_driver.c @@ -79,7 +79,7 @@ > > > int read_usb_vudc_device(struct udev_device *sdev, struct > > > usbip_usb_device *dev) if (!fd) return -1; > > > ret = fread((char *) &descr, sizeof(descr), 1, fd); > > > - if (ret < 0) > + if (ret != sizeof(descr)) > > > > Are you sure this check is correct? fread() returns the number > > of elements read, # elements = 1 in this case. > > Thank you for pointing this out. > Sorry for my mistake. > > > fread() returns 0 when size or # of elements is 0 and in other > > error cases it will return < # of elements. I would think you > > want to check ret != 1 here. > > You're right. > This should be changed to "ret != 1". > > Should I send a new patch? If you want to have it applied, yes. thanks, greg k-h
diff --git a/tools/usb/usbip/libsrc/usbip_device_driver.c b/tools/usb/usbip/libsrc/usbip_device_driver.c index 051d7d3f443b..49760b98aabc 100644 --- a/tools/usb/usbip/libsrc/usbip_device_driver.c +++ b/tools/usb/usbip/libsrc/usbip_device_driver.c @@ -79,7 +79,7 @@ int read_usb_vudc_device(struct udev_device *sdev, struct usbip_usb_device *dev) if (!fd) return -1; ret = fread((char *) &descr, sizeof(descr), 1, fd); - if (ret < 0) + if (ret != sizeof(descr)) goto err; fclose(fd);
cannot be less than 0 - fread() returns 0 on error. Signed-off-by: GwanYeong Kim <gy741.kim@gmail.com> --- tools/usb/usbip/libsrc/usbip_device_driver.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)