[PATCH 0/3] USB core defense "hardening" changes

[PATCH 0/3] USB core defense "hardening" changes

[Griffin Kroah-Hartman]

A small collection of USB patches to protect against broken usb
descriptors and a fix for a potential race condition in the hub driver.

Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
---
Griffin Kroah-Hartman (3):
      usb: core: Add size check to find_next_descriptor()
      usb: core: strengthen size check in usb_parse_interface()
      usb: core: Add lock to usb_wakeup_notification()

 drivers/usb/core/config.c | 4 +++-
 drivers/usb/core/hub.c    | 3 +++
 2 files changed, 6 insertions(+), 1 deletion(-)
---
base-commit: a13c140cc289c0b7b3770bce5b3ad42ab35074aa
change-id: 20260713-usb_core_patches_1-84e2fb7b5bce

Best regards,
--  
Griffin Kroah-Hartman <griffin@kroah.com>

[PATCH 1/3] usb: core: Add size check to find_next_descriptor()

[Griffin Kroah-Hartman]

Add a size check for the descriptor header in find_next_descriptor().
This prevents a scenario where h->blength = 0, causing the while loop to
spin forever.

Assisted-by: gkh_clanker_t1000
Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
---
 drivers/usb/core/config.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 45e20c6d76c0..17c93e95945f 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -29,6 +29,8 @@ static int find_next_descriptor(unsigned char *buffer, int size,
 	/* Find the next descriptor of type dt1 or dt2 */
 	while (size > 0) {
 		h = (struct usb_descriptor_header *) buffer;
+		if (h->bLength < sizeof(struct usb_descriptor_header))
+			break;
 		if (h->bDescriptorType == dt1 || h->bDescriptorType == dt2)
 			break;
 		buffer += h->bLength;

-- 
2.55.0

Re: [PATCH 1/3] usb: core: Add size check to find_next_descriptor()

[Alan Stern]

On Mon, Jul 13, 2026 at 05:43:51PM +0200, Griffin Kroah-Hartman wrote:
> Add a size check for the descriptor header in find_next_descriptor().
> This prevents a scenario where h->blength = 0, causing the while loop to
> spin forever.

Just as before, this check has already been performed by 
usb_parse_configuration().  It doesn't need to be performed again.

Alan Stern

> Assisted-by: gkh_clanker_t1000
> Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
> ---
>  drivers/usb/core/config.c | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 45e20c6d76c0..17c93e95945f 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -29,6 +29,8 @@ static int find_next_descriptor(unsigned char *buffer, int size,
>  	/* Find the next descriptor of type dt1 or dt2 */
>  	while (size > 0) {
>  		h = (struct usb_descriptor_header *) buffer;
> +		if (h->bLength < sizeof(struct usb_descriptor_header))
> +			break;
>  		if (h->bDescriptorType == dt1 || h->bDescriptorType == dt2)
>  			break;
>  		buffer += h->bLength;
> 
> -- 
> 2.55.0
> 
> 

[PATCH 2/3] usb: core: strengthen size check in usb_parse_interface()

[Griffin Kroah-Hartman]

Change the size check from size > 0 to size >= sizeof(struct
usb_descriptor_header) in usb_parse_interface().  This prevents a
malicious device from utilizing trailing bytes to incur an OOB read.

This matches the size check in usb_parse_configuration() as well.

Assisted-by: gkh_clanker_t1000
Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
---
 drivers/usb/core/config.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
index 17c93e95945f..91a51cedc1b8 100644
--- a/drivers/usb/core/config.c
+++ b/drivers/usb/core/config.c
@@ -623,7 +623,7 @@ static int usb_parse_interface(struct device *ddev, int cfgno,
 
 	/* Parse all the endpoint descriptors */
 	n = 0;
-	while (size > 0) {
+	while (size >= sizeof(struct usb_descriptor_header)) {
 		if (((struct usb_descriptor_header *) buffer)->bDescriptorType
 		     == USB_DT_INTERFACE)
 			break;

-- 
2.55.0

Re: [PATCH 2/3] usb: core: strengthen size check in usb_parse_interface()

[Alan Stern]

On Mon, Jul 13, 2026 at 05:43:52PM +0200, Griffin Kroah-Hartman wrote:
> Change the size check from size > 0 to size >= sizeof(struct
> usb_descriptor_header) in usb_parse_interface().  This prevents a
> malicious device from utilizing trailing bytes to incur an OOB read.

No, because usb_parse_configuration() has already performed this check.  
A malicious device can't avoid that earlier check, and so we don't need 
to do it again.

> This matches the size check in usb_parse_configuration() as well.

Right.  There's no point in doing the same size check twice.

Alan Stern

> 
> Assisted-by: gkh_clanker_t1000
> Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
> ---
>  drivers/usb/core/config.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/core/config.c b/drivers/usb/core/config.c
> index 17c93e95945f..91a51cedc1b8 100644
> --- a/drivers/usb/core/config.c
> +++ b/drivers/usb/core/config.c
> @@ -623,7 +623,7 @@ static int usb_parse_interface(struct device *ddev, int cfgno,
>  
>  	/* Parse all the endpoint descriptors */
>  	n = 0;
> -	while (size > 0) {
> +	while (size >= sizeof(struct usb_descriptor_header)) {
>  		if (((struct usb_descriptor_header *) buffer)->bDescriptorType
>  		     == USB_DT_INTERFACE)
>  			break;
> 
> -- 
> 2.55.0
> 
> 

Re: [PATCH 2/3] usb: core: strengthen size check in usb_parse_interface()

[Greg Kroah-Hartman]

On Mon, Jul 13, 2026 at 12:13:04PM -0400, Alan Stern wrote:
> On Mon, Jul 13, 2026 at 05:43:52PM +0200, Griffin Kroah-Hartman wrote:
> > Change the size check from size > 0 to size >= sizeof(struct
> > usb_descriptor_header) in usb_parse_interface().  This prevents a
> > malicious device from utilizing trailing bytes to incur an OOB read.
> 
> No, because usb_parse_configuration() has already performed this check.  
> A malicious device can't avoid that earlier check, and so we don't need 
> to do it again.

Ok, this is my fault.  I gave these tasks to the author here, as part of
an project for computer security masters students at vu.nl to help grind
down the huge number of LLM-reported issues that I've gotten over the
past few months (you'll see other patches from these students being
submitted all over the tree right now as well).  Right now the LLMs seem
to be running at about 1/3 to 1/4 of "false positives", and these two
patches are a good example of that.

The thread here:
	https://lore.kernel.org/all/alEsSl8i1_FpoU0f@fudgebox/
is also another example of where the LLM wanted to add more defensive
code, despite it not really being needed.

The bots really really want to please you, and come up with ways to
swear that the change is needed, and so manual code review is still
always required.

Even with that manual review, these two fooled me, as they are obviously
not wrong (what's the harm in adding more checks?)  I caught that we
parsed the descriptors in usb_parse_configuration() but missed where
size was not incremented (I was looking at the buffer pointers).
Sashiko also missed this (catching it for the first patch, but not for
this one), proof that even the "best" LLM models out there today are
still not reliable.

Thanks for the review, much appreciated.

As for patch 3/3, I'm pretty sure that can't hurt as well.  I'm starting
to get a lot of "something could happen in an IRQ while a device is
being removed" bug reports from these tools.  Looks like they all just
discovered that very rare code path at the same time, something that
syzbot can't really fuzz for.

thanks,

greg k-h

Re: [PATCH 2/3] usb: core: strengthen size check in usb_parse_interface()

[Oliver Neukum]

On 14.07.26 07:53, Greg Kroah-Hartman wrote:
> On Mon, Jul 13, 2026 at 12:13:04PM -0400, Alan Stern wrote:

>> No, because usb_parse_configuration() has already performed this check.
>> A malicious device can't avoid that earlier check, and so we don't need
>> to do it again.
> 
> Ok, this is my fault.
Speaking about fault here isn't likely to be productive.
The error is easy to make. I made it once. We have the power
of using comments. That place needs one.

	Regards
		Oliver

Re: [PATCH 2/3] usb: core: strengthen size check in usb_parse_interface()

[Greg Kroah-Hartman]

On Tue, Jul 14, 2026 at 08:20:32AM +0200, Oliver Neukum wrote:
> 
> 
> On 14.07.26 07:53, Greg Kroah-Hartman wrote:
> > On Mon, Jul 13, 2026 at 12:13:04PM -0400, Alan Stern wrote:
> 
> > > No, because usb_parse_configuration() has already performed this check.
> > > A malicious device can't avoid that earlier check, and so we don't need
> > > to do it again.
> > 
> > Ok, this is my fault.
> Speaking about fault here isn't likely to be productive.

But it is cathartic :)

> The error is easy to make. I made it once. We have the power
> of using comments. That place needs one.

Sure, which place, usb_parse_configuration() or the other parse
locations or both?

thanks,

greg k-h

Re: [PATCH 2/3] usb: core: strengthen size check in usb_parse_interface()

[Oliver Neukum]

On 14.07.26 08:30, Greg Kroah-Hartman wrote:
> On Tue, Jul 14, 2026 at 08:20:32AM +0200, Oliver Neukum wrote:

>> The error is easy to make. I made it once. We have the power
>> of using comments. That place needs one.
> 
> Sure, which place, usb_parse_configuration() or the other parse
> locations or both?

We are not taxed for comments.
If you make a patch adding comments, really go for it.
Both places.

	Regards
		Oliver

Re: [PATCH 2/3] usb: core: strengthen size check in usb_parse_interface()

[Alan Stern]

On Tue, Jul 14, 2026 at 07:53:14AM +0200, Greg Kroah-Hartman wrote:
> As for patch 3/3, I'm pretty sure that can't hurt as well.  I'm starting
> to get a lot of "something could happen in an IRQ while a device is
> being removed" bug reports from these tools.  Looks like they all just
> discovered that very rare code path at the same time, something that
> syzbot can't really fuzz for.

The 3/3 patch looked okay to me too, but I wasn't entirely sure because 
it does a bunch of stuff while holding the device_state_lock.  It seemed 
better to let the xHCI people comment on it, because it's in a code path 
that applies only to USB-3 devices.

Alan Stern

[PATCH 3/3] usb: core: Add lock to usb_wakeup_notification()

[Griffin Kroah-Hartman]

Add a spin lock to usb_wakeup notification to prevent a race condition
with dereferencing freed memory. This could be hit by the xHCI driver as
it calls this function from an IRQ and could race with the
hub_disconnect() function, which properly grabs this lock to protect the
state of the device.

Assisted-by: gkh_clanker_t1000
Signed-off-by: Griffin Kroah-Hartman <griffin@kroah.com>
---
 drivers/usb/core/hub.c | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/drivers/usb/core/hub.c b/drivers/usb/core/hub.c
index 5262e11c12cd..5798efdd91a9 100644
--- a/drivers/usb/core/hub.c
+++ b/drivers/usb/core/hub.c
@@ -753,10 +753,12 @@ void usb_wakeup_notification(struct usb_device *hdev,
 {
 	struct usb_hub *hub;
 	struct usb_port *port_dev;
+	unsigned long flags;
 
 	if (!hdev)
 		return;
 
+	spin_lock_irqsave(&device_state_lock, flags);
 	hub = usb_hub_to_struct_hub(hdev);
 	if (hub) {
 		port_dev = hub->ports[portnum - 1];
@@ -766,6 +768,7 @@ void usb_wakeup_notification(struct usb_device *hdev,
 		set_bit(portnum, hub->wakeup_bits);
 		kick_hub_wq(hub);
 	}
+	spin_unlock_irqrestore(&device_state_lock, flags);
 }
 EXPORT_SYMBOL_GPL(usb_wakeup_notification);
 

-- 
2.55.0