Re: usb: devio: USBFS_XFER_MAX check bypassed for iso urbs due to incorrect ordering, potential oob heap write

Re: usb: devio: USBFS_XFER_MAX check bypassed for iso urbs due to incorrect ordering, potential oob heap write

[Greg KH]

On Wed, Oct 14, 2026 at 05:36:56PM -0400, Reid D wrote:
> usb: devio: USBFS_XFER_MAX check bypassed for iso urbs
> due to incorrect ordering, potential oob heap write
> 
> file: drivers/usb/core/devio.c
> function: proc_do_submiturb()
> ioctl: USBDEVFS_SUBMITURB
> 
> the bug:
> 
> the size check at line 1647 runs on buffer_length before the iso
> path has computed totlen. the iso path then overwrites buffer_length
> with totlen at line 1768, so the check never actually validates
> the real transfer size.
> 
>     line 1647 - check runs here on the wrong value:
>     if ((unsigned int)uurb->buffer_length >= USBFS_XFER_MAX)
>         return -EINVAL;
> 
>     line 1768 - real value computed after the check:
>     uurb->buffer_length = totlen;
> 
>     line 1843 - kmalloc uses the unchecked value:
>     as->urb->transfer_buffer = kmalloc(uurb->buffer_length, ...);
> 
> impact:
> 
> if totlen exceeds the kmalloc allocation size, the copy_from_user
> that follows writes past the end of the heap buffer. for an out
> transfer the user controls both the length and the data, giving a
> heap write-what-where primitive.
> 
> current constants (128 packets * 98304 max each = 12mb) keep
> totlen below USBFS_XFER_MAX (16mb) so this is not exploitable
> today. the check ordering is structurally wrong regardless and
> will become a live bug if those constants change.
> 
> fix:
> 
>     uurb->buffer_length = totlen;
>     if (uurb->buffer_length >= USBFS_XFER_MAX)
>         return -EINVAL;
> 

Great, please turn this into a real patch that can be applied so you get
full credit for resolving this.  Your favorite llm can easily do this
for you (and it can test it, which you really should also do...)

thanks,

greg k-h

Re: usb: devio: USBFS_XFER_MAX check bypassed for iso urbs due to incorrect ordering, potential oob heap write

[Reid D]

[-- Attachment #1.1: Type: text/plain, Size: 560 bytes --]

Greg, sorry about that.

The fix moves the USBFS_XFER_MAX check to after totlen is computed so it
validates the actual transfer size used for the kmalloc.

On Thu, Oct 15, 2026 at 8:02 AM Greg KH <gregkh@linuxfoundation.org> wrote:

> On Thu, Oct 15, 2026 at 07:30:26AM -0400, Reid D wrote:
> > Greg,
> >
> > Please check out the fix,
>
> <snip>
>
> For some reason you sent this only to me, which is a bit rude to
> everyone else on the mailing list.  I'll be glad to respond if you
> resend it to everyone.
>
> thanks,
>
> greg k-h
>

[-- Attachment #1.2: Type: text/html, Size: 949 bytes --]

[-- Attachment #2: 0001-usb-devio-fix-USBFS_XFER_MAX-check-ordering-for-iso-.patch --]
[-- Type: text/x-patch, Size: 1822 bytes --]

From e3b32fafac13bc84183624fabc616cdcf3719cad Mon Sep 17 00:00:00 2001
From: "Reid Downing (b1nv0id)" <skibletz@gmail.com>
Date: Thu, 15 Oct 2026 07:24:20 -0400
Subject: [PATCH] usb: devio: fix USBFS_XFER_MAX check ordering for iso urbs

Signed-off-by: Reid Downing (b1nv0id) <skibletz@gmail.com>
---
 drivers/usb/core/devio.c | 20 +++++++++++---------
 1 file changed, 11 insertions(+), 9 deletions(-)

diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index e779ebdfa153..fdb545e4b511 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -1764,10 +1764,15 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 		}
 		u *= sizeof(struct usb_iso_packet_descriptor);
 		uurb->buffer_length = totlen;
-				if (uurb->buffer_length >= USBFS_XFER_MAX) {
+		if (uurb->buffer_length >= USBFS_XFER_MAX) {
 			ret = -EINVAL;
 			goto error;
 		}
+		break;
+
+	default:
+		return -EINVAL;
+	}
 
 	if (uurb->buffer_length > 0 &&
 			!access_ok(uurb->buffer, uurb->buffer_length)) {
@@ -2326,13 +2331,6 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
 	if (!connected(ps))
 		return -ENODEV;
 
-	if (ps->dev->state != USB_STATE_CONFIGURED)
-		return -EHOSTUNREACH;
-
-	intf = usb_ifnum_to_if(ps->dev, ctl->ifno);
-	if (!intf)
-		return -EINVAL;
-
 	/* alloc buffer */
 	size = _IOC_SIZE(ctl->ioctl_code);
 	if (size > 0) {
@@ -2349,7 +2347,11 @@ static int proc_ioctl(struct usb_dev_state *ps, struct usbdevfs_ioctl *ctl)
 		}
 	}
 
-	switch (ctl->ioctl_code) {
+	if (ps->dev->state != USB_STATE_CONFIGURED)
+		retval = -EHOSTUNREACH;
+	else if (!(intf = usb_ifnum_to_if(ps->dev, ctl->ifno)))
+		retval = -EINVAL;
+	else switch (ctl->ioctl_code) {
 
 	/* disconnect kernel driver from interface */
 	case USBDEVFS_DISCONNECT:
-- 
2.53.0

Re: usb: devio: USBFS_XFER_MAX check bypassed for iso urbs due to incorrect ordering, potential oob heap write

[Greg KH]

On Thu, Oct 15, 2026 at 08:07:49AM -0400, Reid D wrote:
> Greg, sorry about that.
> 
> The fix moves the USBFS_XFER_MAX check to after totlen is computed so it
> validates the actual transfer size used for the kmalloc.
> 
> On Thu, Oct 15, 2026 at 8:02 AM Greg KH <gregkh@linuxfoundation.org> wrote:
> 
> > On Thu, Oct 15, 2026 at 07:30:26AM -0400, Reid D wrote:
> > > Greg,
> > >
> > > Please check out the fix,
> >
> > <snip>
> >
> > For some reason you sent this only to me, which is a bit rude to
> > everyone else on the mailing list.  I'll be glad to respond if you
> > resend it to everyone.
> >
> > thanks,
> >
> > greg k-h
> >

> From e3b32fafac13bc84183624fabc616cdcf3719cad Mon Sep 17 00:00:00 2001
> From: "Reid Downing (b1nv0id)" <skibletz@gmail.com>
> Date: Thu, 15 Oct 2026 07:24:20 -0400
> Subject: [PATCH] usb: devio: fix USBFS_XFER_MAX check ordering for iso urbs
> 
> Signed-off-by: Reid Downing (b1nv0id) <skibletz@gmail.com>
> ---
>  drivers/usb/core/devio.c | 20 +++++++++++---------
>  1 file changed, 11 insertions(+), 9 deletions(-)

Please point your llm at the docs to show how to create a real patch
(hint you need a changelog and it can't be attached.)

Also please test your patch so that you know it actually fixes the issue
you are reporting.  Remember a huge % of llm-found bugs are actually not
bugs at all, you need to make a patch (which you did here, so that cuts
the odds down to 50%) and then test it (which cuts down the number even
more.)

thanks,

greg k-h