Subject: kern/32011: usb HC detach race condition
To: None <kern-bug-people@netbsd.org, gnats-admin@netbsd.org,>
From: Matthew Orgass <darkstar@city-net.com>
List: netbsd-bugs
Date: 11/07/2005 21:26:00
>Number:         32011
>Category:       kern
>Synopsis:       usb HC detach race condition
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Nov 07 21:26:00 +0000 2005
>Originator:     darkstar@city-net.com
>Release:        NetBSD 2.0_RC4
>Organization:
>Description:

  A uvm_fault dereferencing 0xdeadb000 occurs when detaching a USB host
controller that is in usbd_get_desc before the device is attached.  The reason
is that usb_detach calls usb_disconnect_port which frees the device structure
before the transfering thread is awakened; when usbd_do_request_flags_pipe
later tries to free its xfer it will dereference the already freed device.

  Also, usb_detach does not wait for the event thread to exit, but will
exit after its sleep timer returns.

>How-To-Repeat:

  Make a removeable HC get stuck in an early transfer (or add some delays,
etc.) and remove (with DIAGNOSTIC kernel).  Removing and inserting an HC
with devices connected could normally cause the same problem, but the window
should be short.

>Fix:

  Loop waiting for the event thread to return before doing the detach.

Index: usb.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usb.c,v
retrieving revision 1.80
diff -u -r1.80 usb.c
--- usb.c	7 Nov 2003 17:03:25 -0000	1.80
+++ usb.c	7 Nov 2005 21:14:07 -0000
@@ -803,18 +803,16 @@

 	sc->sc_dying = 1;

-	/* Make all devices disconnect. */
-	if (sc->sc_port.device != NULL)
-		usb_disconnect_port(&sc->sc_port, self);
-
 	/* Kill off event thread. */
-	if (sc->sc_event_thread != NULL) {
+	while (sc->sc_event_thread != NULL) {
 		wakeup(&sc->sc_bus->needs_explore);
-		if (tsleep(sc, PWAIT, "usbdet", hz * 60))
-			printf("%s: event thread didn't die\n",
-			       USBDEVNAME(sc->sc_dev));
-		DPRINTF(("usb_detach: event thread dead\n"));
+		tsleep(sc, PWAIT, "usbdet", hz * 60);
 	}
+	DPRINTF(("usb_detach: event thread dead\n"));
+
+	/* Make all devices disconnect. */
+	if (sc->sc_port.device != NULL)
+		usb_disconnect_port(&sc->sc_port, self);

 	usbd_finish();


>Unformatted: