[PATCH v3]USB:OHCI: BugFix:Proper handling of ed_rm_list to handle race condition between usb_kill_urb() and finish_unlinks()

[AMAN DEEP <aman.deep@xxxxxxxxxxx>]

There is a race condition between 
 finish_unlinks->finish_urb() function and 
 usb_kill_urb() in ohci controller case. The finish_urb 
 calls spin_unlock(&ohci->lock) before 
 usb_hcd_giveback_urb() function call, then if during 
 this time, usb_kill_urb is called for another endpoint,
  then new ed will be added to ed_rm_list at beginning 
 for unlink. and ed_rm_list will point to newly added 
 ed.

When finish_urb() is completed in finish_unlinks() and
ed->td_list becomes empty as in below code (in finish_unlinks() function)
        if (list_empty(&ed->td_list)) {
                *last = ed->ed_next;
                ed->ed_next = NULL;
        } else if (ohci->rh_state == OHCI_RH_RUNNING) {
                *last = ed->ed_next;
                ed->ed_next = NULL;
                ed_schedule(ohci, ed);
        }
*last = ed->ed_next will make ed_rm_list to point to ed->ed_next and
previously added ed by usb_kill_urb will be left unreferenced by
ed_rm_list. This causes usb_kill_urb() hang forever waiting for
finish_unlink to remove added ed from ed_rm_list.

The main reason for hang in this race condtion is addition and removal
of ed from ed_rm_list in the beginning during usb_kill_urb and later last*
is modified in finish_unlinks().
As suggested by Alan Stern, the solution for proper handling of
ohci->ed_rm_list is to remove ed from the ed_rm_list before finishing
any URBs. Then at the end, we can add ed back to the list if necessary.
This properly handle the updated ohci->ed_rm_list in
usb_kill_urb().

Fixes:977dcfdc6031("USB:OHCI:don't lose track of EDs when a 
				controller dies")

Acked-by: Alan Stern <stern@xxxxxxxxxxxxxxxxxxx>

Signed-off-by: Aman Deep <aman.deep@xxxxxxxxxxx>
CC: <stable@xxxxxxxxxxxxxxx>
---
 drivers/usb/host/ohci-q.c |   17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/drivers/usb/host/ohci-q.c b/drivers/usb/host/ohci-q.c
index f7d561e..496658b 100644
--- a/drivers/usb/host/ohci-q.c
+++ b/drivers/usb/host/ohci-q.c
@@ -1023,6 +1023,8 @@ ed_idle:
 		 * have modified this list.  normally it's just prepending
 		 * entries (which we'd ignore), but paranoia won't hurt.
 		 */
+		*last = ed->ed_next;
+		ed->ed_next = NULL;
 		modified = 0;
 
 		/* unlink urbs as requested, but rescan the list after
@@ -1081,20 +1083,21 @@ rescan_this:
 			goto rescan_this;
 
 		/*
-		 * If no TDs are queued, take ED off the ed_rm_list.
+		 * If no TDs are queued, ED is now idle.
 		 * Otherwise, if the HC is running, reschedule.
-		 * If not, leave it on the list for further dequeues.
+		 * If the HC isn't running, add ED back to the
+		 * start of the list for later processing.
 		 */
 		if (list_empty(&ed->td_list)) {
-			*last = ed->ed_next;
-			ed->ed_next = NULL;
 			list_del(&ed->in_use_list);
 		} else if (ohci->rh_state == OHCI_RH_RUNNING) {
-			*last = ed->ed_next;
-			ed->ed_next = NULL;
 			ed_schedule(ohci, ed);
 		} else {
-			last = &ed->ed_next;
+			ed->ed_next = ohci->ed_rm_list;
+			ohci->ed_rm_list = ed;
+			/* Don't loop on the same ED */
+			if (last == &ohci->ed_rm_list)
+				last = &ed->ed_next;
 		}
 
 		if (modified)
-- 
1.7.9.5
ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ¥Šwÿº{.nÇ+‰·¥Š{±þëþ)í…æèw*jg¬±¨¶‰šŽŠÝ¢jÿ¾«þG«?éÿ¢¸¢·¦j:+v‰¨ŠwèjØm¶Ÿÿþø¯ù®w¥þŠàþf£¢·hš?â?úÿ†Ù¥