NetBSD Problem Report #44883

From www@NetBSD.org  Tue Apr 19 17:44:44 2011
Return-Path: <www@NetBSD.org>
Received: from mail.netbsd.org (mail.netbsd.org [204.152.190.11])
	by www.NetBSD.org (Postfix) with ESMTP id E68AB63C37C
	for <gnats-bugs@gnats.NetBSD.org>; Tue, 19 Apr 2011 17:44:43 +0000 (UTC)
Message-Id: <20110419174442.C916563BBDB@www.NetBSD.org>
Date: Tue, 19 Apr 2011 17:44:42 +0000 (UTC)
From: gmcnutt@cradlepoint.com
Reply-To: gmcnutt@cradlepoint.com
To: gnats-bugs@NetBSD.org
Subject: ehci.c setting up bogus qtd chain
X-Send-Pr-Version: www-1.0

>Number:         44883
>Category:       kern
>Synopsis:       ehci.c setting up bogus qtd chain
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Apr 19 17:45:00 +0000 2011
>Closed-Date:    Thu May 05 18:15:34 +0000 2011
>Last-Modified:  Fri May 20 19:35:01 +0000 2011
>Originator:     Gordon McNutt
>Release:        5.0
>Organization:
CradlePoint, Inc
>Environment:
NetBSD  5.1 NetBSD 5.1 (ER95) #0: Mon Apr 18 12:28:24 MDT 2011  gmcnutt@gmcnutt-deskts

>Description:
There's a bug in ehci.c current where it sets up the qtd chain. Here's the DIAGNOSTIC output from ehci.c showing the conditions that setup the problem:

ehci_alloc_sqtd_chain: dataphys=0x007e9fc0 dataphyslastpage=0x007e9000 len=64 curlen=64 mps=64

Also USBD_FORCE_SHORT_XFER must be set. What is happening is that dataphys is coincidentally len bytes away from an EHCI_PAGE boundary. After the first iteration through the loop we end up with len=0 but next!=NULL and dataphyspage > dataphyslastpage:

ehci_alloc_sqtd_chain: curlen=0x5000 len=0x0 offs=0x0 lastpage=0x7e9000 page=0x7ea000 phys=0x7ea000
panic: ehci_alloc_sqtd_chain: curlen == 0

Without DIAGNOSTIC enabled the panic would not occur; instead len would become negative, and the loop would continue running, setting up bogus qtd's until we hit a nomem condition.

>How-To-Repeat:
I do it with multiple torrents over multiple usb modems. I expect any heavy usb traffic (other than mass storage, which does not use short xfers) would show it sooner or later.
>Fix:
Index: sys/dev/usb/ehci.c
===================================================================
--- sys/dev/usb/ehci.c	(revision 4081)
+++ sys/dev/usb/ehci.c	(working copy)
@@ -2627,8 +2627,11 @@
 	for (;;) {
 		dataphyspage = EHCI_PAGE(dataphys);
 		/* The EHCI hardware can handle at most 5 pages. */
-		if (dataphyslastpage - dataphyspage <
-		    EHCI_QTD_NBUFFERS * EHCI_PAGE_SIZE) {
+		/* If dataphyspage > dataphyslastpage then len = 0, but we
+		 * still need to setup cur as a zero-length packet. */
+		if ((dataphyspage > dataphyslastpage) ||
+		    (dataphyslastpage - dataphyspage <
+		     EHCI_QTD_NBUFFERS * EHCI_PAGE_SIZE)) {
 			/* we can handle it in this QTD */
 			curlen = len;
 		} else {

>Release-Note:

>Audit-Trail:
From: "Matthias Drochner" <drochner@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44883 CVS commit: src/sys/dev/usb
Date: Wed, 20 Apr 2011 09:32:43 +0000

 Module Name:	src
 Committed By:	drochner
 Date:		Wed Apr 20 09:32:43 UTC 2011

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

 Log Message:
 in alloc_sqtd_chain(), deal with the case where a data packet ends
 exactly at a page boundary, and the FORCE_SHORT_XFER was set by the
 client (which causes that an empty descriptor is needed to terminate
 the transfer), from Gordon McNutt per PR kern/44883
 (fixed a bit differently than the proposed patch for aesthetical
 reasons -- avoids the page pointer to come into unexpexted area earlier)


 To generate a diff of this commit:
 cvs rdiff -u -r1.173 -r1.174 src/sys/dev/usb/ehci.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: drochner@NetBSD.org
State-Changed-When: Wed, 20 Apr 2011 09:36:18 +0000
State-Changed-Why:
should be fixed in ehci.c rev.1.174; I'll request a pullup to 5.x
if no problems show up after a week or so


From: Gordon McNutt <gmcnutt@cradlepoint.com>
To: gnats-bugs@gnats.netbsd.org
Cc: 
Subject: Re: kern/44883
Date: Thu, 5 May 2011 09:14:29 -0600

 We've been running with the fix and have seen no further problems.

State-Changed-From-To: feedback->closed
State-Changed-By: drochner@NetBSD.org
State-Changed-When: Thu, 05 May 2011 18:15:34 +0000
State-Changed-Why:
submitter confirmed, pullup requested


From: "Manuel Bouyer" <bouyer@netbsd.org>
To: gnats-bugs@gnats.NetBSD.org
Cc: 
Subject: PR/44883 CVS commit: [netbsd-5] src/sys/dev/usb
Date: Fri, 20 May 2011 19:31:47 +0000

 Module Name:	src
 Committed By:	bouyer
 Date:		Fri May 20 19:31:47 UTC 2011

 Modified Files:
 	src/sys/dev/usb [netbsd-5]: ehci.c

 Log Message:
 Pull up following revision(s) (requested by drochner in ticket #1620):
 	sys/dev/usb/ehci.c: revision 1.174
 in alloc_sqtd_chain(), deal with the case where a data packet ends
 exactly at a page boundary, and the FORCE_SHORT_XFER was set by the
 client (which causes that an empty descriptor is needed to terminate
 the transfer), from Gordon McNutt per PR kern/44883
 (fixed a bit differently than the proposed patch for aesthetical
 reasons -- avoids the page pointer to come into unexpexted area earlier)


 To generate a diff of this commit:
 cvs rdiff -u -r1.154.4.2 -r1.154.4.3 src/sys/dev/usb/ehci.c

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

>Unformatted:

Home	PR Database Search