Skip to content

Commit

Permalink
dwc_otg: Fix unsafe access of QTD during URB enqueue
Browse files Browse the repository at this point in the history
In dwc_otg_hcd_urb_enqueue during qtd creation, it was possible that the
transaction could complete almost immediately after the qtd was assigned
to a host channel during URB enqueue, which meant the qtd pointer was no
longer valid having been completed and removed. Usually, this resulted in
an OOPS during URB submission. By predetermining whether transactions
need to be queued or not, this unsafe pointer access is avoided.

This bug was only evident on the Pi model A where a device was attached
that had no periodic endpoints (e.g. USB pendrive or some wlan devices).
  • Loading branch information
P33M authored and P33M committed Feb 15, 2013
1 parent 871eef1 commit 3e964c4
Show file tree
Hide file tree
Showing 2 changed files with 13 additions and 12 deletions.
23 changes: 12 additions & 11 deletions drivers/usb/host/dwc_otg/dwc_otg_hcd.c
Original file line number Diff line number Diff line change
Expand Up @@ -462,6 +462,8 @@ int dwc_otg_hcd_urb_enqueue(dwc_otg_hcd_t * hcd,
{
dwc_irqflags_t flags;
int retval = 0;
uint8_t needs_scheduling = 0;
dwc_otg_transaction_type_e tr_type;
dwc_otg_qtd_t *qtd;
gintmsk_data_t intr_mask = {.d32 = 0 };

Expand Down Expand Up @@ -493,30 +495,29 @@ int dwc_otg_hcd_urb_enqueue(dwc_otg_hcd_t * hcd,
return -DWC_E_NO_MEMORY;
}
#endif
retval =
dwc_otg_hcd_qtd_add(qtd, hcd, (dwc_otg_qh_t **) ep_handle, atomic_alloc);
intr_mask.d32 = DWC_READ_REG32(&hcd->core_if->core_global_regs->gintmsk);
if(!intr_mask.b.sofintr) needs_scheduling = 1;
if((((dwc_otg_qh_t *)ep_handle)->ep_type == UE_BULK) && !(qtd->urb->flags & URB_GIVEBACK_ASAP))

This comment has been minimized.

Copy link
@popcornmix

popcornmix Feb 16, 2013

Why is the first half of expression using ((dwc_otg_qh_t *)ep_handle)->ep_type rather than qtd->qh->ep_type ?

This comment has been minimized.

Copy link
@P33M

P33M Feb 16, 2013

Owner

Because in raspberrypi#190 I moved the assigment of the qtd->qh pointer inside the spinlock in the call to dwc_otg_qtd_add to fix the memory corruption bug. As the ep_handle is passed as **void it needs a typecast to a dwc_otg_qh_t type to get the member correctly.

/* Do not schedule SG transactions until qtd has URB_GIVEBACK_ASAP set */
needs_scheduling = 0;

retval = dwc_otg_hcd_qtd_add(qtd, hcd, (dwc_otg_qh_t **) ep_handle, atomic_alloc);
// creates a new queue in ep_handle if it doesn't exist already
if (retval < 0) {
DWC_ERROR("DWC OTG HCD URB Enqueue failed adding QTD. "
"Error status %d\n", retval);
dwc_otg_hcd_qtd_free(qtd);
return retval;
}
intr_mask.d32 = DWC_READ_REG32(&hcd->core_if->core_global_regs->gintmsk);
if (!intr_mask.b.sofintr && retval == 0) {
dwc_otg_transaction_type_e tr_type;
if ((qtd->qh->ep_type == UE_BULK)
&& !(qtd->urb->flags & URB_GIVEBACK_ASAP)) {
/* Do not schedule SG transactions until qtd has URB_GIVEBACK_ASAP set */
return 0;
}

if(needs_scheduling) {
DWC_SPINLOCK_IRQSAVE(hcd->lock, &flags);
tr_type = dwc_otg_hcd_select_transactions(hcd);
if (tr_type != DWC_OTG_TRANSACTION_NONE) {
dwc_otg_hcd_queue_transactions(hcd, tr_type);
}
DWC_SPINUNLOCK_IRQRESTORE(hcd->lock, flags);
}

return retval;
}

Expand Down
2 changes: 1 addition & 1 deletion drivers/usb/host/dwc_otg/dwc_otg_hcd_queue.c
Original file line number Diff line number Diff line change
Expand Up @@ -937,7 +937,7 @@ int dwc_otg_hcd_qtd_add(dwc_otg_qtd_t * qtd,
if (*qh == NULL) {
*qh = dwc_otg_hcd_qh_create(hcd, urb, atomic_alloc);
if (*qh == NULL) {
retval = -1;
retval = -DWC_E_NO_MEMORY;
goto done;
}
}
Expand Down

0 comments on commit 3e964c4

Please sign in to comment.