Skip to content

Commit eb1b482

Browse files
P33MP33M
P33M
authored and
P33M
committed
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.
1 parent e66eaac commit eb1b482

File tree

3 files changed

+6
-17
lines changed

3 files changed

+6
-17
lines changed

drivers/usb/host/dwc_otg/dwc_otg_hcd.c

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -464,7 +464,6 @@ int dwc_otg_hcd_urb_enqueue(dwc_otg_hcd_t * hcd,
464464
dwc_otg_hcd_urb_t * dwc_otg_urb, void **ep_handle,
465465
int atomic_alloc)
466466
{
467-
dwc_irqflags_t flags;
468467
int retval = 0;
469468
uint8_t needs_scheduling = 0;
470469
dwc_otg_transaction_type_e tr_type;
@@ -515,12 +514,10 @@ int dwc_otg_hcd_urb_enqueue(dwc_otg_hcd_t * hcd,
515514
}
516515

517516
if(needs_scheduling) {
518-
DWC_SPINLOCK_IRQSAVE(hcd->lock, &flags);
519517
tr_type = dwc_otg_hcd_select_transactions(hcd);
520518
if (tr_type != DWC_OTG_TRANSACTION_NONE) {
521519
dwc_otg_hcd_queue_transactions(hcd, tr_type);
522520
}
523-
DWC_SPINUNLOCK_IRQRESTORE(hcd->lock, flags);
524521
}
525522
return retval;
526523
}

drivers/usb/host/dwc_otg/dwc_otg_hcd_linux.c

Lines changed: 5 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -679,9 +679,7 @@ static int dwc_otg_urb_enqueue(struct usb_hcd *hcd,
679679
#if LINUX_VERSION_CODE >= KERNEL_VERSION(2,6,28)
680680
struct usb_host_endpoint *ep = urb->ep;
681681
#endif
682-
#if USB_URB_EP_LINKING
683682
dwc_irqflags_t irqflags;
684-
#endif
685683
void **ref_ep_hcpriv = &ep->hcpriv;
686684
dwc_otg_hcd_t *dwc_otg_hcd = hcd_to_dwc_otg_hcd(hcd);
687685
dwc_otg_hcd_urb_t *dwc_otg_urb;
@@ -733,7 +731,6 @@ static int dwc_otg_urb_enqueue(struct usb_hcd *hcd,
733731
if(dwc_otg_urb == NULL)
734732
return -ENOMEM;
735733

736-
urb->hcpriv = dwc_otg_urb;
737734
if (!dwc_otg_urb && urb->number_of_packets)
738735
return -ENOMEM;
739736

@@ -775,10 +772,10 @@ static int dwc_otg_urb_enqueue(struct usb_hcd *hcd,
775772
iso_frame_desc[i].length);
776773
}
777774

778-
#if USB_URB_EP_LINKING
779775
DWC_SPINLOCK_IRQSAVE(dwc_otg_hcd->lock, &irqflags);
776+
urb->hcpriv = dwc_otg_urb;
777+
#if USB_URB_EP_LINKING
780778
retval = usb_hcd_link_urb_to_ep(hcd, urb);
781-
DWC_SPINUNLOCK_IRQRESTORE(dwc_otg_hcd->lock, irqflags);
782779
if (0 == retval)
783780
#endif
784781
{
@@ -794,17 +791,16 @@ static int dwc_otg_urb_enqueue(struct usb_hcd *hcd,
794791
urb);
795792
}
796793
} else {
797-
#if USB_URB_EP_LINKING
798-
dwc_irqflags_t irqflags;
799794
DWC_DEBUGPL(DBG_HCD, "DWC OTG dwc_otg_hcd_urb_enqueue failed rc %d\n", retval);
800-
DWC_SPINLOCK_IRQSAVE(dwc_otg_hcd->lock, &irqflags);
795+
#if USB_URB_EP_LINKING
801796
usb_hcd_unlink_urb_from_ep(hcd, urb);
802-
DWC_SPINUNLOCK_IRQRESTORE(dwc_otg_hcd->lock, irqflags);
803797
#endif
798+
urb->hcpriv = NULL;
804799
if (retval == -DWC_E_NO_DEVICE)
805800
retval = -ENODEV;
806801
}
807802
}
803+
DWC_SPINUNLOCK_IRQRESTORE(dwc_otg_hcd->lock, irqflags);
808804
return retval;
809805
}
810806

drivers/usb/host/dwc_otg/dwc_otg_hcd_queue.c

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -919,6 +919,7 @@ void dwc_otg_hcd_qtd_init(dwc_otg_qtd_t * qtd, dwc_otg_hcd_urb_t * urb)
919919
* QH to place the QTD into. If it does not find a QH, then it will create a
920920
* new QH. If the QH to which the QTD is added is not currently scheduled, it
921921
* is placed into the proper schedule based on its EP type.
922+
* HCD lock must be held and interrupts must be disabled on entry
922923
*
923924
* @param[in] qtd The QTD to add
924925
* @param[in] hcd The DWC HCD structure
@@ -931,8 +932,6 @@ int dwc_otg_hcd_qtd_add(dwc_otg_qtd_t * qtd,
931932
dwc_otg_hcd_t * hcd, dwc_otg_qh_t ** qh, int atomic_alloc)
932933
{
933934
int retval = 0;
934-
dwc_irqflags_t flags;
935-
936935
dwc_otg_hcd_urb_t *urb = qtd->urb;
937936

938937
/*
@@ -946,15 +945,12 @@ int dwc_otg_hcd_qtd_add(dwc_otg_qtd_t * qtd,
946945
goto done;
947946
}
948947
}
949-
DWC_SPINLOCK_IRQSAVE(hcd->lock, &flags);
950948
retval = dwc_otg_hcd_qh_add(hcd, *qh);
951949
if (retval == 0) {
952950
DWC_CIRCLEQ_INSERT_TAIL(&((*qh)->qtd_list), qtd,
953951
qtd_list_entry);
954952
qtd->qh = *qh;
955953
}
956-
DWC_SPINUNLOCK_IRQRESTORE(hcd->lock, flags);
957-
958954
done:
959955

960956
return retval;

0 commit comments

Comments
 (0)