On 3/9/2015 8:05 AM, Mian Yousaf Kaukab wrote:
> From: Gregory Herrero <gregory.herrero@xxxxxxxxx>
>
> 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.
>
> Signed-off-by: Gregory Herrero <gregory.herrero@xxxxxxxxx>
> ---
> drivers/usb/dwc2/hcd.c | 14 +++-----------
> drivers/usb/dwc2/hcd_queue.c | 6 ------
> 2 files changed, 3 insertions(+), 17 deletions(-)
>
> diff --git a/drivers/usb/dwc2/hcd.c b/drivers/usb/dwc2/hcd.c
> index fd02b43..011b679 100644
> --- a/drivers/usb/dwc2/hcd.c
> +++ b/drivers/usb/dwc2/hcd.c
> @@ -354,7 +354,6 @@ static int dwc2_hcd_urb_enqueue(struct dwc2_hsotg *hsotg,
> gfp_t mem_flags)
> {
> struct dwc2_qtd *qtd;
> - unsigned long flags;
> u32 intr_mask;
> int retval;
> int dev_speed;
> @@ -405,11 +404,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;
> @@ -2424,6 +2421,7 @@ static int _dwc2_hcd_urb_enqueue(struct usb_hcd *hcd, struct urb *urb,
> void *buf;
> unsigned long flags;
>
> + spin_lock_irqsave(&hsotg->lock, flags);
Missing a couple places where this will not be unlocked in this
function.
Also, seems like you could hold off on locking until just before
usb_hcd_link_urb_to_ep().
John
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
