[PATCH 16/21] USB: musb: host endpoint_disable() oops fixes

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



From: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>

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@xxxxxxxxxxxxxxxxxxxxx: massively simplify
  patch description; add key points as code comments ]

Signed-off-by: Sergei Shtylyov <sshtylyov@xxxxxxxxxxxxx>
Signed-off-by: David Brownell <dbrownell@xxxxxxxxxxxxxxxxxxxxx>
Signed-off-by: Greg Kroah-Hartman <gregkh@xxxxxxx>
---
 drivers/usb/musb/musb_host.c |   36 ++++++++++++++++++++++++++----------
 1 files changed, 26 insertions(+), 10 deletions(-)

diff --git a/drivers/usb/musb/musb_host.c b/drivers/usb/musb/musb_host.c
index e323239..c74ebad 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);
 }
 
-- 
1.6.0.2

--
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

[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux