kern/38528: panic on unplugging Apple USB keyboard

[Christoph_Egger%gmx.de@localhost]

>Number:         38528
>Category:       kern
>Synopsis:       panic on unplugging Apple USB keyboard
>Confidential:   no
>Severity:       serious
>Priority:       medium
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Sun Apr 27 11:40:00 +0000 2008
>Originator:     Christoph Egger
>Release:        4.99.61
>Organization:
>Environment:
NetBSD netbsdamd64 4.99.61 NetBSD 4.99.61 (GENERIC) #22: Sun Apr 27 12:17:06 
CEST 2008  
cegger%powermacg5.local@localhost:/Users/cegger/devel/bsd/netbsd/obj.kern.amd64/GENERIC
 amd64

>Description:

A DIAGNOSTIC + DEBUG  kernel panics when unplugging
an Apple USB Keyboard.

It calls KASSERT(false); in src/sys/dev/usb/uhub.c:uhub_childdet(),
line 641.

On the uhub attaches uhidev0 and uhidev1.
Detaching uhidev0, but on detaching uhidev1, dev->subdevs[0] is NULL
in line 634: "for (i = 0; dev->subdevs[i]; i++) {"

Therefore, the loop is not entered and KASSERT(false); is triggered.

A successful tested possible solution is this diff written
by mlelstv:

Index: uhub.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/uhub.c,v
retrieving revision 1.96
diff -u -r1.96 uhub.c
--- uhub.c      21 Apr 2008 23:31:18 -0000      1.96
+++ uhub.c      27 Apr 2008 09:39:14 -0000
@@ -633,7 +633,9 @@
                        continue;
                for (i = 0; dev->subdevs[i]; i++) {
                        if (dev->subdevs[i] == child) {
-                               dev->subdevs[i] = NULL;
+                               do {
+                                       dev->subdevs[i] = dev->subdevs[i+1];
+                               } while (dev->subdevs[++i] != NULL);
                                return;
                        }
                }


However, this triggers another bug in 
src/sys/dev/usb/usb_subr.c:usb_disconnect_port :

config_detach: detached device uhub2 has children uhidev1
panic: config_detach

backtrace:
config_detach+0x388
usb_disconnect_port+0x7d
uhub_explore+0x12b
usb_discover+0x37
usb_event_thread+0x3e

The problem in usb_disconnect_port is 1506:

for (i = 0; dev->subdevs[i]; i++) {

It assumes an 0 terminated list whereby the config_detach()
modifies it. So the assumption is wrong.

A successful tested possible solution is this diff written by
mlelstv:

Index: usb_subr.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.152
diff -u -r1.152 usb_subr.c
--- usb_subr.c  5 Apr 2008 16:35:35 -0000       1.152
+++ usb_subr.c  27 Apr 2008 10:15:26 -0000
@@ -1503,7 +1503,9 @@
 
        if (dev->subdevs != NULL) {
                DPRINTFN(3,("usb_disconnect_port: disconnect subdevs\n"));
-               for (i = 0; dev->subdevs[i]; i++) {
+               for (i = 0; dev->subdevs[i]; i++)
+                       continue;
+               while (--i >= 0) {
                        printf("%s: at %s", USBDEVPTRNAME(dev->subdevs[i]),
                               hubname);
                        if (up->portno != 0)




>How-To-Repeat:

>Fix:

Use an DIAGNOSTIC+DEBUG kernel and unplug an Apple USB keyboard.