linux-kernel - [34-longterm 078/209] USB: EHCI: fix obscure race in ehci_endpoint

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <1302803039-9400-79-git-send-email-paul.gortmaker@windriver.com>
Date:	Thu, 14 Apr 2011 13:41:48 -0400
From:	Paul Gortmaker <paul.gortmaker@...driver.com>
To:	stable@...nel.org, linux-kernel@...r.kernel.org
Cc:	stable-review@...nel.org, Alan Stern <stern@...land.harvard.edu>,
	David Brownell <david-b@...bell.net>,
	Greg Kroah-Hartman <gregkh@...e.de>,
	Paul Gortmaker <paul.gortmaker@...driver.com>
Subject: [34-longterm 078/209] USB: EHCI: fix obscure race in ehci_endpoint_disable

From: Alan Stern <stern@...land.harvard.edu>

  =====================================================================
  | This is a commit scheduled for the next v2.6.34 longterm release. |
  | If you see a problem with using this for longterm, please comment.|
  =====================================================================

commit 02e2c51ba3e80acde600721ea784c3ef84da5ea1 upstream.

This patch (as1435) fixes an obscure and unlikely race in ehci-hcd.
When an async URB is unlinked, the corresponding QH is removed from
the async list.  If the QH's endpoint is then disabled while the URB
is being given back, ehci_endpoint_disable() won't find the QH on the
async list, causing it to believe that the QH has been lost.  This
will lead to a memory leak at best and quite possibly to an oops.

The solution is to trust usbcore not to lose track of endpoints.  If
the QH isn't on the async list then it doesn't need to be taken off
the list, but the driver should still wait for the QH to become IDLE
before disabling it.

In theory this fixes Bugzilla #20182.  In fact the race is so rare
that it's not possible to tell whether the bug is still present.
However, adding delays and making other changes to force the race
seems to show that the patch works.

Signed-off-by: Alan Stern <stern@...land.harvard.edu>
Reported-by: Stefan Richter <stefanr@...6.in-berlin.de>
CC: David Brownell <david-b@...bell.net>
Signed-off-by: Greg Kroah-Hartman <gregkh@...e.de>
Signed-off-by: Paul Gortmaker <paul.gortmaker@...driver.com>
---
 drivers/usb/host/ehci-hcd.c |   10 +++++-----
 1 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/drivers/usb/host/ehci-hcd.c b/drivers/usb/host/ehci-hcd.c
index 13ead00..d2dd6e1 100644
--- a/drivers/usb/host/ehci-hcd.c
+++ b/drivers/usb/host/ehci-hcd.c
@@ -1010,10 +1010,11 @@ rescan:
 				tmp && tmp != qh;
 				tmp = tmp->qh_next.qh)
 			continue;
-		/* periodic qh self-unlinks on empty */
-		if (!tmp)
-			goto nogood;
-		unlink_async (ehci, qh);
+		/* periodic qh self-unlinks on empty, and a COMPLETING qh
+		 * may already be unlinked.
+		 */
+		if (tmp)
+			unlink_async(ehci, qh);
 		/* FALL THROUGH */
 	case QH_STATE_UNLINK:		/* wait for hw to finish? */
 	case QH_STATE_UNLINK_WAIT:
@@ -1030,7 +1031,6 @@ idle_timeout:
 		}
 		/* else FALL THROUGH */
 	default:
-nogood:
 		/* caller was supposed to have unlinked any requests;
 		 * that's not our job.  just leak this memory.
 		 */
-- 
1.7.4.4

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/