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

[YuanZhe Liu]

Alan,

Thanks - that is a fair point, and I agree that the generic exposure is in
the core: usbfs can drive port state on a hub whose driver is being
unbound, and usb core does not serialize the two.

The awkward part is where the lock would go.  do_proc_control() releases
usb_lock_device() around usbfs_start_wait_urb(), so the control request to
the hub runs without the device lock, while usb_unbind_interface() ->
hub_disconnect() holds it.  Making those two exclude each other on that
mutex would mean holding it across a synchronous URB, which I do not think
is acceptable.

Two shapes I can see on the core side:

 - refuse or queue hub class requests to a hub while its driver is being
   unbound, or
 - make usb_disconnect() of that hub's children wait for in-flight
   port-state requests on that hub first.

I am happy to prototype either one; tell me which you would prefer.

What is xhci-specific is that xhci_hub_control() is the only one of these
paths that walks per-device state: xhci_stop_device() looks up
xhci->devs[slot_id] and drops xhci->lock while sending the stop endpoint
commands.  ehci/ohci/uhci only write port registers there, so the same
interleaving does not give them a use-after-free.

So I would like to keep the xhci hunk as well, as memory-safety hardening
for the case where the core does not catch it.  If you would rather see
the core change first, I can send that separately and rebase this on top
of it.

YuanZhe