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

[Michal Pecio <michal.pecio@gmail.com>]

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:
> > The port suspend is a hub class SET_FEATURE(PORT_SUSPEND) sent from
> > userspace through usbfs, and the teardown is userspace releasing the
> > hub driver interface, which quiesces the hub and disconnects every
> > child under it.  Neither side takes a per-device reference, and the
> > usbfs control request is not serialized against the hub driver being
> > unbound.  That is also why xhci_bus_suspend() 100 ms after a disconnect
> > looks fine: this is a different entry into xhci_stop_device().
> > 
> > CLEAR_FEATURE(PORT_SUSPEND) is the resume direction: in
> > xhci_hub_control() it ends in xhci_ring_device(), so it cannot produce
> > this trace.  
> 
> > The patch makes xhci_stop_device() re-read xhci->devs[] and the "device
> > is going away" marker under xhci->lock, and give up with -ENODEV
> > instead of touching a device that xhci_free_dev() has already detached.
> > For that check to mean anything, xhci_free_dev() has to clear
> > virt_dev->udev under the same lock, which is the second hunk.
> > 
> > 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?

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 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);

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.

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.

Regards,
Michal