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

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

[Alan Stern]

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.

It isn't necessary to do this.  The check you added can never be 
violated.

To see why, read the code.  The first thing the USBDEVFS_URB_TYPE_ISO 
case does is ensure that uurb->number_of_packets <= 128.  The loop that 
accumulates the individual packet sizes also limits isopkt[u].length to 
be <= 98304.  Therefore the largest possible buffer size for an 
isochronous transfer is 128 * 98304 = 12582912.  But USBFS_XFER_MAX is 
(UINT_MAX / 2 - 1000000) or 2146483648, which is a lot bigger (about 2 
billion vs. 12 million).  This means the calculated value for 
uurb->buffer_length cannot exceed USBFS_XFER_MAX.

The real point of the USBFS_XFER_MAX check is to limit the size of bulk 
and interrupt transfers.

Alan Stern

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 10:54:31AM -0400, Alan Stern wrote:
> 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.
> 
> It isn't necessary to do this.  The check you added can never be 
> violated.
> 
> To see why, read the code.  The first thing the USBDEVFS_URB_TYPE_ISO 
> case does is ensure that uurb->number_of_packets <= 128.  The loop that 
> accumulates the individual packet sizes also limits isopkt[u].length to 
> be <= 98304.  Therefore the largest possible buffer size for an 
> isochronous transfer is 128 * 98304 = 12582912.  But USBFS_XFER_MAX is 
> (UINT_MAX / 2 - 1000000) or 2146483648, which is a lot bigger (about 2 
> billion vs. 12 million).  This means the calculated value for 
> uurb->buffer_length cannot exceed USBFS_XFER_MAX.
> 
> The real point of the USBFS_XFER_MAX check is to limit the size of bulk 
> and interrupt transfers.

Awe, you spoiled the test :)

Reid, again, remember that the _BEST_ LLM right now (top of the line,
frontier model, no guardrails) makes up patches that are 50% of the time
wrong.  You MUST have a reproducer to have a LLM be able to prove it, OR
take the time to manually review it yourself.

Best of luck!

greg k-h

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

[Alan Stern]

On Thu, Oct 15, 2026 at 05:26:18PM +0200, Greg KH wrote:
> Awe, you spoiled the test :)

Oops, sorry about that.

> Reid, again, remember that the _BEST_ LLM right now (top of the line,
> frontier model, no guardrails) makes up patches that are 50% of the time
> wrong.  You MUST have a reproducer to have a LLM be able to prove it, OR
> take the time to manually review it yourself.

Absolutely.  Don't believe anything an LLM says without doing at least a 
little verification.  Lots of the patches they make up are just plain no 
good.  And even when the patches are okay, often they are "solutions" to 
things that weren't problems in the first place.

Alan Stern