Skip to contentSkip to conversation

patch "USB: devio: Prevent integer overflow in proc_do_submiturb()" added to usb-linus

PATCH [email protected] 1 participant 1 message Sep 25, 2017

Subsystems (2)
lore
Share & tools
lore

[email protected]

author maintainer Applied
#0 lore raw

This is a note to let you know that I've just added the patch titled

USB: devio: Prevent integer overflow in proc_do_submiturb()

to my usb git tree which can be found at
git://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git
in the usb-linus branch.

The patch will show up in the next release of the linux-next tree (usually sometime within the next 24 hours during the week.)

The patch will hopefully also be merged in Linus's tree for the next -rc kernel release.

If you have any questions about this process, please let me know.

Quoted 1 line

From 57999d1107c1e60c2ca7088f2ac0f819e2f554b3 Mon Sep 17 00:00:00 2001

From: Dan Carpenter <[email protected]>
Date: Fri, 22 Sep 2017 23:43:25 +0300
Subject: USB: devio: Prevent integer overflow in proc_do_submiturb()

There used to be an integer overflow check in proc_do_submiturb() but we removed it. It turns out that it's still required. The uurb->buffer_length variable is a signed integer and it's controlled by the user. It can lead to an integer overflow when we do:

	num_sgs = DIV_ROUND_UP(uurb->buffer_length, USB_SG_SIZE);

If we strip away the macro then that line looks like this:

	num_sgs = (uurb->buffer_length + USB_SG_SIZE - 1) / USB_SG_SIZE;
                   ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

It's the first addition which can overflow.

Fixes 1129d270cbfb ("USB: Increase usbfs transfer limit")
Cc stable <[email protected]>
---
 drivers/usb/core/devio.c | 5 +++++
 1 file changed, 5 insertions(+)
drivers/usb/core/devio.c +5 -0
index 318bb3b96687..e9326f31db8d 100644
@@ -140,6 +140,9 @@ module_param(usbfs_memory_mb, uint, 0644);
 MODULE_PARM_DESC(usbfs_memory_mb,
 		"maximum MB allowed for usbfs buffers (0 = no limit)");
 
+/* Hard limit, necessary to avoid arithmetic overflow */
+#define USBFS_XFER_MAX         (UINT_MAX / 2 - 1000000)
+
 static atomic64_t usbfs_memory_usage;	/* Total memory currently allocated */
 
 /* Check whether it's okay to allocate more memory for a transfer */
@@ -1460,6 +1463,8 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 				USBDEVFS_URB_ZERO_PACKET |
 				USBDEVFS_URB_NO_INTERRUPT))
 		return -EINVAL;
+	if ((unsigned int)uurb->buffer_length >= USBFS_XFER_MAX)
+		return -EINVAL;
 	if (uurb->buffer_length > 0 && !uurb->buffer)
 		return -EINVAL;
 	if (!(uurb->type == USBDEVFS_URB_TYPE_CONTROL &&
Signature · 1 line
2.14.1