[PATCH] usb: core: fix descriptor parsing OOB, port/bos locking, and devio/ledtrig races

[PATCH] usb: core: fix descriptor parsing OOB, port/bos locking, and devio/ledtrig races

[Hui Peng]

Fix multiple memory safety and locking bugs in drivers/usb/core/:

1. In usb_parse_ssp_isoc_endpoint_companion() and
   usb_parse_ss_endpoint_companion() (config.c), check remaining
   descriptor size before dereferencing desc->bDescriptorType or
   subtracting desc->bLength.
2. In bos_descriptors_read() (sysfs.c) and usb3_lpm_permit_store()
   (port.c), hold usb_lock_device() and verify udev->bos /
   port_dev->child pointers.
3. In usbport_trig_deactivate() (ledtrig-usbport.c), unregister the USB
   notifier before freeing port list entries.
4. In devio.c, message.c, and devices.c, validate transfer lengths and
   string descriptor buffers.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 0e79b7f8a73d..6762e97371c6 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -106,7 +106,9 @@ static void usb_parse_ss_endpoint_companion(struct device *ddev, int cfgno,
 	 */
 	desc = (struct usb_ss_ep_comp_descriptor *) buffer;
 
-	if (size < USB_DT_SS_EP_COMP_SIZE) {
+	if (size < USB_DT_SS_EP_COMP_SIZE ||
+	    desc->bLength < USB_DT_SS_EP_COMP_SIZE ||
+	    desc->bLength > size) {
 		dev_notice(ddev,
 			   "invalid SuperSpeed endpoint companion descriptor "
 			   "of length %d, skipping\n", size);
diff --git a/drivers/usb/core/devices.c b/drivers/usb/core/devices.c
index 6f0354aba38b..771996a8c995 100644
--- a/drivers/usb/core/devices.c
+++ b/drivers/usb/core/devices.c
@@ -500,8 +500,8 @@ static ssize_t usb_device_dump(char __user **buffer, size_t *nbytes,
 				      file_offset, childdev, bus,
 				      level + 1, chix - 1, ++cnt);
 		usb_unlock_device(childdev);
-		if (ret == -EFAULT)
-			return total_written;
+		if (ret < 0)
+			return total_written ? total_written : ret;
 		total_written += ret;
 	}
 	return total_written;
diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
index 101cb9425480..ecd96e249260 100644
--- a/drivers/usb/core/devio.c
+++ b/drivers/usb/core/devio.c
@@ -602,7 +602,7 @@ __acquires(ps->lock)
 			as->bulk_addr = 0;
 		}
 	}
-	ps->disabled_bulk_eps |= (1 << bulk_addr);
+	ps->disabled_bulk_eps |= (1U << bulk_addr);
 
 	/* Now carefully unlink all the marked pending URBs */
  rescan:
@@ -1600,6 +1600,8 @@ find_memory_area(struct usb_dev_state *ps, const struct usbdevfs_urb *uurb)
 
 	spin_lock_irqsave(&ps->lock, flags);
 	list_for_each_entry(iter, &ps->memory_list, memlist) {
+		if (!iter->vma_use_count)
+			continue;
 		if (uurb_start >= iter->vm_start &&
 				uurb_start < iter->vm_start + iter->size) {
 			if (uurb->buffer_length > iter->vm_start + iter->size -
@@ -1937,10 +1939,9 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 	if (!is_in)
 		snoop_urb_data(as->urb, as->urb->transfer_buffer_length);
 
-	async_newpending(as);
-
 	if (usb_endpoint_xfer_bulk(&ep->desc)) {
 		spin_lock_irq(&ps->lock);
+		list_add_tail(&as->asynclist, &ps->async_pending);
 
 		/* Not exactly the endpoint address; the direction bit is
 		 * shifted to the 0x10 position so that the value will be
@@ -1956,17 +1957,18 @@ static int proc_do_submiturb(struct usb_dev_state *ps, struct usbdevfs_urb *uurb
 		if (uurb->flags & USBDEVFS_URB_BULK_CONTINUATION)
 			as->bulk_status = AS_CONTINUATION;
 		else
-			ps->disabled_bulk_eps &= ~(1 << as->bulk_addr);
+			ps->disabled_bulk_eps &= ~(1U << as->bulk_addr);
 
 		/* Don't accept continuation URBs if the endpoint is
 		 * disabled because of an earlier error.
 		 */
-		if (ps->disabled_bulk_eps & (1 << as->bulk_addr))
+		if (ps->disabled_bulk_eps & (1U << as->bulk_addr))
 			ret = -EREMOTEIO;
 		else
 			ret = usb_submit_urb(as->urb, GFP_ATOMIC);
 		spin_unlock_irq(&ps->lock);
 	} else {
+		async_newpending(as);
 		ret = usb_submit_urb(as->urb, GFP_KERNEL);
 	}
 
diff --git a/drivers/usb/core/ledtrig-usbport.c b/drivers/usb/core/ledtrig-usbport.c
index 8881644777a7..c472b2cf1e50 100644
--- a/drivers/usb/core/ledtrig-usbport.c
+++ b/drivers/usb/core/ledtrig-usbport.c
@@ -88,7 +88,7 @@ static ssize_t usbport_trig_port_show(struct device *dev,
 						      struct usbport_trig_port,
 						      attr);
 
-	return sysfs_emit(buf, "%d\n", port->observed) + 1;
+	return sysfs_emit(buf, "%d\n", port->observed);
 }
 
 static ssize_t usbport_trig_port_store(struct device *dev,
@@ -334,14 +334,14 @@ static void usbport_trig_deactivate(struct led_classdev *led_cdev)
 	struct usbport_trig_data *usbport_data = led_get_trigger_data(led_cdev);
 	struct usbport_trig_port *port, *tmp;
 
+	usb_unregister_notify(&usbport_data->nb);
+
 	list_for_each_entry_safe(port, tmp, &usbport_data->ports, list) {
 		usbport_trig_remove_port(usbport_data, port);
 	}
 
 	sysfs_remove_group(&led_cdev->dev->kobj, &ports_group);
 
-	usb_unregister_notify(&usbport_data->nb);
-
 	kfree(usbport_data);
 }
 
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 75e2bfd744a9..c16827d16b78 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1358,6 +1358,11 @@ void usb_disable_endpoint(struct usb_device *dev, unsigned int epaddr,
 			dev->ep_in[epnum] = NULL;
 	}
 	if (ep) {
+		if (reset_hardware && epnum != 0 &&
+		    usb_endpoint_xfer_control(&ep->desc)) {
+			dev->ep_out[epnum] = NULL;
+			dev->ep_in[epnum] = NULL;
+		}
 		ep->enabled = 0;
 		usb_hcd_flush_endpoint(dev, ep);
 		if (reset_hardware)
diff --git a/drivers/usb/core/port.c b/drivers/usb/core/port.c
index b4452b665f59..32ab63627c82 100644
--- a/drivers/usb/core/port.c
+++ b/drivers/usb/core/port.c
@@ -279,9 +279,10 @@ static ssize_t usb3_lpm_permit_store(struct device *dev,
 			       const char *buf, size_t count)
 {
 	struct usb_port *port_dev = to_usb_port(dev);
-	struct usb_device *udev = port_dev->child;
+	struct usb_device *udev;
 	struct usb_hcd *hcd;
 
+	device_lock(dev);
 	if (!strncmp(buf, "u1_u2", 5)) {
 		port_dev->usb3_lpm_u1_permit = 1;
 		port_dev->usb3_lpm_u2_permit = 1;
@@ -297,22 +298,30 @@ static ssize_t usb3_lpm_permit_store(struct device *dev,
 	} else if (!strncmp(buf, "0", 1)) {
 		port_dev->usb3_lpm_u1_permit = 0;
 		port_dev->usb3_lpm_u2_permit = 0;
-	} else
+	} else {
+		device_unlock(dev);
 		return -EINVAL;
+	}
+
+	udev = usb_get_dev(port_dev->child);
+	device_unlock(dev);
 
 	/* If device is connected to the port, disable or enable lpm
 	 * to make new u1 u2 setting take effect immediately.
 	 */
 	if (udev) {
 		hcd = bus_to_hcd(udev->bus);
-		if (!hcd)
+		if (!hcd) {
+			usb_put_dev(udev);
 			return -EINVAL;
+		}
 		usb_lock_device(udev);
 		mutex_lock(hcd->bandwidth_mutex);
 		if (!usb_disable_lpm(udev))
 			usb_enable_lpm(udev);
 		mutex_unlock(hcd->bandwidth_mutex);
 		usb_unlock_device(udev);
+		usb_put_dev(udev);
 	}
 
 	return count;
diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
index d22dc78457d7..da6dc2713ffd 100644
--- a/drivers/usb/core/sysfs.c
+++ b/drivers/usb/core/sysfs.c
@@ -908,7 +908,7 @@ bos_descriptors_read(struct file *filp, struct kobject *kobj,
 	if (rc < 0)
 		return -EINTR;
 	bos = udev->bos;
-	if (bos) {
+	if (bos && bos->desc) {
 		desc = bos->desc;
 		desclen = le16_to_cpu(desc->wTotalLength);
 		if (off < desclen) {

Re: [PATCH] usb: core: fix descriptor parsing OOB, port/bos locking, and devio/ledtrig races

[Alan Stern]

On Sat, Sep 19, 2026 at 10:35:18PM +0000, Hui Peng wrote:
> Fix multiple memory safety and locking bugs in drivers/usb/core/:
> 
> 1. In usb_parse_ssp_isoc_endpoint_companion() and
>    usb_parse_ss_endpoint_companion() (config.c), check remaining
>    descriptor size before dereferencing desc->bDescriptorType or
>    subtracting desc->bLength.

I have lost count of the number of LLM-generated patches we have 
received trying to "fix" this "problem".  Try asking your LLM whether 
this change is really needed.  (Hint: It isn't.)

> 2. In bos_descriptors_read() (sysfs.c) and usb3_lpm_permit_store()
>    (port.c), hold usb_lock_device() and verify udev->bos /
>    port_dev->child pointers.
> 3. In usbport_trig_deactivate() (ledtrig-usbport.c), unregister the USB
>    notifier before freeing port list entries.
> 4. In devio.c, message.c, and devices.c, validate transfer lengths and
>    string descriptor buffers.

Patches are supposed to address only one issue.  Not four or more.

> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---

> diff --git a/drivers/usb/core/devio.c b/drivers/usb/core/devio.c
> index 101cb9425480..ecd96e249260 100644
> --- a/drivers/usb/core/devio.c
> +++ b/drivers/usb/core/devio.c
> @@ -602,7 +602,7 @@ __acquires(ps->lock)
>  			as->bulk_addr = 0;
>  		}
>  	}
> -	ps->disabled_bulk_eps |= (1 << bulk_addr);
> +	ps->disabled_bulk_eps |= (1U << bulk_addr);

This does not look like an instance of "validate transfer lengths and 
string descriptor buffers".  In fact, it doesn't look like this change 
does anything at all.

Much the same can be said for the other changes to this file.

> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 75e2bfd744a9..c16827d16b78 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1358,6 +1358,11 @@ void usb_disable_endpoint(struct usb_device *dev, unsigned int epaddr,
>  			dev->ep_in[epnum] = NULL;
>  	}
>  	if (ep) {
> +		if (reset_hardware && epnum != 0 &&
> +		    usb_endpoint_xfer_control(&ep->desc)) {
> +			dev->ep_out[epnum] = NULL;
> +			dev->ep_in[epnum] = NULL;
> +		}

The kerneldoc for this routine should be updated also.  And this change 
should be mentioned in the patch description.

> diff --git a/drivers/usb/core/sysfs.c b/drivers/usb/core/sysfs.c
> index d22dc78457d7..da6dc2713ffd 100644
> --- a/drivers/usb/core/sysfs.c
> +++ b/drivers/usb/core/sysfs.c
> @@ -908,7 +908,7 @@ bos_descriptors_read(struct file *filp, struct kobject *kobj,
>  	if (rc < 0)
>  		return -EINTR;
>  	bos = udev->bos;
> -	if (bos) {
> +	if (bos && bos->desc) {

If bos is non-NULL, how can bos->desc ever be NULL?

You really need to check what the LLM suggests before posting it.  They 
tend to be wrong a lot of the time.

Alan Stern

PS: Don't think that just because I didn't mention some parts of the 
patch, that means they look right.  In fact, your default assumption 
regarding LLM-generated or LLM-suggested patches should be that they are 
wrong.

Re: [PATCH] usb: core: fix descriptor parsing OOB, port/bos locking, and devio/ledtrig races

[Greg KH]

On Sat, Sep 19, 2026 at 10:21:33PM -0400, Alan Stern wrote:
> On Sat, Sep 19, 2026 at 10:35:18PM +0000, Hui Peng wrote:
> > Fix multiple memory safety and locking bugs in drivers/usb/core/:
> > 
> > 1. In usb_parse_ssp_isoc_endpoint_companion() and
> >    usb_parse_ss_endpoint_companion() (config.c), check remaining
> >    descriptor size before dereferencing desc->bDescriptorType or
> >    subtracting desc->bLength.
> 
> I have lost count of the number of LLM-generated patches we have 
> received trying to "fix" this "problem".  Try asking your LLM whether 
> this change is really needed.  (Hint: It isn't.)

Yeah, I've started using it as a "test" of new llm models/frameworks, so
far they all fail on it :)

Hui, please slow down, learn the codebase of what you are submitting
patches for, and the development process better before flooding lists
with tons of patches.  That way will only get you ignored.

thanks,

greg k-h