From dc61d238b8c850c34632ae1fbbdea529f8c41d16 Mon Sep 17 00:00:00 2001 From: Sergei Shtylyov Date: Sat, 21 Feb 2009 15:31:01 -0800 Subject: [PATCH] USB: musb: host endpoint_disable() oops fixes The musb_h_disable() routine can oops in some cases: - It's not safe to read hep->hcpriv outside musb->lock, since it gets changed on completion IRQ paths. - The list iterators aren't safe to use in that way; just remove the first element while !list_empty(), so deletions on other code paths can't make trouble. We need two "scrub the list" loops because only one branch should touch hardware and advance the schedule. [ dbrownell@users.sourceforge.net: massively simplify patch description; add key points as code comments ] Signed-off-by: Sergei Shtylyov Signed-off-by: David Brownell Signed-off-by: Greg Kroah-Hartman --- drivers/usb/musb/musb_host.c | 36 ++++++++++++++++++++++++++---------- 1 file changed, 26 insertions(+), 10 deletions(-) diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c index e32323938e98..c74ebadd4b9d 100644 --- a/drivers/usb/musb/musb_host.c +++ b/drivers/usb/musb/musb_host.c @@ -2103,15 +2103,16 @@ musb_h_disable(struct usb_hcd *hcd, struct usb_host_endpoint *hep) unsigned long flags; struct musb *musb = hcd_to_musb(hcd); u8 is_in = epnum & USB_DIR_IN; - struct musb_qh *qh = hep->hcpriv; - struct urb *urb, *tmp; + struct musb_qh *qh; + struct urb *urb; struct list_head *sched; - if (!qh) - return; - spin_lock_irqsave(&musb->lock, flags); + qh = hep->hcpriv; + if (qh == NULL) + goto exit; + switch (qh->type) { case USB_ENDPOINT_XFER_CONTROL: sched = &musb->control; @@ -2145,13 +2146,28 @@ musb_h_disable(struct usb_hcd *hcd, struct usb_host_endpoint *hep) /* cleanup */ musb_cleanup_urb(urb, qh, urb->pipe & USB_DIR_IN); - } else - urb = NULL; - /* then just nuke all the others */ - list_for_each_entry_safe_from(urb, tmp, &hep->urb_list, urb_list) - musb_giveback(qh, urb, -ESHUTDOWN); + /* Then nuke all the others ... and advance the + * queue on hw_ep (e.g. bulk ring) when we're done. + */ + while (!list_empty(&hep->urb_list)) { + urb = next_urb(qh); + urb->status = -ESHUTDOWN; + musb_advance_schedule(musb, urb, qh->hw_ep, is_in); + } + } else { + /* Just empty the queue; the hardware is busy with + * other transfers, and since !qh->is_ready nothing + * will activate any of these as it advances. + */ + while (!list_empty(&hep->urb_list)) + __musb_giveback(musb, next_urb(qh), -ESHUTDOWN); + hep->hcpriv = NULL; + list_del(&qh->ring); + kfree(qh); + } +exit: spin_unlock_irqrestore(&musb->lock, flags); }