* [PATCH] usb: core: fix descriptor parsing OOB, port/bos locking, and devio/ledtrig races
@ 2026-09-19 22:35 Hui Peng
2026-09-20 2:21 ` Alan Stern
0 siblings, 1 reply; 6+ messages in thread
From: Hui Peng @ 2026-09-19 22:35 UTC (permalink / raw)
To: gregkh, stern, michal.pecio; +Cc: linux-usb, linux-kernel
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) {
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] usb: core: fix descriptor parsing OOB, port/bos locking, and devio/ledtrig races
2026-09-19 22:35 [PATCH] usb: core: fix descriptor parsing OOB, port/bos locking, and devio/ledtrig races Hui Peng
@ 2026-09-20 2:21 ` Alan Stern
2026-09-20 5:12 ` Greg KH
2026-09-20 23:17 ` [PATCH v2] usb: core: clear both ep_in and ep_out for non-ep0 control endpoints Hui Peng
0 siblings, 2 replies; 6+ messages in thread
From: Alan Stern @ 2026-09-20 2:21 UTC (permalink / raw)
To: Hui Peng; +Cc: gregkh, michal.pecio, linux-usb, linux-kernel
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.
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] usb: core: fix descriptor parsing OOB, port/bos locking, and devio/ledtrig races
2026-09-20 2:21 ` Alan Stern
@ 2026-09-20 5:12 ` Greg KH
2026-09-20 23:17 ` [PATCH v2] usb: core: clear both ep_in and ep_out for non-ep0 control endpoints Hui Peng
1 sibling, 0 replies; 6+ messages in thread
From: Greg KH @ 2026-09-20 5:12 UTC (permalink / raw)
To: Alan Stern; +Cc: Hui Peng, michal.pecio, linux-usb, linux-kernel
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH v2] usb: core: clear both ep_in and ep_out for non-ep0 control endpoints
2026-09-20 2:21 ` Alan Stern
2026-09-20 5:12 ` Greg KH
@ 2026-09-20 23:17 ` Hui Peng
2026-09-21 0:21 ` Alan Stern
1 sibling, 1 reply; 6+ messages in thread
From: Hui Peng @ 2026-09-20 23:17 UTC (permalink / raw)
To: Alan Stern, Greg Kroah-Hartman
Cc: michal.pecio, linux-usb, linux-kernel, Hui Peng, stable
In usb_enable_endpoint(), non-zero control endpoints (where
usb_endpoint_xfer_control(&ep->desc) is true) populate both
dev->ep_in[epnum] and dev->ep_out[epnum] with the same
struct usb_host_endpoint pointer.
However, usb_disable_endpoint() only clears either dev->ep_out[epnum]
or dev->ep_in[epnum] depending on the direction bit of epaddr when
reset_hardware is set, leaving the opposite direction's array slot
pointing to the disabled/freed endpoint.
Clear both dev->ep_out[epnum] and dev->ep_in[epnum] when disabling a
non-ep0 control endpoint with reset_hardware set, and update the
function kerneldoc accordingly.
Tested in QEMU against Linux 7.3.0-rc3 using dummy_hcd and raw-gadget to
enumerate a USB device with a non-zero control endpoint (bEndpointAddress
0x01, bmAttributes USB_ENDPOINT_XFER_CONTROL), verifying that both
dev->ep_in[1] and dev->ep_out[1] are cleared when the interface is
disabled.
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
Changes in v2:
- Dropped all changes in the other files (config.c, hub.c, port.c,
devio.c, sysfs.c, and ledtrig-usbport.c) per Alan Stern and Greg
Kroah-Hartman, keeping only the usb_disable_endpoint() fix in
drivers/usb/core/message.c.
- Updated the usb_disable_endpoint() kerneldoc comment and commit
description per Alan Stern.
drivers/usb/core/message.c | 8 +++++++-
1 file changed, 7 insertions(+), 1 deletion(-)
diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
index 75e2bfd744a9..0cd6dd2b6334 100644
--- a/drivers/usb/core/message.c
+++ b/drivers/usb/core/message.c
@@ -1337,7 +1337,8 @@ static void remove_intf_ep_devs(struct usb_interface *intf)
*
* Disables the endpoint for URB submission and nukes all pending URBs.
* If @reset_hardware is set then also deallocates hcd/hardware state
- * for the endpoint.
+ * for the endpoint (clearing both ep_in and ep_out pointers for
+ * bidirectional non-ep0 control endpoints).
*/
void usb_disable_endpoint(struct usb_device *dev, unsigned int epaddr,
bool reset_hardware)
@@ -1358,6 +1359,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)
--
2.47.3
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] usb: core: clear both ep_in and ep_out for non-ep0 control endpoints
2026-09-20 23:17 ` [PATCH v2] usb: core: clear both ep_in and ep_out for non-ep0 control endpoints Hui Peng
@ 2026-09-21 0:21 ` Alan Stern
2026-09-21 1:42 ` Hui Peng
0 siblings, 1 reply; 6+ messages in thread
From: Alan Stern @ 2026-09-21 0:21 UTC (permalink / raw)
To: Hui Peng
Cc: Greg Kroah-Hartman, michal.pecio, linux-usb, linux-kernel, stable
On Sun, Sep 20, 2026 at 11:17:02PM +0000, Hui Peng wrote:
> In usb_enable_endpoint(), non-zero control endpoints (where
> usb_endpoint_xfer_control(&ep->desc) is true) populate both
> dev->ep_in[epnum] and dev->ep_out[epnum] with the same
> struct usb_host_endpoint pointer.
>
> However, usb_disable_endpoint() only clears either dev->ep_out[epnum]
> or dev->ep_in[epnum] depending on the direction bit of epaddr when
> reset_hardware is set, leaving the opposite direction's array slot
> pointing to the disabled/freed endpoint.
>
> Clear both dev->ep_out[epnum] and dev->ep_in[epnum] when disabling a
> non-ep0 control endpoint with reset_hardware set, and update the
> function kerneldoc accordingly.
>
> Tested in QEMU against Linux 7.3.0-rc3 using dummy_hcd and raw-gadget to
> enumerate a USB device with a non-zero control endpoint (bEndpointAddress
> 0x01, bmAttributes USB_ENDPOINT_XFER_CONTROL), verifying that both
> dev->ep_in[1] and dev->ep_out[1] are cleared when the interface is
> disabled.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Assisted-by: LLM
> Signed-off-by: Hui Peng <benquike@gmail.com>
> ---
> Changes in v2:
> - Dropped all changes in the other files (config.c, hub.c, port.c,
> devio.c, sysfs.c, and ledtrig-usbport.c) per Alan Stern and Greg
> Kroah-Hartman, keeping only the usb_disable_endpoint() fix in
> drivers/usb/core/message.c.
> - Updated the usb_disable_endpoint() kerneldoc comment and commit
> description per Alan Stern.
Acked-by: Alan Stern <stern@rowland.harvard.edu>
You might consider looking through other parts of the code to see if
they need similar attention. Historically we haven't been very good
about supporting control endpoints other than ep0.
Alan Stern
> drivers/usb/core/message.c | 8 +++++++-
> 1 file changed, 7 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/usb/core/message.c b/drivers/usb/core/message.c
> index 75e2bfd744a9..0cd6dd2b6334 100644
> --- a/drivers/usb/core/message.c
> +++ b/drivers/usb/core/message.c
> @@ -1337,7 +1337,8 @@ static void remove_intf_ep_devs(struct usb_interface *intf)
> *
> * Disables the endpoint for URB submission and nukes all pending URBs.
> * If @reset_hardware is set then also deallocates hcd/hardware state
> - * for the endpoint.
> + * for the endpoint (clearing both ep_in and ep_out pointers for
> + * bidirectional non-ep0 control endpoints).
> */
> void usb_disable_endpoint(struct usb_device *dev, unsigned int epaddr,
> bool reset_hardware)
> @@ -1358,6 +1359,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)
> --
> 2.47.3
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH v2] usb: core: clear both ep_in and ep_out for non-ep0 control endpoints
2026-09-21 0:21 ` Alan Stern
@ 2026-09-21 1:42 ` Hui Peng
0 siblings, 0 replies; 6+ messages in thread
From: Hui Peng @ 2026-09-21 1:42 UTC (permalink / raw)
To: Alan Stern
Cc: Hui Peng, Greg Kroah-Hartman, michal.pecio, linux-usb,
linux-kernel, stable
On Sun, Sep 20, 2026 at 8:21 PM Alan Stern <stern@rowland.harvard.edu> wrote:
>
> Acked-by: Alan Stern <stern@rowland.harvard.edu>
>
> You might consider looking through other parts of the code to see if
> they need similar attention. Historically we haven't been very good
> about supporting control endpoints other than ep0.
>
> Alan Stern
Thank you for the review and Acked-by, Alan.
We will audit the rest of the USB core and host controller paths for
non-ep0 control endpoint handling, and if we find any additional issues,
we will report and submit them in separate patches.
Best regards,
Hui Peng
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-09-21 1:42 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-09-19 22:35 [PATCH] usb: core: fix descriptor parsing OOB, port/bos locking, and devio/ledtrig races Hui Peng
2026-09-20 2:21 ` Alan Stern
2026-09-20 5:12 ` Greg KH
2026-09-20 23:17 ` [PATCH v2] usb: core: clear both ep_in and ep_out for non-ep0 control endpoints Hui Peng
2026-09-21 0:21 ` Alan Stern
2026-09-21 1:42 ` Hui Peng
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox
all inboxes | Powered by JetHome®