* [PATCH 4.9] usbip: Fix potential format overflow in userspace tools
2018-01-24 2:32 [PATCH 4.9] usbip: Fix implicit fallthrough warning Shuah Khan
@ 2018-01-24 2:32 ` Shuah Khan
2018-01-24 9:11 ` [PATCH 4.9] usbip: Fix implicit fallthrough warning Greg KH
1 sibling, 0 replies; 3+ messages in thread
From: Shuah Khan @ 2018-01-24 2:32 UTC (permalink / raw)
To: valentina.manea.m, shuah, gregkh, jdieter
Cc: peter.senna, linux-usb, linux-kernel, stable, Shuah Khan
From: Jonathan Dieter <jdieter@lesbg.com>
Upstream commit e5dfa3f902b9 ("usbip: Fix potential format overflow in
userspace tools")
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>
Acked-by: Shuah Khan <shuahkh@osg.samsung.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
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 ac73710473de..8000445ff884 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,
+ (unsigned long)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 9d415228883d..c10379439668 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,
+ (unsigned long)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,
+ (unsigned long)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,
+ (unsigned long)sizeof(sockfd_buff));
+ return -1;
+ }
ret = write_sysfs_attribute(sockfd_attr_path, sockfd_buff,
strlen(sockfd_buff));
--
2.14.1
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH 4.9] usbip: Fix implicit fallthrough warning
2018-01-24 2:32 [PATCH 4.9] usbip: Fix implicit fallthrough warning Shuah Khan
2018-01-24 2:32 ` [PATCH 4.9] usbip: Fix potential format overflow in userspace tools Shuah Khan
@ 2018-01-24 9:11 ` Greg KH
1 sibling, 0 replies; 3+ messages in thread
From: Greg KH @ 2018-01-24 9:11 UTC (permalink / raw)
To: Shuah Khan
Cc: valentina.manea.m, shuah, jdieter, peter.senna, linux-usb,
linux-kernel, stable
On Tue, Jan 23, 2018 at 07:32:11PM -0700, Shuah Khan wrote:
> From: Jonathan Dieter <jdieter@lesbg.com>
>
> Upstream commit cfd6ed4537a9 ("usbip: Fix implicit fallthrough warning")
>
> 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>
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
> ---
> tools/usb/usbip/src/usbip.c | 2 ++
> 1 file changed, 2 insertions(+)
All of these usbip tools patches are now queued up, thanks!
greg k-h
^ permalink raw reply [flat|nested] 3+ messages in thread