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 <sarah.a.sharp@linux.intel.com>
Cc: Andiry Xu <andiry.xu@amd.com>
Cc: stable@kernel.org

diff --git a/drivers/usb/host/xhci-ring.c b/drivers/usb/host/xhci-ring.c
index f72149b666b105e24a98d19c9220f1fe46ee02ae..b2d654b7477eebb7a2c67c680629d20fbc71d8fd 100644 (file)
--- 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 8e84acff113499ceff07b0ca0e80378117a61eae..3a0f695138f4195d4caf988c2fb0266ca7678e5f 100644 (file)
--- 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);