* [PATCH 1/2] Fix format overflows @ 2017-02-20 20:51 ` Jonathan Dieter 2017-02-20 20:51 ` [PATCH 2/2] Fix implicit fallthrough warning Jonathan Dieter 2017-02-21 6:12 ` [PATCH 1/2] Fix format overflows Krzysztof Opasiak 0 siblings, 2 replies; 16+ messages in thread From: Jonathan Dieter @ 2017-02-20 20:51 UTC (permalink / raw) To: linux-kernel Cc: Jonathan Dieter, Valentina Manea, Shuah Khan, Peter Senna Tschudin, Krzysztof Opasiak, Greg Kroah-Hartman, open list:USB OVER IP DRIVER The usbip userspace tools call sprintf()/snprintf() and don't check for the return value which can lead the paths to overflow, truncating the final file in the path. More urgently, GCC 7 now warns that these aren't checked with -Wformat-overflow, and with -Werror enabled in configure.ac, that makes these tools unbuildable. This patch fixes these problems by replacing sprintf() with snprintf() in one place and adding checks for the return value of snprintf(). Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> --- tools/usb/usbip/libsrc/usbip_common.c | 8 +++++++- tools/usb/usbip/libsrc/usbip_host_common.c | 25 ++++++++++++++++++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/tools/usb/usbip/libsrc/usbip_common.c b/tools/usb/usbip/libsrc/usbip_common.c index ac73710..fc875ee 100644 --- a/tools/usb/usbip/libsrc/usbip_common.c +++ b/tools/usb/usbip/libsrc/usbip_common.c @@ -215,9 +215,15 @@ int read_usb_interface(struct usbip_usb_device *udev, int i, struct usbip_usb_interface *uinf) { char busid[SYSFS_BUS_ID_SIZE]; + int size; struct udev_device *sif; - sprintf(busid, "%s:%d.%d", udev->busid, udev->bConfigurationValue, i); + size = snprintf(busid, SYSFS_BUS_ID_SIZE, "%s:%d.%d", + udev->busid, udev->bConfigurationValue, i); + if (size >= SYSFS_BUS_ID_SIZE) { + err("busid length %i >= SYSFS_BUS_ID_SIZE", size); + return -1; + } sif = udev_device_new_from_subsystem_sysname(udev_context, "usb", busid); if (!sif) { diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c index 9d41522..690cd49 100644 --- a/tools/usb/usbip/libsrc/usbip_host_common.c +++ b/tools/usb/usbip/libsrc/usbip_host_common.c @@ -40,13 +40,19 @@ struct udev *udev_context; static int32_t read_attr_usbip_status(struct usbip_usb_device *udev) { char status_attr_path[SYSFS_PATH_MAX]; + int size; int fd; int length; char status; int value = 0; - snprintf(status_attr_path, SYSFS_PATH_MAX, "%s/usbip_status", - udev->path); + size = snprintf(status_attr_path, SYSFS_PATH_MAX, "%s/usbip_status", + udev->path); + if (size >= SYSFS_PATH_MAX) { + err("usbip_status path length %i >= SYSFS_PATH_MAX", size); + return -1; + } + fd = open(status_attr_path, O_RDONLY); if (fd < 0) { @@ -218,6 +224,7 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) { char attr_name[] = "usbip_sockfd"; char sockfd_attr_path[SYSFS_PATH_MAX]; + int size; char sockfd_buff[30]; int ret; @@ -237,10 +244,18 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) } /* only the first interface is true */ - snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", - edev->udev.path, attr_name); + size = snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", + edev->udev.path, attr_name); + if (size >= SYSFS_PATH_MAX) { + err("exported device path length %i >= SYSFS_PATH_MAX", size); + return -1; + } - snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); + size = snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); + if (size >= 30) { + err("socket length %i >= 30", size); + return -1; + } ret = write_sysfs_attribute(sockfd_attr_path, sockfd_buff, strlen(sockfd_buff)); -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH 2/2] Fix implicit fallthrough warning 2017-02-20 20:51 ` [PATCH 1/2] Fix format overflows Jonathan Dieter @ 2017-02-20 20:51 ` Jonathan Dieter 2017-02-21 6:12 ` [PATCH 1/2] Fix format overflows Krzysztof Opasiak 1 sibling, 0 replies; 16+ messages in thread From: Jonathan Dieter @ 2017-02-20 20:51 UTC (permalink / raw) To: linux-kernel Cc: Jonathan Dieter, Valentina Manea, Shuah Khan, Peter Senna Tschudin, open list:USB OVER IP DRIVER GCC 7 now warns when switch statements fall through implicitly, and with -Werror enabled in configure.ac, that makes these tools unbuildable. We fix this by notifying the compiler that this particular case statement is meant to fall through. Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> --- tools/usb/usbip/src/usbip.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/usb/usbip/src/usbip.c b/tools/usb/usbip/src/usbip.c index d7599d9..73d8eee 100644 --- a/tools/usb/usbip/src/usbip.c +++ b/tools/usb/usbip/src/usbip.c @@ -176,6 +176,8 @@ int main(int argc, char *argv[]) break; case '?': printf("usbip: invalid option\n"); + /* Terminate after printing error */ + /* FALLTHRU */ default: usbip_usage(); goto out; -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Fix format overflows 2017-02-20 20:51 ` [PATCH 1/2] Fix format overflows Jonathan Dieter 2017-02-20 20:51 ` [PATCH 2/2] Fix implicit fallthrough warning Jonathan Dieter @ 2017-02-21 6:12 ` Krzysztof Opasiak 2017-02-21 6:48 ` Jonathan Dieter 1 sibling, 1 reply; 16+ messages in thread From: Krzysztof Opasiak @ 2017-02-21 6:12 UTC (permalink / raw) To: Jonathan Dieter, linux-kernel Cc: Valentina Manea, Shuah Khan, Peter Senna Tschudin, Greg Kroah-Hartman, open list:USB OVER IP DRIVER Hi, W dniu 2017-02-20 o 21:51, Jonathan Dieter pisze: > The usbip userspace tools call sprintf()/snprintf() and don't check for > the return value which can lead the paths to overflow, truncating the > final file in the path. > > More urgently, GCC 7 now warns that these aren't checked with > -Wformat-overflow, and with -Werror enabled in configure.ac, that makes > these tools unbuildable. > > This patch fixes these problems by replacing sprintf() with snprintf() in > one place and adding checks for the return value of snprintf(). > > Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> > Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> > --- > tools/usb/usbip/libsrc/usbip_common.c | 8 +++++++- > tools/usb/usbip/libsrc/usbip_host_common.c | 25 ++++++++++++++++++++----- > 2 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/tools/usb/usbip/libsrc/usbip_common.c b/tools/usb/usbip/libsrc/usbip_common.c > index ac73710..fc875ee 100644 > --- a/tools/usb/usbip/libsrc/usbip_common.c > +++ b/tools/usb/usbip/libsrc/usbip_common.c > @@ -215,9 +215,15 @@ int read_usb_interface(struct usbip_usb_device *udev, int i, > struct usbip_usb_interface *uinf) > { > char busid[SYSFS_BUS_ID_SIZE]; > + int size; > struct udev_device *sif; > > - sprintf(busid, "%s:%d.%d", udev->busid, udev->bConfigurationValue, i); > + size = snprintf(busid, SYSFS_BUS_ID_SIZE, "%s:%d.%d", why not sizeof(busid)? > + udev->busid, udev->bConfigurationValue, i); > + if (size >= SYSFS_BUS_ID_SIZE) { As above. > + err("busid length %i >= SYSFS_BUS_ID_SIZE", size); > + return -1; > + } > > sif = udev_device_new_from_subsystem_sysname(udev_context, "usb", busid); > if (!sif) { > diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c > index 9d41522..690cd49 100644 > --- a/tools/usb/usbip/libsrc/usbip_host_common.c > +++ b/tools/usb/usbip/libsrc/usbip_host_common.c > @@ -40,13 +40,19 @@ struct udev *udev_context; > static int32_t read_attr_usbip_status(struct usbip_usb_device *udev) > { > char status_attr_path[SYSFS_PATH_MAX]; > + int size; > int fd; > int length; > char status; > int value = 0; > > - snprintf(status_attr_path, SYSFS_PATH_MAX, "%s/usbip_status", > - udev->path); > + size = snprintf(status_attr_path, SYSFS_PATH_MAX, "%s/usbip_status", The same here. > + udev->path); > + if (size >= SYSFS_PATH_MAX) { > + err("usbip_status path length %i >= SYSFS_PATH_MAX", size); > + return -1; > + } > + > > fd = open(status_attr_path, O_RDONLY); > if (fd < 0) { > @@ -218,6 +224,7 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) > { > char attr_name[] = "usbip_sockfd"; > char sockfd_attr_path[SYSFS_PATH_MAX]; > + int size; > char sockfd_buff[30]; > int ret; > > @@ -237,10 +244,18 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) > } > > /* only the first interface is true */ > - snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", > - edev->udev.path, attr_name); > + size = snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", > + edev->udev.path, attr_name); > + if (size >= SYSFS_PATH_MAX) { hmmm this should be sizeof(sockfd_attr_path) not SYSFS_PATH_MAX > + err("exported device path length %i >= SYSFS_PATH_MAX", size); > + return -1; > + } > > - snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); > + size = snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); > + if (size >= 30) { Please don't hardcode such values in if. use sizeof() like one line above > + err("socket length %i >= 30", size); > + return -1; > + } > > ret = write_sysfs_attribute(sockfd_attr_path, sockfd_buff, > strlen(sockfd_buff)); > Best regards, -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Fix format overflows 2017-02-21 6:12 ` [PATCH 1/2] Fix format overflows Krzysztof Opasiak @ 2017-02-21 6:48 ` Jonathan Dieter 2017-02-21 17:50 ` Jonathan Dieter 0 siblings, 1 reply; 16+ messages in thread From: Jonathan Dieter @ 2017-02-21 6:48 UTC (permalink / raw) To: Krzysztof Opasiak, linux-kernel Cc: Valentina Manea, Shuah Khan, Peter Senna Tschudin, Greg Kroah-Hartman, open list:USB OVER IP DRIVER Thanks for looking at this. One quick question before I put out version two with your corrections: On Tue, 2017-02-21 at 07:12 +0100, Krzysztof Opasiak wrote: > Hi, > > W dniu 2017-02-20 o 21:51, Jonathan Dieter pisze: > > The usbip userspace tools call sprintf()/snprintf() and don't check > > for > > the return value which can lead the paths to overflow, truncating > > the > > final file in the path. > > > > More urgently, GCC 7 now warns that these aren't checked with > > -Wformat-overflow, and with -Werror enabled in configure.ac, that > > makes > > these tools unbuildable. > > > > This patch fixes these problems by replacing sprintf() with > > snprintf() in > > one place and adding checks for the return value of snprintf(). > > > > Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> > > Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> > > --- > > tools/usb/usbip/libsrc/usbip_common.c | 8 +++++++- > > tools/usb/usbip/libsrc/usbip_host_common.c | 25 > > ++++++++++++++++++++----- > > 2 files changed, 27 insertions(+), 6 deletions(-) > > > > diff --git a/tools/usb/usbip/libsrc/usbip_common.c > > b/tools/usb/usbip/libsrc/usbip_common.c > > index ac73710..fc875ee 100644 > > --- a/tools/usb/usbip/libsrc/usbip_common.c > > +++ b/tools/usb/usbip/libsrc/usbip_common.c > > @@ -215,9 +215,15 @@ int read_usb_interface(struct usbip_usb_device > > *udev, int i, > > struct usbip_usb_interface *uinf) > > { > > char busid[SYSFS_BUS_ID_SIZE]; > > + int size; > > struct udev_device *sif; > > > > - sprintf(busid, "%s:%d.%d", udev->busid, udev- > > >bConfigurationValue, i); > > + size = snprintf(busid, SYSFS_BUS_ID_SIZE, "%s:%d.%d", > > why not sizeof(busid)? > > > + udev->busid, udev->bConfigurationValue, > > i); > > + if (size >= SYSFS_BUS_ID_SIZE) { > > As above. > > > + err("busid length %i >= SYSFS_BUS_ID_SIZE", size); Should I also change the error messages to use real values? Ex: err("busid length %i >= %i", size, sizeof(busid)); > > + return -1; > > + } > > > > sif = udev_device_new_from_subsystem_sysname(udev_context, > > "usb", busid); > > if (!sif) { > > diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c > > b/tools/usb/usbip/libsrc/usbip_host_common.c > > index 9d41522..690cd49 100644 > > --- a/tools/usb/usbip/libsrc/usbip_host_common.c > > +++ b/tools/usb/usbip/libsrc/usbip_host_common.c > > @@ -40,13 +40,19 @@ struct udev *udev_context; > > static int32_t read_attr_usbip_status(struct usbip_usb_device > > *udev) > > { > > char status_attr_path[SYSFS_PATH_MAX]; > > + int size; > > int fd; > > int length; > > char status; > > int value = 0; > > > > - snprintf(status_attr_path, SYSFS_PATH_MAX, > > "%s/usbip_status", > > - udev->path); > > + size = snprintf(status_attr_path, SYSFS_PATH_MAX, > > "%s/usbip_status", > > The same here. > > > + udev->path); > > + if (size >= SYSFS_PATH_MAX) { > > + err("usbip_status path length %i >= > > SYSFS_PATH_MAX", size); > > + return -1; > > + } > > + > > > > fd = open(status_attr_path, O_RDONLY); > > if (fd < 0) { > > @@ -218,6 +224,7 @@ int usbip_export_device(struct > > usbip_exported_device *edev, int sockfd) > > { > > char attr_name[] = "usbip_sockfd"; > > char sockfd_attr_path[SYSFS_PATH_MAX]; > > + int size; > > char sockfd_buff[30]; > > int ret; > > > > @@ -237,10 +244,18 @@ int usbip_export_device(struct > > usbip_exported_device *edev, int sockfd) > > } > > > > /* only the first interface is true */ > > - snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), > > "%s/%s", > > - edev->udev.path, attr_name); > > + size = snprintf(sockfd_attr_path, > > sizeof(sockfd_attr_path), "%s/%s", > > + edev->udev.path, attr_name); > > + if (size >= SYSFS_PATH_MAX) { > > hmmm this should be sizeof(sockfd_attr_path) not SYSFS_PATH_MAX > > > + err("exported device path length %i >= > > SYSFS_PATH_MAX", size); > > + return -1; > > + } > > > > - snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", > > sockfd); > > + size = snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", > > sockfd); > > + if (size >= 30) { > > Please don't hardcode such values in if. use sizeof() like one line > above > > > + err("socket length %i >= 30", size); > > + return -1; > > + } > > > > ret = write_sysfs_attribute(sockfd_attr_path, sockfd_buff, > > strlen(sockfd_buff)); > > > > Best regards, ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH 1/2] Fix format overflows 2017-02-21 6:48 ` Jonathan Dieter @ 2017-02-21 17:50 ` Jonathan Dieter 2017-02-21 17:57 ` [PATCH v2 1/2] usbip: Fix-format-overflow Jonathan Dieter 0 siblings, 1 reply; 16+ messages in thread From: Jonathan Dieter @ 2017-02-21 17:50 UTC (permalink / raw) To: Krzysztof Opasiak, linux-kernel Cc: Valentina Manea, Shuah Khan, Peter Senna Tschudin, Greg Kroah-Hartman, open list:USB OVER IP DRIVER On Tue, 2017-02-21 at 08:48 +0200, Jonathan Dieter wrote: > On Tue, 2017-02-21 at 07:12 +0100, Krzysztof Opasiak wrote: > > Hi, > > > > W dniu 2017-02-20 o 21:51, Jonathan Dieter pisze: > > > + err("busid length %i >= SYSFS_BUS_ID_SIZE", > > > size); > > Should I also change the error messages to use real values? > Ex: > err("busid length %i >= %i", size, sizeof(busid)); On further reflection, I do think changing the error messages to use sizeof() is cleaner. If you disagree, please let me know. I don't have a strong feeling about it one way or the other. The next revision uses sizeof() in the places you indicated and changes size to an unsigned int since we're now comparing it to the result of sizeof(). Thanks again, Jonathan ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v2 1/2] usbip: Fix-format-overflow 2017-02-21 17:50 ` Jonathan Dieter @ 2017-02-21 17:57 ` Jonathan Dieter 2017-02-21 17:57 ` [PATCH v2 2/2] usbip: Fix implicit fallthrough warning Jonathan Dieter 2017-02-22 5:49 ` [PATCH v2 1/2] usbip: Fix-format-overflow Krzysztof Opasiak 0 siblings, 2 replies; 16+ messages in thread From: Jonathan Dieter @ 2017-02-21 17:57 UTC (permalink / raw) To: linux-kernel Cc: Jonathan Dieter, Valentina Manea, Shuah Khan, Peter Senna Tschudin, Krzysztof Opasiak, Greg Kroah-Hartman, open list:USB OVER IP DRIVER The usbip userspace tools call sprintf()/snprintf() and don't check for the return value which can lead the paths to overflow, truncating the final file in the path. More urgently, GCC 7 now warns that these aren't checked with -Wformat-overflow, and with -Werror enabled in configure.ac, that makes these tools unbuildable. This patch fixes these problems by replacing sprintf() with snprintf() in one place and adding checks for the return value of snprintf(). Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> --- tools/usb/usbip/libsrc/usbip_common.c | 8 +++++++- tools/usb/usbip/libsrc/usbip_host_common.c | 25 ++++++++++++++++++++----- 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/tools/usb/usbip/libsrc/usbip_common.c b/tools/usb/usbip/libsrc/usbip_common.c index ac73710..01dd4b2 100644 --- a/tools/usb/usbip/libsrc/usbip_common.c +++ b/tools/usb/usbip/libsrc/usbip_common.c @@ -215,9 +215,15 @@ int read_usb_interface(struct usbip_usb_device *udev, int i, struct usbip_usb_interface *uinf) { char busid[SYSFS_BUS_ID_SIZE]; + unsigned int size; struct udev_device *sif; - sprintf(busid, "%s:%d.%d", udev->busid, udev->bConfigurationValue, i); + size = snprintf(busid, sizeof(busid), "%s:%d.%d", + udev->busid, udev->bConfigurationValue, i); + if (size >= sizeof(busid)) { + err("busid length %u >= %lu", size, sizeof(busid)); + return -1; + } sif = udev_device_new_from_subsystem_sysname(udev_context, "usb", busid); if (!sif) { diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c index 9d41522..398c1fa 100644 --- a/tools/usb/usbip/libsrc/usbip_host_common.c +++ b/tools/usb/usbip/libsrc/usbip_host_common.c @@ -40,13 +40,19 @@ struct udev *udev_context; static int32_t read_attr_usbip_status(struct usbip_usb_device *udev) { char status_attr_path[SYSFS_PATH_MAX]; + unsigned int size; int fd; int length; char status; int value = 0; - snprintf(status_attr_path, SYSFS_PATH_MAX, "%s/usbip_status", - udev->path); + size = snprintf(status_attr_path, sizeof(status_attr_path), "%s/usbip_status", + udev->path); + if (size >= sizeof(status_attr_path)) { + err("usbip_status path length %u >= %lu", size, sizeof(status_attr_path)); + return -1; + } + fd = open(status_attr_path, O_RDONLY); if (fd < 0) { @@ -218,6 +224,7 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) { char attr_name[] = "usbip_sockfd"; char sockfd_attr_path[SYSFS_PATH_MAX]; + unsigned int size; char sockfd_buff[30]; int ret; @@ -237,10 +244,18 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) } /* only the first interface is true */ - snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", - edev->udev.path, attr_name); + size = snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", + edev->udev.path, attr_name); + if (size >= sizeof(sockfd_attr_path)) { + err("exported device path length %u >= %lu", size, sizeof(sockfd_attr_path)); + return -1; + } - snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); + size = snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); + if (size >= sizeof(sockfd_buff)) { + err("socket length %u >= %lu", size, sizeof(sockfd_buff)); + return -1; + } ret = write_sysfs_attribute(sockfd_attr_path, sockfd_buff, strlen(sockfd_buff)); -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v2 2/2] usbip: Fix implicit fallthrough warning 2017-02-21 17:57 ` [PATCH v2 1/2] usbip: Fix-format-overflow Jonathan Dieter @ 2017-02-21 17:57 ` Jonathan Dieter 2017-02-22 5:49 ` [PATCH v2 1/2] usbip: Fix-format-overflow Krzysztof Opasiak 1 sibling, 0 replies; 16+ messages in thread From: Jonathan Dieter @ 2017-02-21 17:57 UTC (permalink / raw) To: linux-kernel Cc: Jonathan Dieter, Valentina Manea, Shuah Khan, Peter Senna Tschudin, open list:USB OVER IP DRIVER GCC 7 now warns when switch statements fall through implicitly, and with -Werror enabled in configure.ac, that makes these tools unbuildable. We fix this by notifying the compiler that this particular case statement is meant to fall through. Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> --- tools/usb/usbip/src/usbip.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/usb/usbip/src/usbip.c b/tools/usb/usbip/src/usbip.c index d7599d9..73d8eee 100644 --- a/tools/usb/usbip/src/usbip.c +++ b/tools/usb/usbip/src/usbip.c @@ -176,6 +176,8 @@ int main(int argc, char *argv[]) break; case '?': printf("usbip: invalid option\n"); + /* Terminate after printing error */ + /* FALLTHRU */ default: usbip_usage(); goto out; -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] usbip: Fix-format-overflow 2017-02-21 17:57 ` [PATCH v2 1/2] usbip: Fix-format-overflow Jonathan Dieter 2017-02-21 17:57 ` [PATCH v2 2/2] usbip: Fix implicit fallthrough warning Jonathan Dieter @ 2017-02-22 5:49 ` Krzysztof Opasiak 2017-02-22 18:14 ` Jonathan Dieter 2017-02-22 18:17 ` [PATCH v3 " Jonathan Dieter 1 sibling, 2 replies; 16+ messages in thread From: Krzysztof Opasiak @ 2017-02-22 5:49 UTC (permalink / raw) To: Jonathan Dieter, linux-kernel Cc: Valentina Manea, Shuah Khan, Peter Senna Tschudin, Greg Kroah-Hartman, open list:USB OVER IP DRIVER Hi, W dniu 2017-02-21 o 18:57, Jonathan Dieter pisze: > The usbip userspace tools call sprintf()/snprintf() and don't check for > the return value which can lead the paths to overflow, truncating the > final file in the path. > > More urgently, GCC 7 now warns that these aren't checked with > -Wformat-overflow, and with -Werror enabled in configure.ac, that makes > these tools unbuildable. > > This patch fixes these problems by replacing sprintf() with snprintf() in > one place and adding checks for the return value of snprintf(). > > Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> > Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> > --- > tools/usb/usbip/libsrc/usbip_common.c | 8 +++++++- > tools/usb/usbip/libsrc/usbip_host_common.c | 25 ++++++++++++++++++++----- > 2 files changed, 27 insertions(+), 6 deletions(-) > > diff --git a/tools/usb/usbip/libsrc/usbip_common.c b/tools/usb/usbip/libsrc/usbip_common.c > index ac73710..01dd4b2 100644 > --- a/tools/usb/usbip/libsrc/usbip_common.c > +++ b/tools/usb/usbip/libsrc/usbip_common.c > @@ -215,9 +215,15 @@ int read_usb_interface(struct usbip_usb_device *udev, int i, > struct usbip_usb_interface *uinf) > { > char busid[SYSFS_BUS_ID_SIZE]; > + unsigned int size; I'm not really convinced to use unsigned here. snprintf() is declared to return signed integer so we should assume that some of its implementation may return negative error code. Any rationale to this instead of just doing a cast for comparsion but signed value to print error? Best regards -- Krzysztof Opasiak Samsung R&D Institute Poland Samsung Electronics ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v2 1/2] usbip: Fix-format-overflow 2017-02-22 5:49 ` [PATCH v2 1/2] usbip: Fix-format-overflow Krzysztof Opasiak @ 2017-02-22 18:14 ` Jonathan Dieter 2017-02-22 18:17 ` [PATCH v3 " Jonathan Dieter 1 sibling, 0 replies; 16+ messages in thread From: Jonathan Dieter @ 2017-02-22 18:14 UTC (permalink / raw) To: Krzysztof Opasiak, linux-kernel Cc: Valentina Manea, Shuah Khan, Peter Senna Tschudin, Greg Kroah-Hartman, open list:USB OVER IP DRIVER On Wed, 2017-02-22 at 06:49 +0100, Krzysztof Opasiak wrote: > Hi, > > W dniu 2017-02-21 o 18:57, Jonathan Dieter pisze: <snip> > > char busid[SYSFS_BUS_ID_SIZE]; > > + unsigned int size; > > I'm not really convinced to use unsigned here. snprintf() is declared > to > return signed integer so we should assume that some of its > implementation may return negative error code. Any rationale to this > instead of just doing a cast for comparsion but signed value to print > error? No, you're absolutely right. I've fixed this and am sending the next revision now. Jonathan ^ permalink raw reply [flat|nested] 16+ messages in thread
* [PATCH v3 1/2] usbip: Fix-format-overflow 2017-02-22 5:49 ` [PATCH v2 1/2] usbip: Fix-format-overflow Krzysztof Opasiak 2017-02-22 18:14 ` Jonathan Dieter @ 2017-02-22 18:17 ` Jonathan Dieter 2017-02-22 18:18 ` [PATCH v3 2/2] usbip: Fix implicit fallthrough warning Jonathan Dieter 2017-02-27 8:31 ` [PATCH v4 1/2] usbip: Fix-format-overflow Jonathan Dieter 1 sibling, 2 replies; 16+ messages in thread From: Jonathan Dieter @ 2017-02-22 18:17 UTC (permalink / raw) To: linux-kernel Cc: Jonathan Dieter, Valentina Manea, Shuah Khan, Peter Senna Tschudin, Greg Kroah-Hartman, Krzysztof Opasiak, open list:USB OVER IP DRIVER The usbip userspace tools call sprintf()/snprintf() and don't check for the return value which can lead the paths to overflow, truncating the final file in the path. More urgently, GCC 7 now warns that these aren't checked with -Wformat-overflow, and with -Werror enabled in configure.ac, that makes these tools unbuildable. This patch fixes these problems by replacing sprintf() with snprintf() in one place and adding checks for the return value of snprintf(). Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> --- Changes since v2 * Converted size back to int because snprintf may return < 0 if there's an error * Expanded error messages to indicate failure may be because size < 0 tools/usb/usbip/libsrc/usbip_common.c | 8 +++++++- tools/usb/usbip/libsrc/usbip_host_common.c | 28 +++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 6 deletions(-) diff --git a/tools/usb/usbip/libsrc/usbip_common.c b/tools/usb/usbip/libsrc/usbip_common.c index ac73710..66017d7 100644 --- a/tools/usb/usbip/libsrc/usbip_common.c +++ b/tools/usb/usbip/libsrc/usbip_common.c @@ -215,9 +215,15 @@ int read_usb_interface(struct usbip_usb_device *udev, int i, struct usbip_usb_interface *uinf) { char busid[SYSFS_BUS_ID_SIZE]; + int size; struct udev_device *sif; - sprintf(busid, "%s:%d.%d", udev->busid, udev->bConfigurationValue, i); + size = snprintf(busid, sizeof(busid), "%s:%d.%d", + udev->busid, udev->bConfigurationValue, i); + if (size < 0 || (unsigned int)size >= sizeof(busid)) { + err("busid length %i >= %lu or < 0", size, sizeof(busid)); + return -1; + } sif = udev_device_new_from_subsystem_sysname(udev_context, "usb", busid); if (!sif) { diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c index 9d41522..6fb91d9 100644 --- a/tools/usb/usbip/libsrc/usbip_host_common.c +++ b/tools/usb/usbip/libsrc/usbip_host_common.c @@ -40,13 +40,20 @@ struct udev *udev_context; static int32_t read_attr_usbip_status(struct usbip_usb_device *udev) { char status_attr_path[SYSFS_PATH_MAX]; + int size; int fd; int length; char status; int value = 0; - snprintf(status_attr_path, SYSFS_PATH_MAX, "%s/usbip_status", - udev->path); + size = snprintf(status_attr_path, sizeof(status_attr_path), + "%s/usbip_status", udev->path); + if (size < 0 || (unsigned int)size >= sizeof(status_attr_path)) { + err("usbip_status path length %i >= %lu or < 0", size, + sizeof(status_attr_path)); + return -1; + } + fd = open(status_attr_path, O_RDONLY); if (fd < 0) { @@ -218,6 +225,7 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) { char attr_name[] = "usbip_sockfd"; char sockfd_attr_path[SYSFS_PATH_MAX]; + int size; char sockfd_buff[30]; int ret; @@ -237,10 +245,20 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) } /* only the first interface is true */ - snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", - edev->udev.path, attr_name); + size = snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", + edev->udev.path, attr_name); + if (size < 0 || (unsigned int)size >= sizeof(sockfd_attr_path)) { + err("exported device path length %i >= %lu or < 0", size, + sizeof(sockfd_attr_path)); + return -1; + } - snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); + size = snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); + if (size < 0 || (unsigned int)size >= sizeof(sockfd_buff)) { + err("socket length %i >= %lu or < 0", size, + sizeof(sockfd_buff)); + return -1; + } ret = write_sysfs_attribute(sockfd_attr_path, sockfd_buff, strlen(sockfd_buff)); -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v3 2/2] usbip: Fix implicit fallthrough warning 2017-02-22 18:17 ` [PATCH v3 " Jonathan Dieter @ 2017-02-22 18:18 ` Jonathan Dieter 2017-02-27 8:31 ` [PATCH v4 1/2] usbip: Fix-format-overflow Jonathan Dieter 1 sibling, 0 replies; 16+ messages in thread From: Jonathan Dieter @ 2017-02-22 18:18 UTC (permalink / raw) To: linux-kernel Cc: Jonathan Dieter, Valentina Manea, Shuah Khan, Peter Senna Tschudin, open list:USB OVER IP DRIVER GCC 7 now warns when switch statements fall through implicitly, and with -Werror enabled in configure.ac, that makes these tools unbuildable. We fix this by notifying the compiler that this particular case statement is meant to fall through. Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> --- tools/usb/usbip/src/usbip.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/usb/usbip/src/usbip.c b/tools/usb/usbip/src/usbip.c index d7599d9..73d8eee 100644 --- a/tools/usb/usbip/src/usbip.c +++ b/tools/usb/usbip/src/usbip.c @@ -176,6 +176,8 @@ int main(int argc, char *argv[]) break; case '?': printf("usbip: invalid option\n"); + /* Terminate after printing error */ + /* FALLTHRU */ default: usbip_usage(); goto out; -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 1/2] usbip: Fix-format-overflow 2017-02-22 18:17 ` [PATCH v3 " Jonathan Dieter 2017-02-22 18:18 ` [PATCH v3 2/2] usbip: Fix implicit fallthrough warning Jonathan Dieter @ 2017-02-27 8:31 ` Jonathan Dieter 2017-02-27 8:31 ` [PATCH v4 2/2] usbip: Fix implicit fallthrough warning Jonathan Dieter ` (2 more replies) 1 sibling, 3 replies; 16+ messages in thread From: Jonathan Dieter @ 2017-02-27 8:31 UTC (permalink / raw) To: linux-kernel Cc: Jonathan Dieter, Valentina Manea, Shuah Khan, Peter Senna Tschudin, Greg Kroah-Hartman, Krzysztof Opasiak, open list:USB OVER IP DRIVER The usbip userspace tools call sprintf()/snprintf() and don't check for the return value which can lead the paths to overflow, truncating the final file in the path. More urgently, GCC 7 now warns that these aren't checked with -Wformat-overflow, and with -Werror enabled in configure.ac, that makes these tools unbuildable. This patch fixes these problems by replacing sprintf() with snprintf() in one place and adding checks for the return value of snprintf(). Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> --- Changes since v3 * Cast sizeof to long unsigned when printing errors to fix building for 32-bit architectures tools/usb/usbip/libsrc/usbip_common.c | 9 ++++++++- tools/usb/usbip/libsrc/usbip_host_common.c | 28 +++++++++++++++++++++++----- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/tools/usb/usbip/libsrc/usbip_common.c b/tools/usb/usbip/libsrc/usbip_common.c index ac73710..1517a23 100644 --- a/tools/usb/usbip/libsrc/usbip_common.c +++ b/tools/usb/usbip/libsrc/usbip_common.c @@ -215,9 +215,16 @@ int read_usb_interface(struct usbip_usb_device *udev, int i, struct usbip_usb_interface *uinf) { char busid[SYSFS_BUS_ID_SIZE]; + int size; struct udev_device *sif; - sprintf(busid, "%s:%d.%d", udev->busid, udev->bConfigurationValue, i); + size = snprintf(busid, sizeof(busid), "%s:%d.%d", + udev->busid, udev->bConfigurationValue, i); + if (size < 0 || (unsigned int)size >= sizeof(busid)) { + err("busid length %i >= %lu or < 0", size, + (long unsigned)sizeof(busid)); + return -1; + } sif = udev_device_new_from_subsystem_sysname(udev_context, "usb", busid); if (!sif) { diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c index 9d41522..6ff7b60 100644 --- a/tools/usb/usbip/libsrc/usbip_host_common.c +++ b/tools/usb/usbip/libsrc/usbip_host_common.c @@ -40,13 +40,20 @@ struct udev *udev_context; static int32_t read_attr_usbip_status(struct usbip_usb_device *udev) { char status_attr_path[SYSFS_PATH_MAX]; + int size; int fd; int length; char status; int value = 0; - snprintf(status_attr_path, SYSFS_PATH_MAX, "%s/usbip_status", - udev->path); + size = snprintf(status_attr_path, sizeof(status_attr_path), + "%s/usbip_status", udev->path); + if (size < 0 || (unsigned int)size >= sizeof(status_attr_path)) { + err("usbip_status path length %i >= %lu or < 0", size, + (long unsigned)sizeof(status_attr_path)); + return -1; + } + fd = open(status_attr_path, O_RDONLY); if (fd < 0) { @@ -218,6 +225,7 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) { char attr_name[] = "usbip_sockfd"; char sockfd_attr_path[SYSFS_PATH_MAX]; + int size; char sockfd_buff[30]; int ret; @@ -237,10 +245,20 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) } /* only the first interface is true */ - snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", - edev->udev.path, attr_name); + size = snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", + edev->udev.path, attr_name); + if (size < 0 || (unsigned int)size >= sizeof(sockfd_attr_path)) { + err("exported device path length %i >= %lu or < 0", size, + (long unsigned)sizeof(sockfd_attr_path)); + return -1; + } - snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); + size = snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); + if (size < 0 || (unsigned int)size >= sizeof(sockfd_buff)) { + err("socket length %i >= %lu or < 0", size, + (long unsigned)sizeof(sockfd_buff)); + return -1; + } ret = write_sysfs_attribute(sockfd_attr_path, sockfd_buff, strlen(sockfd_buff)); -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* [PATCH v4 2/2] usbip: Fix implicit fallthrough warning 2017-02-27 8:31 ` [PATCH v4 1/2] usbip: Fix-format-overflow Jonathan Dieter @ 2017-02-27 8:31 ` Jonathan Dieter 2017-03-16 7:47 ` [PATCH v4 1/2] usbip: Fix-format-overflow Jonathan Dieter 2017-03-16 15:04 ` Shuah Khan 2 siblings, 0 replies; 16+ messages in thread From: Jonathan Dieter @ 2017-02-27 8:31 UTC (permalink / raw) To: linux-kernel Cc: Jonathan Dieter, Valentina Manea, Shuah Khan, Peter Senna Tschudin, open list:USB OVER IP DRIVER GCC 7 now warns when switch statements fall through implicitly, and with -Werror enabled in configure.ac, that makes these tools unbuildable. We fix this by notifying the compiler that this particular case statement is meant to fall through. Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> --- tools/usb/usbip/src/usbip.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tools/usb/usbip/src/usbip.c b/tools/usb/usbip/src/usbip.c index d7599d9..73d8eee 100644 --- a/tools/usb/usbip/src/usbip.c +++ b/tools/usb/usbip/src/usbip.c @@ -176,6 +176,8 @@ int main(int argc, char *argv[]) break; case '?': printf("usbip: invalid option\n"); + /* Terminate after printing error */ + /* FALLTHRU */ default: usbip_usage(); goto out; -- 2.9.3 ^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] usbip: Fix-format-overflow 2017-02-27 8:31 ` [PATCH v4 1/2] usbip: Fix-format-overflow Jonathan Dieter 2017-02-27 8:31 ` [PATCH v4 2/2] usbip: Fix implicit fallthrough warning Jonathan Dieter @ 2017-03-16 7:47 ` Jonathan Dieter 2017-03-16 15:04 ` Shuah Khan 2 siblings, 0 replies; 16+ messages in thread From: Jonathan Dieter @ 2017-03-16 7:47 UTC (permalink / raw) To: linux-kernel Cc: Valentina Manea, Shuah Khan, Peter Senna Tschudin, Greg Kroah-Hartman, Krzysztof Opasiak, open list:USB OVER IP DRIVER On Mon, 2017-02-27 at 10:31 +0200, Jonathan Dieter wrote: > The usbip userspace tools call sprintf()/snprintf() and don't check for > the return value which can lead the paths to overflow, truncating the > final file in the path. > > More urgently, GCC 7 now warns that these aren't checked with > -Wformat-overflow, and with -Werror enabled in configure.ac, that makes > these tools unbuildable. > > This patch fixes these problems by replacing sprintf() with snprintf() in > one place and adding checks for the return value of snprintf(). > > > Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> > > Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> > --- > Changes since v3 > * Cast sizeof to long unsigned when printing errors to fix building for 32-bit > architectures Just wanted to check if there's anything else I need to do to make this ready? Thanks, Jonathan ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] usbip: Fix-format-overflow 2017-02-27 8:31 ` [PATCH v4 1/2] usbip: Fix-format-overflow Jonathan Dieter 2017-02-27 8:31 ` [PATCH v4 2/2] usbip: Fix implicit fallthrough warning Jonathan Dieter 2017-03-16 7:47 ` [PATCH v4 1/2] usbip: Fix-format-overflow Jonathan Dieter @ 2017-03-16 15:04 ` Shuah Khan 2017-03-17 1:37 ` Greg Kroah-Hartman 2 siblings, 1 reply; 16+ messages in thread From: Shuah Khan @ 2017-03-16 15:04 UTC (permalink / raw) To: Jonathan Dieter, linux-kernel, Greg Kroah-Hartman, Krzysztof Opasiak Cc: Valentina Manea, Peter Senna Tschudin, open list:USB OVER IP DRIVER On 02/27/2017 01:31 AM, Jonathan Dieter wrote: > The usbip userspace tools call sprintf()/snprintf() and don't check for > the return value which can lead the paths to overflow, truncating the > final file in the path. > > More urgently, GCC 7 now warns that these aren't checked with > -Wformat-overflow, and with -Werror enabled in configure.ac, that makes > these tools unbuildable. > > This patch fixes these problems by replacing sprintf() with snprintf() in > one place and adding checks for the return value of snprintf(). > > Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> > Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> Greg, Please pick this up. Acked-by: Shuah Khan <shuahkh@osg.samsung.com> thanks, -- Shuah > --- > Changes since v3 > * Cast sizeof to long unsigned when printing errors to fix building for 32-bit > architectures > > tools/usb/usbip/libsrc/usbip_common.c | 9 ++++++++- > tools/usb/usbip/libsrc/usbip_host_common.c | 28 +++++++++++++++++++++++----- > 2 files changed, 31 insertions(+), 6 deletions(-) > > diff --git a/tools/usb/usbip/libsrc/usbip_common.c b/tools/usb/usbip/libsrc/usbip_common.c > index ac73710..1517a23 100644 > --- a/tools/usb/usbip/libsrc/usbip_common.c > +++ b/tools/usb/usbip/libsrc/usbip_common.c > @@ -215,9 +215,16 @@ int read_usb_interface(struct usbip_usb_device *udev, int i, > struct usbip_usb_interface *uinf) > { > char busid[SYSFS_BUS_ID_SIZE]; > + int size; > struct udev_device *sif; > > - sprintf(busid, "%s:%d.%d", udev->busid, udev->bConfigurationValue, i); > + size = snprintf(busid, sizeof(busid), "%s:%d.%d", > + udev->busid, udev->bConfigurationValue, i); > + if (size < 0 || (unsigned int)size >= sizeof(busid)) { > + err("busid length %i >= %lu or < 0", size, > + (long unsigned)sizeof(busid)); > + return -1; > + } > > sif = udev_device_new_from_subsystem_sysname(udev_context, "usb", busid); > if (!sif) { > diff --git a/tools/usb/usbip/libsrc/usbip_host_common.c b/tools/usb/usbip/libsrc/usbip_host_common.c > index 9d41522..6ff7b60 100644 > --- a/tools/usb/usbip/libsrc/usbip_host_common.c > +++ b/tools/usb/usbip/libsrc/usbip_host_common.c > @@ -40,13 +40,20 @@ struct udev *udev_context; > static int32_t read_attr_usbip_status(struct usbip_usb_device *udev) > { > char status_attr_path[SYSFS_PATH_MAX]; > + int size; > int fd; > int length; > char status; > int value = 0; > > - snprintf(status_attr_path, SYSFS_PATH_MAX, "%s/usbip_status", > - udev->path); > + size = snprintf(status_attr_path, sizeof(status_attr_path), > + "%s/usbip_status", udev->path); > + if (size < 0 || (unsigned int)size >= sizeof(status_attr_path)) { > + err("usbip_status path length %i >= %lu or < 0", size, > + (long unsigned)sizeof(status_attr_path)); > + return -1; > + } > + > > fd = open(status_attr_path, O_RDONLY); > if (fd < 0) { > @@ -218,6 +225,7 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) > { > char attr_name[] = "usbip_sockfd"; > char sockfd_attr_path[SYSFS_PATH_MAX]; > + int size; > char sockfd_buff[30]; > int ret; > > @@ -237,10 +245,20 @@ int usbip_export_device(struct usbip_exported_device *edev, int sockfd) > } > > /* only the first interface is true */ > - snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", > - edev->udev.path, attr_name); > + size = snprintf(sockfd_attr_path, sizeof(sockfd_attr_path), "%s/%s", > + edev->udev.path, attr_name); > + if (size < 0 || (unsigned int)size >= sizeof(sockfd_attr_path)) { > + err("exported device path length %i >= %lu or < 0", size, > + (long unsigned)sizeof(sockfd_attr_path)); > + return -1; > + } > > - snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); > + size = snprintf(sockfd_buff, sizeof(sockfd_buff), "%d\n", sockfd); > + if (size < 0 || (unsigned int)size >= sizeof(sockfd_buff)) { > + err("socket length %i >= %lu or < 0", size, > + (long unsigned)sizeof(sockfd_buff)); > + return -1; > + } > > ret = write_sysfs_attribute(sockfd_attr_path, sockfd_buff, > strlen(sockfd_buff)); > ^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [PATCH v4 1/2] usbip: Fix-format-overflow 2017-03-16 15:04 ` Shuah Khan @ 2017-03-17 1:37 ` Greg Kroah-Hartman 0 siblings, 0 replies; 16+ messages in thread From: Greg Kroah-Hartman @ 2017-03-17 1:37 UTC (permalink / raw) To: Shuah Khan Cc: Jonathan Dieter, linux-kernel, Krzysztof Opasiak, Valentina Manea, Peter Senna Tschudin, open list:USB OVER IP DRIVER On Thu, Mar 16, 2017 at 09:04:54AM -0600, Shuah Khan wrote: > On 02/27/2017 01:31 AM, Jonathan Dieter wrote: > > The usbip userspace tools call sprintf()/snprintf() and don't check for > > the return value which can lead the paths to overflow, truncating the > > final file in the path. > > > > More urgently, GCC 7 now warns that these aren't checked with > > -Wformat-overflow, and with -Werror enabled in configure.ac, that makes > > these tools unbuildable. > > > > This patch fixes these problems by replacing sprintf() with snprintf() in > > one place and adding checks for the return value of snprintf(). > > > > Reviewed-by: Peter Senna Tschudin <peter.senna@gmail.com> > > Signed-off-by: Jonathan Dieter <jdieter@lesbg.com> > > Greg, > > Please pick this up. > > Acked-by: Shuah Khan <shuahkh@osg.samsung.com> Thanks, still digging through USB patches... ^ permalink raw reply [flat|nested] 16+ messages in thread
end of thread, other threads:[~2017-03-17 1:40 UTC | newest]
Thread overview: 16+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <CGME20170220205937epcas3p4e334a658cf4d67f4a7d5a4ae7ce10afc@epcas3p4.samsung.com>
2017-02-20 20:51 ` [PATCH 1/2] Fix format overflows Jonathan Dieter
2017-02-20 20:51 ` [PATCH 2/2] Fix implicit fallthrough warning Jonathan Dieter
2017-02-21 6:12 ` [PATCH 1/2] Fix format overflows Krzysztof Opasiak
2017-02-21 6:48 ` Jonathan Dieter
2017-02-21 17:50 ` Jonathan Dieter
2017-02-21 17:57 ` [PATCH v2 1/2] usbip: Fix-format-overflow Jonathan Dieter
2017-02-21 17:57 ` [PATCH v2 2/2] usbip: Fix implicit fallthrough warning Jonathan Dieter
2017-02-22 5:49 ` [PATCH v2 1/2] usbip: Fix-format-overflow Krzysztof Opasiak
2017-02-22 18:14 ` Jonathan Dieter
2017-02-22 18:17 ` [PATCH v3 " Jonathan Dieter
2017-02-22 18:18 ` [PATCH v3 2/2] usbip: Fix implicit fallthrough warning Jonathan Dieter
2017-02-27 8:31 ` [PATCH v4 1/2] usbip: Fix-format-overflow Jonathan Dieter
2017-02-27 8:31 ` [PATCH v4 2/2] usbip: Fix implicit fallthrough warning Jonathan Dieter
2017-03-16 7:47 ` [PATCH v4 1/2] usbip: Fix-format-overflow Jonathan Dieter
2017-03-16 15:04 ` Shuah Khan
2017-03-17 1:37 ` Greg Kroah-Hartman
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).