From owner-freebsd-current@FreeBSD.ORG Wed Sep 8 15:09:21 2004 Return-Path: Delivered-To: freebsd-current@freebsd.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id E46D016A4CF for ; Wed, 8 Sep 2004 15:09:21 +0000 (GMT) Received: from gandalf.online.bg (gandalf.online.bg [217.75.128.9]) by mx1.FreeBSD.org (Postfix) with SMTP id 4C22643D5E for ; Wed, 8 Sep 2004 15:09:20 +0000 (GMT) (envelope-from roam@ringlet.net) Received: (qmail 3398 invoked from network); 8 Sep 2004 15:07:20 -0000 Received: from unknown (HELO straylight.m.ringlet.net) (217.75.134.254) by gandalf.online.bg with SMTP; 8 Sep 2004 15:07:20 -0000 Received: (qmail 3301 invoked by uid 1000); 8 Sep 2004 15:09:43 -0000 Date: Wed, 8 Sep 2004 18:09:43 +0300 From: Peter Pentchev To: freebsd-hackers@FreeBSD.org Message-ID: <20040908150943.GA1924@straylight.m.ringlet.net> Mail-Followup-To: freebsd-hackers@FreeBSD.org, freebsd-current@FreeBSD.org Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="OBd5C1Lgu00Gd/Tn" Content-Disposition: inline User-Agent: Mutt/1.5.6i cc: freebsd-current@FreeBSD.org Subject: [PATCH] Fix USB panics X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.1 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 08 Sep 2004 15:09:22 -0000 --OBd5C1Lgu00Gd/Tn Content-Type: multipart/mixed; boundary="2B/JsCI69OhZNC5r" Content-Disposition: inline --2B/JsCI69OhZNC5r Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable 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 --=20 Peter Pentchev roam@ringlet.net roam@cnsys.bg roam@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 wou= ldn't be false. --2B/JsCI69OhZNC5r Content-Type: text/plain; charset=us-ascii Content-Disposition: attachment; filename="usb-keep-subdevs.patch" Content-Transfer-Encoding: quoted-printable Index: dev/usb/uhub.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D 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 =3D 0; dev->subdevs[i]; i++) { if (dev->subdevs[i] =3D=3D child) { +#ifdef ROAM_SKIP_USB_DEVICE_LEAK dev->subdevs[i] =3D NULL; +#endif return; } } Index: dev/usb/usb_subr.c =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D= =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D 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 !=3D NULL) { return (USBD_NORMAL_COMPLETION); } +#ifdef ROAM_SKIP_USB_DEVICE_LEAK tmpdv =3D dev->subdevs; dev->subdevs =3D 0; free(tmpdv, M_USB); +#endif =20 /* * The generic attach failed, but leave the device as it is. @@ -1346,7 +1348,7 @@ di->udi_speed =3D dev->speed; =20 if (dev->subdevs !=3D NULL) { - for (i =3D 0; dev->subdevs[i] && + for (i =3D 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); --2B/JsCI69OhZNC5r-- --OBd5C1Lgu00Gd/Tn Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.2.6 (FreeBSD) iD8DBQFBPyC27Ri2jRYZRVMRAljhAKC0NSaiYjxVChqo5KOEAUn0MvRn6wCgmE8l f5EV/LNvWB7ysJH3gcsDnE8= =k7lY -----END PGP SIGNATURE----- --OBd5C1Lgu00Gd/Tn--