Re: [PATCH v5] usb: xhci: Fix use-after-free race in xhci_stop_device()

[Alan Stern]

On Wed, Nov 04, 2026 at 05:59:49PM +0100, Michal Pecio wrote:
> On Wed, 4 Nov 2026 10:02:48 -0500, Alan Stern wrote:
> > On Wed, Nov 04, 2026 at 11:56:37AM +0800, YuanZhe Liu wrote:
> > > If you think the mutual exclusion belongs higher up instead - do not
> > > let usbfs suspend a port while the hub is being unbound - I am happy to
> > > work on that; from your message it sounds like you would prefer the fix
> > > there.  
> > 
> > Don't know about Michal, but _I_ think the problem should be solved 
> > higher up in the stack.
> 
> Perhaps in userspace? Or is it expected that root hubs (or other hubs)
> receive random and conflicting requests through USBFS?

In usbfs, as I just wrote to YuanZhe.  It should not be allowed to 
send random requests to hubs while they are running.

> Do we know that the device whose port is being suspended has been
> quiesced? Is its driver even aware of anything?
> 
> What I find more disturbing is the existing protection from this race
> added by 88716a93766b - it looks like core was triggering this too.

I don't know anything about it.  That commit only touches xhci-hub.c, 
not usbcore.

> I found this usb_disconnect() code suspicious:
> 
>         if (port_dev && test_and_clear_bit(port1, hub->child_usage_bits))
>                 pm_runtime_put(&port_dev->dev);
> 
>         msleep(150);		// my addition
> 
>         hub_free_dev(udev);	// this drops xHCI device state
> 
>         put_device(&udev->dev);

I don't see how this is related to the problem at hand.  The 
pm_runtime_put() above merely undoes a pm_runtime_get_sync() earlier in 
the same routine.

> but it seems that even with the sleep, the bus only gets suspended
> after put_device(). So IDK how that panic was possible in 2016.
> 
> > It sounds like the sort of race that could  affect any HCD, not just
> > xhci-hcd.
> 
> xhci-hcd is affected because it tracks association between devices
> and ports and accesses devices on port suspend. IDK if others do.
> 
> xhci-hcd shouldn't even need to do this, becaues it's pointless if
> devices behind the port have already had their URBs unlinked. Case in
> point, nobody noticed it's only done to the immediate child :)
> 
> I would have sent a patch to nuke xhci_stop_device() altogether, if
> there weren't known cases where core leaves URBs on suspended ports.
> That stuff should probably be fixed, it caused problems before.

There should never be any outstanding URBs on suspended ports.  If you 
have any examples, they need to be fixed.

> I recall the main blocker being that core wants to setup wakeups in
> the same call which puts ports to suspend, so it cannot reject ep0
> URBs before doing this call.

The core relies on drivers to unlink outstanding URBs and not submit any 
new ones after they have been told that their devices are going into 
suspend.  I suppose we could check that there are no errant URBs still 
in progress just before putting the port into suspend, but is this 
really necessary?

Alan Stern