dwc_otg: prevent OOPSes during device disconnects

The dwc_otg_urb_enqueue function is thread-unsafe. In particular the
access of urb->hcpriv, usb_hcd_link_urb_to_ep, dwc_otg_urb->qtd and
friends does not occur within a critical section and so if a device
was unplugged during activity there was a high chance that the
usbcore hub_thread would try to disable the endpoint with partially-
formed entries in the URB queue. This would result in BUG() or null
pointer dereferences.

Fix so that access of urb->hcpriv, enqueuing to the hardware and
adding to usbcore endpoint URB lists is contained within a single
critical section.
This commit is contained in:
P33M
2013-07-18 17:07:26 +01:00
parent e66eaacae2
commit eb1b482a93
3 changed files with 6 additions and 17 deletions

View File

@@ -464,7 +464,6 @@ int dwc_otg_hcd_urb_enqueue(dwc_otg_hcd_t * hcd,
dwc_otg_hcd_urb_t * dwc_otg_urb, void **ep_handle, dwc_otg_hcd_urb_t * dwc_otg_urb, void **ep_handle,
int atomic_alloc) int atomic_alloc)
{ {
dwc_irqflags_t flags;
int retval = 0; int retval = 0;
uint8_t needs_scheduling = 0; uint8_t needs_scheduling = 0;
dwc_otg_transaction_type_e tr_type; dwc_otg_transaction_type_e tr_type;
@@ -515,12 +514,10 @@ int dwc_otg_hcd_urb_enqueue(dwc_otg_hcd_t * hcd,
} }
if(needs_scheduling) { if(needs_scheduling) {
DWC_SPINLOCK_IRQSAVE(hcd->lock, &flags);
tr_type = dwc_otg_hcd_select_transactions(hcd); tr_type = dwc_otg_hcd_select_transactions(hcd);
if (tr_type != DWC_OTG_TRANSACTION_NONE) { if (tr_type != DWC_OTG_TRANSACTION_NONE) {
dwc_otg_hcd_queue_transactions(hcd, tr_type); dwc_otg_hcd_queue_transactions(hcd, tr_type);
} }
DWC_SPINUNLOCK_IRQRESTORE(hcd->lock, flags);
} }
return retval; return retval;
} }

View File

@@ -679,9 +679,7 @@ static int dwc_otg_urb_enqueue(struct usb_hcd *hcd,
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,28) #if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,28)
struct usb_host_endpoint *ep = urb->ep; struct usb_host_endpoint *ep = urb->ep;
#endif #endif
#if USB_URB_EP_LINKING
dwc_irqflags_t irqflags; dwc_irqflags_t irqflags;
#endif
void **ref_ep_hcpriv = &ep->hcpriv; void **ref_ep_hcpriv = &ep->hcpriv;
dwc_otg_hcd_t *dwc_otg_hcd = hcd_to_dwc_otg_hcd(hcd); dwc_otg_hcd_t *dwc_otg_hcd = hcd_to_dwc_otg_hcd(hcd);
dwc_otg_hcd_urb_t *dwc_otg_urb; dwc_otg_hcd_urb_t *dwc_otg_urb;
@@ -733,7 +731,6 @@ static int dwc_otg_urb_enqueue(struct usb_hcd *hcd,
if(dwc_otg_urb == NULL) if(dwc_otg_urb == NULL)
return -ENOMEM; return -ENOMEM;
urb->hcpriv = dwc_otg_urb;
if (!dwc_otg_urb && urb->number_of_packets) if (!dwc_otg_urb && urb->number_of_packets)
return -ENOMEM; return -ENOMEM;
@@ -775,10 +772,10 @@ static int dwc_otg_urb_enqueue(struct usb_hcd *hcd,
iso_frame_desc[i].length); iso_frame_desc[i].length);
} }
#if USB_URB_EP_LINKING
DWC_SPINLOCK_IRQSAVE(dwc_otg_hcd->lock, &irqflags); DWC_SPINLOCK_IRQSAVE(dwc_otg_hcd->lock, &irqflags);
urb->hcpriv = dwc_otg_urb;
#if USB_URB_EP_LINKING
retval = usb_hcd_link_urb_to_ep(hcd, urb); retval = usb_hcd_link_urb_to_ep(hcd, urb);
DWC_SPINUNLOCK_IRQRESTORE(dwc_otg_hcd->lock, irqflags);
if (0 == retval) if (0 == retval)
#endif #endif
{ {
@@ -794,17 +791,16 @@ static int dwc_otg_urb_enqueue(struct usb_hcd *hcd,
urb); urb);
} }
} else { } else {
#if USB_URB_EP_LINKING
dwc_irqflags_t irqflags;
DWC_DEBUGPL(DBG_HCD, "DWC OTG dwc_otg_hcd_urb_enqueue failed rc %d\n", retval); DWC_DEBUGPL(DBG_HCD, "DWC OTG dwc_otg_hcd_urb_enqueue failed rc %d\n", retval);
DWC_SPINLOCK_IRQSAVE(dwc_otg_hcd->lock, &irqflags); #if USB_URB_EP_LINKING
usb_hcd_unlink_urb_from_ep(hcd, urb); usb_hcd_unlink_urb_from_ep(hcd, urb);
DWC_SPINUNLOCK_IRQRESTORE(dwc_otg_hcd->lock, irqflags);
#endif #endif
urb->hcpriv = NULL;
if (retval == -DWC_E_NO_DEVICE) if (retval == -DWC_E_NO_DEVICE)
retval = -ENODEV; retval = -ENODEV;
} }
} }
DWC_SPINUNLOCK_IRQRESTORE(dwc_otg_hcd->lock, irqflags);
return retval; return retval;
} }

View File

@@ -919,6 +919,7 @@ void dwc_otg_hcd_qtd_init(dwc_otg_qtd_t * qtd, dwc_otg_hcd_urb_t * urb)
* QH to place the QTD into. If it does not find a QH, then it will create a * QH to place the QTD into. If it does not find a QH, then it will create a
* new QH. If the QH to which the QTD is added is not currently scheduled, it * new QH. If the QH to which the QTD is added is not currently scheduled, it
* is placed into the proper schedule based on its EP type. * is placed into the proper schedule based on its EP type.
* HCD lock must be held and interrupts must be disabled on entry
* *
* @param[in] qtd The QTD to add * @param[in] qtd The QTD to add
* @param[in] hcd The DWC HCD structure * @param[in] hcd The DWC HCD structure
@@ -931,8 +932,6 @@ int dwc_otg_hcd_qtd_add(dwc_otg_qtd_t * qtd,
dwc_otg_hcd_t * hcd, dwc_otg_qh_t ** qh, int atomic_alloc) dwc_otg_hcd_t * hcd, dwc_otg_qh_t ** qh, int atomic_alloc)
{ {
int retval = 0; int retval = 0;
dwc_irqflags_t flags;
dwc_otg_hcd_urb_t *urb = qtd->urb; dwc_otg_hcd_urb_t *urb = qtd->urb;
/* /*
@@ -946,15 +945,12 @@ int dwc_otg_hcd_qtd_add(dwc_otg_qtd_t * qtd,
goto done; goto done;
} }
} }
DWC_SPINLOCK_IRQSAVE(hcd->lock, &flags);
retval = dwc_otg_hcd_qh_add(hcd, *qh); retval = dwc_otg_hcd_qh_add(hcd, *qh);
if (retval == 0) { if (retval == 0) {
DWC_CIRCLEQ_INSERT_TAIL(&((*qh)->qtd_list), qtd, DWC_CIRCLEQ_INSERT_TAIL(&((*qh)->qtd_list), qtd,
qtd_list_entry); qtd_list_entry);
qtd->qh = *qh; qtd->qh = *qh;
} }
DWC_SPINUNLOCK_IRQRESTORE(hcd->lock, flags);
done: done:
return retval; return retval;