[PATCH] Fix USB panics

Peter Pentchev roam at ringlet.net
Wed Sep 8 08:09:22 PDT 2004


Hi,

A couple of days ago I started experimenting with a new USB device,
which has no FreeBSD driver, and within the first minute of playing with
it, my kernel panicked - something that I hadn't seen for about a month,
even with 5.x :)

So here's a How to Panic a RELENG_5 Kernel In Five Easy Steps :)

1.  Make sure usbd is not running so it cannot load drivers on demand.

2.  kldload usb, and do not load *any* USB device drivers, not even ugen.

3.  Plug a device, any device.
3a. usbd_probe_and_attach() looks for a specific driver, and fails.
3b. usbd_probe_and_attach() looks for a generic driver, and fails.
3c. usbd_probe_and_attach() leaves the newly-allocated device_t
    structure in place (and added to the parent bus), but removes any
    subdevs traces in the usbd_device_handle.

4.  Unplug the device.
4a. usb_disconnect_port() does not find the device_t structure, since it
    has been removed from usbd_device_handle's subdevs.
4b. usb_disconnect_port() frees the usbd_device_handle.

5.  kldload ugen
5a. busdma walks the buses, looking for devices that this driver should
    attach to.
5b. busdma calls ugen_attach() on the somewhat orphaned device_t.
5c. ugen_attach() calls usbd_devinfo() on the usbd_device_handle pointer
    extracted from the device_t's softc - which was kinda freed in step
    4b above :)
5d. Congratulations, you have reached kdb's panic handler! :)

So.. what should be done about it?  Basically, the problem is that
usbd_probe_and_attach() leaves an orphaned device_t pointing to the
usbd_device_handle that will be freed when the hardware device is
physically unplugged.

The attached patch seems to solve the problem - and a couple of related
others - at least for me.  The #ifdef's are effectively #if 0's, I've
just left them in to keep a perspective on the original code.  The idea
is to keep the device_t pointer in the usbd_device_handle, but take care
to check if the device is actually attached before dereferencing it.
Also, uhub had to be taught not to remove the device_t pointer on driver
unload, since the hardware is still physically attached to the machine
and the device_t is still attached to the bus, even though there is no
driver for it.  This makes uhub's child_detached handler mostly a no-op,
with the exception of the panic if the uhub itself is not initialized
yet; should the whole handler be removed, since the only thing it does
ought to be handled by usb_disconnect_port() already?

Is this even the right direction?  IMHO, this situation ought to be
resolved one way or another before 5.3 hits the shelves, even if the
solution has nothing to do with my proposed patch :)

G'luck,
Peter

-- 
Peter Pentchev	roam at ringlet.net    roam at cnsys.bg    roam at FreeBSD.org
PGP key:	http://people.FreeBSD.org/~roam/roam.key.asc
Key fingerprint	FDBA FD79 C26F 3C51 C95E  DF9E ED18 B68D 1619 4553
If the meanings of 'true' and 'false' were switched, then this sentence wouldn't be false.
-------------- next part --------------
Index: dev/usb/uhub.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/usb/uhub.c,v
retrieving revision 1.62
diff -u -r1.62 uhub.c
--- dev/usb/uhub.c	15 Aug 2004 23:39:18 -0000	1.62
+++ dev/usb/uhub.c	8 Sep 2004 14:06:45 -0000
@@ -707,7 +707,9 @@
 			continue;
 		for (i = 0; dev->subdevs[i]; i++) {
 			if (dev->subdevs[i] == child) {
+#ifdef ROAM_SKIP_USB_DEVICE_LEAK
 				dev->subdevs[i] = NULL;
+#endif
 				return;
 			}
 		}
Index: dev/usb/usb_subr.c
===================================================================
RCS file: /home/ncvs/src/sys/dev/usb/usb_subr.c,v
retrieving revision 1.69
diff -u -r1.69 usb_subr.c
--- dev/usb/usb_subr.c	15 Aug 2004 23:39:18 -0000	1.69
+++ dev/usb/usb_subr.c	8 Sep 2004 14:05:51 -0000
@@ -1016,9 +1016,11 @@
 	if (dv != NULL) {
 		return (USBD_NORMAL_COMPLETION);
 	}
+#ifdef ROAM_SKIP_USB_DEVICE_LEAK
 	tmpdv = dev->subdevs;
 	dev->subdevs = 0;
 	free(tmpdv, M_USB);
+#endif
 
 	/*
 	 * The generic attach failed, but leave the device as it is.
@@ -1346,7 +1348,7 @@
 	di->udi_speed = dev->speed;
 
 	if (dev->subdevs != NULL) {
-		for (i = 0; dev->subdevs[i] &&
+		for (i = 0; dev->subdevs[i] && device_is_attached(dev->subdevs[i]) &&
 			     i < USB_MAX_DEVNAMES; i++) {
 			strncpy(di->udi_devnames[i], USBDEVPTRNAME(dev->subdevs[i]),
 				USB_MAX_DEVNAMELEN);
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 187 bytes
Desc: not available
Url : http://lists.freebsd.org/pipermail/freebsd-current/attachments/20040908/f9a73212/attachment.bin


More information about the freebsd-current mailing list