kern/44883: ehci.c setting up bogus qtd chain

[gmcnutt%cradlepoint.com@localhost]

>Number:         44883
>Category:       kern
>Synopsis:       ehci.c setting up bogus qtd chain
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Tue Apr 19 17:45:00 +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 {