dwc_otg: Call usb_hcd_unlink_urb_from_ep with lock held in completion handler

usb_hcd_unlink_urb_from_ep must be called with the HCD lock held.  Calling it
asynchronously in the tasklet was not safe (regression in
c4564d4a1a0a9b10d4419e48239f5d99e88d2667).

This change unlinks it from the endpoint prior to queueing it for handling in
the tasklet, and also adds a check to ensure the urb is OK to be unlinked
before doing so.

NULL pointer dereference kernel oopses had been observed in usb_hcd_giveback_urb
when a USB device was unplugged/replugged during data transfer.  This effect
was reproduced using automated USB port power control, hundreds of replug
events were performed during active transfers to confirm that the problem was
eliminated.

diff --git a/drivers/usb/host/dwc_otg/dwc_otg_hcd.c b/drivers/usb/host/dwc_otg/dwc_otg_hcd.c
index eaa8f384649ac65205e4e5f3b39f3227e0e25c92..9c2e71ac76e011e0d3ebe824497c072e71521dc3 100644 (file)
--- a/drivers/usb/host/dwc_otg/dwc_otg_hcd.c
+++ b/drivers/usb/host/dwc_otg/dwc_otg_hcd.c
@@ -704,6 +704,7 @@ static void completion_tasklet_func(void *ptr)
        urb_tq_entry_t *item;
        dwc_irqflags_t flags;
 
+       /* This could just be spin_lock_irq */
        DWC_SPINLOCK_IRQSAVE(hcd->lock, &flags);
        while (!DWC_TAILQ_EMPTY(&hcd->completed_urb_list)) {
                item = DWC_TAILQ_FIRST(&hcd->completed_urb_list);
@@ -713,7 +714,6 @@ static void completion_tasklet_func(void *ptr)
                DWC_SPINUNLOCK_IRQRESTORE(hcd->lock, flags);
                DWC_FREE(item);
 
-               usb_hcd_unlink_urb_from_ep(hcd->priv, urb);
                usb_hcd_giveback_urb(hcd->priv, urb, urb->status);
 
                DWC_SPINLOCK_IRQSAVE(hcd->lock, &flags);

diff --git a/drivers/usb/host/dwc_otg/dwc_otg_hcd_linux.c b/drivers/usb/host/dwc_otg/dwc_otg_hcd_linux.c
index 0137ce347bec5def6586a7e3867b6cf9289c53f3..dc620f341debf9716ba473dd5697a696c64cb4d3 100644 (file)
--- a/drivers/usb/host/dwc_otg/dwc_otg_hcd_linux.c
+++ b/drivers/usb/host/dwc_otg/dwc_otg_hcd_linux.c
@@ -265,13 +265,15 @@ static void free_bus_bandwidth(struct usb_hcd *hcd, uint32_t bw,
 
 /**
  * Sets the final status of an URB and returns it to the device driver. Any
- * required cleanup of the URB is performed.
+ * required cleanup of the URB is performed.  The HCD lock should be held on
+ * entry.
  */
 static int _complete(dwc_otg_hcd_t * hcd, void *urb_handle,
                     dwc_otg_hcd_urb_t * dwc_otg_urb, int32_t status)
 {
        struct urb *urb = (struct urb *)urb_handle;
        urb_tq_entry_t *new_entry;
+       int rc = 0;
        if (CHK_DEBUG_LEVEL(DBG_HCDV | DBG_HCD_URB)) {
                DWC_PRINTF("%s: urb %p, device %d, ep %d %s, status=%d\n",
                           __func__, urb, usb_pipedevice(urb->pipe),
@@ -363,9 +365,17 @@ static int _complete(dwc_otg_hcd_t * hcd, void *urb_handle,
 #endif
        } else {
                new_entry->urb = urb;
-               DWC_TAILQ_INSERT_TAIL(&hcd->completed_urb_list, new_entry,
-                                       urb_tq_entries);
-               DWC_TASK_HI_SCHEDULE(hcd->completion_tasklet);
+#if USB_URB_EP_LINKING
+               rc = usb_hcd_check_unlink_urb(dwc_otg_hcd_to_hcd(hcd), urb, urb->status);
+               if(0 == rc) {
+                       usb_hcd_unlink_urb_from_ep(dwc_otg_hcd_to_hcd(hcd), urb);
+               }
+#endif
+               if(0 == rc) {
+                       DWC_TAILQ_INSERT_TAIL(&hcd->completed_urb_list, new_entry,
+                                               urb_tq_entries);
+                       DWC_TASK_HI_SCHEDULE(hcd->completion_tasklet);
+               }
        }
        return 0;
 }