From 585df1d90cb07a02ca6c7a7d339e56e46d50dafb Mon Sep 17 00:00:00 2001 From: Sarah Sharp Date: Tue, 2 Aug 2011 15:43:40 -0700 Subject: [PATCH] xhci: Remove TDs from TD lists when URBs are canceled. When a driver tries to cancel an URB, and the host controller is dying, xhci_urb_dequeue will giveback the URB without removing the xhci_tds that comprise that URB from the td_list or the cancelled_td_list. This can cause a race condition between the driver calling URB dequeue and the stop endpoint command watchdog timer. If the timer fires on a dying host, and a driver attempts to resubmit while the watchdog timer has dropped the xhci->lock to giveback a cancelled URB, URBs may be given back by the xhci_urb_dequeue() function. At that point, the URB's priv pointer will be freed and set to NULL, but the TDs will remain on the td_list. This will cause an oops in xhci_giveback_urb_in_irq() when the watchdog timer attempts to loop through the endpoints' td_lists, giving back killed URBs. Make sure that xhci_urb_dequeue() removes TDs from the TD lists and canceled TD lists before it gives back the URB. This patch should be backported to kernels as old as 2.6.36. Signed-off-by: Sarah Sharp Cc: Andiry Xu Cc: stable@kernel.org --- drivers/usb/host/xhci-ring.c | 16 ++++++++-------- drivers/usb/host/xhci.c | 7 +++++++ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c index f72149b666b1..b2d654b7477e 100644 --- a/drivers/usb/host/xhci-ring.c +++ b/drivers/usb/host/xhci-ring.c @@ -741,7 +741,7 @@ remove_finished_td: * so remove it from the endpoint ring's TD list. Keep it in * the cancelled TD list for URB completion later. */ - list_del(&cur_td->td_list); + list_del_init(&cur_td->td_list); } last_unlinked_td = cur_td; xhci_stop_watchdog_timer_in_irq(xhci, ep); @@ -769,7 +769,7 @@ remove_finished_td: do { cur_td = list_entry(ep->cancelled_td_list.next, struct xhci_td, cancelled_td_list); - list_del(&cur_td->cancelled_td_list); + list_del_init(&cur_td->cancelled_td_list); /* Clean up the cancelled URB */ /* Doesn't matter what we pass for status, since the core will @@ -877,9 +877,9 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) cur_td = list_first_entry(&ring->td_list, struct xhci_td, td_list); - list_del(&cur_td->td_list); + list_del_init(&cur_td->td_list); if (!list_empty(&cur_td->cancelled_td_list)) - list_del(&cur_td->cancelled_td_list); + list_del_init(&cur_td->cancelled_td_list); xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN, "killed"); } @@ -888,7 +888,7 @@ void xhci_stop_endpoint_command_watchdog(unsigned long arg) &temp_ep->cancelled_td_list, struct xhci_td, cancelled_td_list); - list_del(&cur_td->cancelled_td_list); + list_del_init(&cur_td->cancelled_td_list); xhci_giveback_urb_in_irq(xhci, cur_td, -ESHUTDOWN, "killed"); } @@ -1580,10 +1580,10 @@ td_cleanup: else *status = 0; } - list_del(&td->td_list); + list_del_init(&td->td_list); /* Was this TD slated to be cancelled but completed anyway? */ if (!list_empty(&td->cancelled_td_list)) - list_del(&td->cancelled_td_list); + list_del_init(&td->cancelled_td_list); urb_priv->td_cnt++; /* Giveback the urb when all the tds are completed */ @@ -3362,7 +3362,7 @@ cleanup: /* Clean up a partially enqueued isoc transfer. */ for (i--; i >= 0; i--) - list_del(&urb_priv->td[i]->td_list); + list_del_init(&urb_priv->td[i]->td_list); /* Use the first TD as a temporary variable to turn the TDs we've queued * into No-ops with a software-owned cycle bit. That way the hardware diff --git a/drivers/usb/host/xhci.c b/drivers/usb/host/xhci.c index 8e84acff1134..3a0f695138f4 100644 --- a/drivers/usb/host/xhci.c +++ b/drivers/usb/host/xhci.c @@ -1252,6 +1252,13 @@ int xhci_urb_dequeue(struct usb_hcd *hcd, struct urb *urb, int status) if (temp == 0xffffffff || (xhci->xhc_state & XHCI_STATE_HALTED)) { xhci_dbg(xhci, "HW died, freeing TD.\n"); urb_priv = urb->hcpriv; + for (i = urb_priv->td_cnt; i < urb_priv->length; i++) { + td = urb_priv->td[i]; + if (!list_empty(&td->td_list)) + list_del_init(&td->td_list); + if (!list_empty(&td->cancelled_td_list)) + list_del_init(&td->cancelled_td_list); + } usb_hcd_unlink_urb_from_ep(hcd, urb); spin_unlock_irqrestore(&xhci->lock, flags); -- 2.39.5