Re: [PATCH 2/2] xhci: Give back URB under lock in xhci_urb_dequeue()

[Mathias Nyman]

On 11/13/26 06:02, Chien Kun Niu wrote:
> Before commit 36dc01657b49 ("usb: host: xhci: Support running urb
> giveback in tasklet context") set HCD_BH, usb_hcd_giveback_urb() ran
> urb->complete() synchronously, so xHCI had to drop xhci->lock before
> giving back an URB. That commit updated xhci_giveback_urb_in_irq() to
> keep xhci->lock held across usb_hcd_giveback_urb(), covering normal
> completions, xhci_process_cancelled_tds(), and xhci_hc_died(), while
> the open-coded err_giveback path in xhci_urb_dequeue() was missed.
>
> Dropping xhci->lock between usb_hcd_unlink_urb_from_ep() and
> usb_hcd_giveback_urb() breaks per-endpoint giveback serialization: a
> concurrent xhci_urb_dequeue() or xhci_hc_died() can acquire xhci->lock
> and queue a later URB onto bh->head ahead of this one, completing URBs
> out of order. When spinlocks disable interrupts, unlocking first also
> causes a redundant IRQ restore/save bounce into bh->lock, and without
> an IRQ-safe bh->lock it exposed task-context unlinks to a same-CPU
> deadlock.
>
> Move usb_hcd_giveback_urb() before spin_unlock_irqrestore(&xhci->lock,
> flags) in err_giveback to match xhci_giveback_urb_in_irq().
>
> Fixes: 36dc01657b49 ("usb: host: xhci: Support running urb giveback in tasklet context")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Chien Kun Niu <rickyniu@xxxxxxxxxx>
> ---


Added to queue

Thanks
Mathias