NetBSD Problem Report #35379

From toshii@w.email.ne.jp  Mon Jan  8 09:10:53 2007
Return-Path: <toshii@w.email.ne.jp>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by narn.NetBSD.org (Postfix) with ESMTP id 5E4A563BB3C
	for <gnats-bugs@gnats.NetBSD.org>; Mon,  8 Jan 2007 09:10:53 +0000 (UTC)
Message-Id: <20070108075152.DCBC454@primula.my.domain>
Date: Mon,  8 Jan 2007 16:51:52 +0900 (JST)
From: toshii@w.email.ne.jp
Reply-To: toshii@w.email.ne.jp
To: gnats-bugs@NetBSD.org
Subject: ohci_abort_xfer is broken
X-Send-Pr-Version: 3.95

>Number:         35379
>Category:       kern
>Synopsis:       ohci_abort_xfer is broken and causes infinite loops
>Confidential:   no
>Severity:       critical
>Priority:       medium
>Responsible:    skrll
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jan 08 09:15:00 +0000 2007
>Closed-Date:    Wed Nov 22 19:10:14 +0000 2017
>Last-Modified:  Wed Nov 22 19:10:14 +0000 2017
>Originator:     toshii@w.email.ne.jp
>Release:        NetBSD 4.99.5
>Organization:

>Environment:


System: NetBSD pepper.my.domain 4.99.5 NetBSD 4.99.5 (PEPPER) #46: Mon Jan  8 01:53:35 JST 2007  toshii@primula.my.domain:/usr/src/current/sys/arch/i386/compile/PEPPER i386
Architecture: i386
Machine: i386
>Description:
	I've found two bugs.
	ohci_abort_xfer assumes xfer->hcpriv STD list is intact, but
	ohci_softintr could run earlier and some of STDs may have been
	connected to the free list.
	Secondly, done STDs shouldn't be reclaimed until they are seen
	on the done list, otherwise a corrupt done list results.

>How-To-Repeat:
	Prepare an nForce 5 motherboard and an Edirol UA-3 USB audio device.
	If the uaudio is plugged in at bootup, usbd_new_device fails with a
	usbd_set_address timeout and a corruption of a done list follows.

	A corruption sometimes can be seen with the following debug code.

Index: ohci.c
===================================================================
RCS file: /usr/local/NetBSD/NetBSD-CVS/src/sys/dev/usb/ohci.c,v
retrieving revision 1.179
diff -u -p -r1.179 ohci.c
--- ohci.c	16 Nov 2006 01:33:26 -0000	1.179
+++ ohci.c	8 Jan 2007 07:11:05 -0000
@@ -1276,6 +1291,9 @@ ohci_softintr(void *v)
 	int len, cc, s;
 	int i, j, actlen, iframes, uedir;
 	ohci_physaddr_t done;
+#ifdef OHCI_DEBUG
+	int listcnt = 0;
+#endif

 	DPRINTFN(10,("ohci_softintr: enter\n"));

@@ -1283,14 +1301,30 @@ ohci_softintr(void *v)

 	s = splhardusb();
 	done = O32TOH(sc->sc_hcca->hcca_done_head) & ~OHCI_DONE_INTRS;
-	sc->sc_hcca->hcca_done_head = 0;
-	OWRITE4(sc, OHCI_INTERRUPT_STATUS, OHCI_WDH);
+	if (done) {
+		sc->sc_hcca->hcca_done_head = 0;
+		OWRITE4(sc, OHCI_INTERRUPT_STATUS, OHCI_WDH);
+	} else {
+		ohci_physaddr_t reg;
+
+		reg = OREAD4(sc, OHCI_DONE_HEAD);
+		if (reg) {
+			DPRINTF(("ohci_softintr: done head %x\n", reg));
+		}
+	}
 	sc->sc_eintrs |= OHCI_WDH;
 	OWRITE4(sc, OHCI_INTERRUPT_ENABLE, OHCI_WDH);
 	splx(s);

 	/* Reverse the done list. */
 	for (sdone = NULL, sidone = NULL; done != 0; ) {
+#ifdef OHCI_DEBUG
+		if (listcnt++ > 300) {
+			ohci_dump_tds(sc, sdone);
+			ohci_dump_itds(sc, sidone);
+			panic("ohci_softintr: loop in done list");
+		}
+#endif
 		std = ohci_hash_find_td(sc, done);
 		if (std != NULL) {
 			std->dnext = sdone;
>Fix:
	The following patch should fix the problem.  It isn't well tested
	but I'll commit in a week unless someone speaks up.  Also, it
	would be better if xfer->hcpriv doesn't get rewritten that often.

Index: ohci.c
===================================================================
RCS file: /usr/local/NetBSD/NetBSD-CVS/src/sys/dev/usb/ohci.c,v
retrieving revision 1.179
diff -u -p -r1.179 ohci.c
--- ohci.c	16 Nov 2006 01:33:26 -0000	1.179
+++ ohci.c	8 Jan 2007 07:11:05 -0000
@@ -1323,7 +1357,9 @@ ohci_softintr(void *v)
 		xfer = std->xfer;
 		stdnext = std->dnext;
 		DPRINTFN(10, ("ohci_process_done: std=%p xfer=%p hcpriv=%p\n",
-				std, xfer, xfer ? xfer->hcpriv : 0));
+				std, xfer,
+				(xfer && xfer != (void *)OHCI_TD_DEFERRED) ?
+				xfer->hcpriv : 0));
 		if (xfer == NULL) {
 			/*
 			 * xfer == NULL: There seems to be no xfer associated
@@ -1333,11 +1369,17 @@ ohci_softintr(void *v)
 			 */
 			continue;
 		}
+		if (xfer == (void *)OHCI_TD_DEFERRED) {
+			ohci_free_std(sc, std);
+			continue;
+		}
 		if (xfer->status == USBD_CANCELLED ||
 		    xfer->status == USBD_TIMEOUT) {
 			DPRINTF(("ohci_process_done: cancel/timeout %p\n",
 				 xfer));
 			/* Handled by abort routine. */
+			xfer->hcpriv = std->nexttd;
+			ohci_free_std(sc, std);
 			continue;
 		}
 		usb_uncallout(xfer->timeout_handle, ohci_timeout, xfer);
@@ -1358,7 +1400,8 @@ ohci_softintr(void *v)
 				s = splusb();
 				usb_transfer_complete(xfer);
 				splx(s);
-			}
+			} else
+				xfer->hcpriv = std->nexttd;
 			ohci_free_std(sc, std);
 		} else {
 			/*
@@ -2270,7 +2315,17 @@ ohci_abort_xfer(usbd_xfer_handle xfer, u
 	for (; p->xfer == xfer; p = n) {
 		hit |= headp == p->physaddr;
 		n = p->nexttd;
-		ohci_free_std(sc, p);
+		if (OHCI_TD_GET_CC(O32TOH(p->td.td_flags)) !=
+		    OHCI_TD_GET_CC(OHCI_TD_NOCC)) {
+			/*
+			 * If a TD is retired, let the softintr handler
+			 * handle this.
+			 */
+			p->xfer = (void *)OHCI_TD_DEFERRED;
+		} else {
+			DPRINTFN(1,("ohci_abort_xfer: free %p\n", p));
+			ohci_free_std(sc, p);
+		}
 	}
 	/* Zap headp register if hardware pointed inside the xfer. */
 	if (hit) {
Index: ohcivar.h
===================================================================
RCS file: /usr/local/NetBSD/NetBSD-CVS/src/sys/dev/usb/ohcivar.h,v
retrieving revision 1.39
diff -u -p -r1.39 ohcivar.h
--- ohcivar.h	27 Dec 2005 04:06:45 -0000	1.39
+++ ohcivar.h	1 Jan 2007 23:14:27 -0000
@@ -54,6 +54,8 @@ typedef struct ohci_soft_td {
 	ohci_physaddr_t physaddr;
 	LIST_ENTRY(ohci_soft_td) hnext;
 	usbd_xfer_handle xfer;
+/* An invalid pointer value for a magic number */
+#define	OHCI_TD_DEFERRED	0x1
 	u_int16_t len;
 	u_int16_t flags;
 #define OHCI_CALL_DONE	0x0001

>Release-Note:

>Audit-Trail:
From: Sergey Svishchev <svs+pr@grep.ru>
To: gnats-bugs@netbsd.org
Cc: toshii@w.email.ne.jp
Subject: Re: kern/35379
Date: Fri, 9 May 2008 22:26:14 +0400

 Will you commit these fixes?

 There are earlier PRs with similar fixes (26545..26547), too.

 -- 
 Sergey Svishchev

From: "Nick Hudson" <skrll@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/35379 CVS commit: src/sys/dev/usb
Date: Fri, 22 Mar 2013 13:20:12 +0000

 Module Name:	src
 Committed By:	skrll
 Date:		Fri Mar 22 13:20:12 UTC 2013

 Modified Files:
 	src/sys/dev/usb: ohci.c

 Log Message:
 When dumping the done list in ohci_softintr / OHCI_DEBUG use the correct
 list next pointer.

 PR/33450 and part of PR/35379.


 To generate a diff of this commit:
 cvs rdiff -u -r1.233 -r1.234 src/sys/dev/usb/ohci.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

Responsible-Changed-From-To: kern-bug-people->skrll
Responsible-Changed-By: skrll@NetBSD.org
Responsible-Changed-When: Mon, 25 Mar 2013 08:00:05 +0000
Responsible-Changed-Why:
I'm looking at ohci atm


From: "Manuel Bouyer" <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/35379 CVS commit: [netbsd-6] src/sys/dev/usb
Date: Sat, 20 Apr 2013 09:52:20 +0000

 Module Name:	src
 Committed By:	bouyer
 Date:		Sat Apr 20 09:52:20 UTC 2013

 Modified Files:
 	src/sys/dev/usb [netbsd-6]: ohci.c

 Log Message:
 Pull up following revision(s) (requested by skrll in ticket #864):
 	sys/dev/usb/ohci.c: revision 1.234
 	sys/dev/usb/ohci.c: revision 1.236
 Don't leak memory if ohci_alloc_std_chain fails.
 When dumping the done list in ohci_softintr / OHCI_DEBUG use the correct
 list next pointer.
 PR/33450 and part of PR/35379.


 To generate a diff of this commit:
 cvs rdiff -u -r1.218.8.1 -r1.218.8.2 src/sys/dev/usb/ohci.c

 Please note that diffs are not public domain; they are subject to the
 copyright notices on the relevant files.

State-Changed-From-To: open->feedback
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Sat, 23 Apr 2016 13:00:49 +0000
State-Changed-Why:
Does this appear in 7.99.28 (nick-nhusb merged)?


State-Changed-From-To: feedback->closed
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Wed, 22 Nov 2017 19:10:14 +0000
State-Changed-Why:
Assume fixed


>Unformatted:

Home	PR Database Search