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@xxxxxxxxx>
> ---

> 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.