From 33ad261aa62be02f0cedeb4d5735cc726de84a3f Mon Sep 17 00:00:00 2001 From: Gregory Herrero Date: Wed, 29 Apr 2015 22:09:15 +0200 Subject: [PATCH] usb: dwc2: host: spinlock urb_enqueue During urb_enqueue, if the urb can't be queued to the endpoint, the urb is freed without any spinlock protection. This leads to memory corruption when concurrent urb_dequeue try to free same urb->hcpriv. Thus, ensure the whole urb_enqueue in spinlocked. Acked-by: John Youn Signed-off-by: Gregory Herrero Signed-off-by: Felipe Balbi --- drivers/usb/dwc2/hcd.c | 15 ++++++--------- drivers/usb/dwc2/hcd_queue.c | 8 +------- 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c index 780f298..d72557c 100644 --- a/drivers/usb/dwc2/hcd.c +++ b/drivers/usb/dwc2/hcd.c @@ -357,12 +357,12 @@ void dwc2_hcd_stop(struct dwc2_hsotg *hsotg) writel(0, hsotg->regs + HPRT0); } +/* Caller must hold driver lock */ static int dwc2_hcd_urb_enqueue(struct dwc2_hsotg *hsotg, struct dwc2_hcd_urb *urb, void **ep_handle, gfp_t mem_flags) { struct dwc2_qtd *qtd; - unsigned long flags; u32 intr_mask; int retval; int dev_speed; @@ -413,11 +413,9 @@ static int dwc2_hcd_urb_enqueue(struct dwc2_hsotg *hsotg, */ return 0; - spin_lock_irqsave(&hsotg->lock, flags); tr_type = dwc2_hcd_select_transactions(hsotg); if (tr_type != DWC2_TRANSACTION_NONE) dwc2_hcd_queue_transactions(hsotg, tr_type); - spin_unlock_irqrestore(&hsotg->lock, flags); } return 0; @@ -2484,7 +2482,7 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, "%s: unaligned transfer with no transfer_buffer", __func__); retval = -EINVAL; - goto fail1; + goto fail0; } } @@ -2512,7 +2510,6 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, spin_lock_irqsave(&hsotg->lock, flags); retval = usb_hcd_link_urb_to_ep(hcd, urb); - spin_unlock_irqrestore(&hsotg->lock, flags); if (retval) goto fail1; @@ -2521,22 +2518,22 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb, goto fail2; if (alloc_bandwidth) { - spin_lock_irqsave(&hsotg->lock, flags); dwc2_allocate_bus_bandwidth(hcd, dwc2_hcd_get_ep_bandwidth(hsotg, ep), urb); - spin_unlock_irqrestore(&hsotg->lock, flags); } + spin_unlock_irqrestore(&hsotg->lock, flags); + return 0; fail2: - spin_lock_irqsave(&hsotg->lock, flags); dwc2_urb->priv = NULL; usb_hcd_unlink_urb_from_ep(hcd, urb); - spin_unlock_irqrestore(&hsotg->lock, flags); fail1: + spin_unlock_irqrestore(&hsotg->lock, flags); urb->hcpriv = NULL; +fail0: kfree(dwc2_urb); return retval; diff --git a/drivers/usb/dwc2/hcd_queue.c b/drivers/usb/dwc2/hcd_queue.c index bb97838..63207dc 100644 --- a/drivers/usb/dwc2/hcd_queue.c +++ b/drivers/usb/dwc2/hcd_queue.c @@ -761,6 +761,7 @@ void dwc2_hcd_qtd_init(struct dwc2_qtd *qtd, struct dwc2_hcd_urb *urb) /** * dwc2_hcd_qtd_add() - Adds a QTD to the QTD-list of a QH + * Caller must hold driver lock. * * @hsotg: The DWC HCD structure * @qtd: The QTD to add @@ -777,7 +778,6 @@ int dwc2_hcd_qtd_add(struct dwc2_hsotg *hsotg, struct dwc2_qtd *qtd, struct dwc2_qh **qh, gfp_t mem_flags) { struct dwc2_hcd_urb *urb = qtd->urb; - unsigned long flags; int allocated = 0; int retval; @@ -792,15 +792,12 @@ int dwc2_hcd_qtd_add(struct dwc2_hsotg *hsotg, struct dwc2_qtd *qtd, allocated = 1; } - spin_lock_irqsave(&hsotg->lock, flags); - retval = dwc2_hcd_qh_add(hsotg, *qh); if (retval) goto fail; qtd->qh = *qh; list_add_tail(&qtd->qtd_list_entry, &(*qh)->qtd_list); - spin_unlock_irqrestore(&hsotg->lock, flags); return 0; @@ -817,10 +814,7 @@ fail: qtd_list_entry) dwc2_hcd_qtd_unlink_and_free(hsotg, qtd2, qh_tmp); - spin_unlock_irqrestore(&hsotg->lock, flags); dwc2_hcd_qh_free(hsotg, qh_tmp); - } else { - spin_unlock_irqrestore(&hsotg->lock, flags); } return retval; -- 2.7.4