Skip to content

Commit

Permalink
dwc_otg: Call usb_hcd_unlink_urb_from_ep with lock held in completion…
Browse files Browse the repository at this point in the history
… handler

usb_hcd_unlink_urb_from_ep must be called with the lock held.  Not doing so
resulted in corruption of the list, and doing it asynchronously in the tasklet
(see c4564d4) made the problem more likely to
occur.

Also ensure the urb is OK to be unlinked before doing so, and unlink it from
the endpoint prior to queueing it for handling in the tasklet.

NULL pointer deref kernel oopses had been occurring 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.
  • Loading branch information
wmdb committed Jun 18, 2013
1 parent 470f0a7 commit 521f206
Show file tree
Hide file tree
Showing 2 changed files with 18 additions and 4 deletions.
2 changes: 1 addition & 1 deletion drivers/usb/host/dwc_otg/dwc_otg_hcd.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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);
Expand Down
20 changes: 17 additions & 3 deletions drivers/usb/host/dwc_otg/dwc_otg_hcd_linux.c
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,8 @@ static int _complete(dwc_otg_hcd_t * hcd, void *urb_handle,
{
struct urb *urb = (struct urb *)urb_handle;
urb_tq_entry_t *new_entry;
dwc_irqflags_t irqflags;
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),
Expand Down Expand Up @@ -354,7 +356,9 @@ static int _complete(dwc_otg_hcd_t * hcd, void *urb_handle,
/* don't schedule the tasklet -
* directly return the packet here with error. */
#if USB_URB_EP_LINKING
DWC_SPINLOCK_IRQSAVE(hcd->lock, &irqflags);
usb_hcd_unlink_urb_from_ep(dwc_otg_hcd_to_hcd(hcd), urb);
DWC_SPINUNLOCK_IRQRESTORE(hcd->lock, irqflags);
#endif
#if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,28)
usb_hcd_giveback_urb(dwc_otg_hcd_to_hcd(hcd), urb);
Expand All @@ -363,9 +367,19 @@ 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);
DWC_SPINLOCK_IRQSAVE(hcd->lock, &irqflags);
#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);
}
DWC_SPINUNLOCK_IRQRESTORE(hcd->lock, irqflags);
}
return 0;
}
Expand Down

0 comments on commit 521f206

Please sign in to comment.