NetBSD Problem Report #32011

From darkstar@city-net.com  Mon Nov  7 21:25:44 2005
Return-Path: <darkstar@city-net.com>
Received: from trixie.city-net.com (mail.city-net.com [198.144.32.6])
	by narn.netbsd.org (Postfix) with ESMTP id 4340463B938
	for <gnats-bugs@gnats.netbsd.org>; Mon,  7 Nov 2005 21:25:44 +0000 (UTC)
Message-Id: <Pine.BSF.4.51.0511071612260.28301@vegeta.city-net.com>
Date: Mon, 7 Nov 2005 16:15:03 -0500 (EST)
From: Matthew Orgass <darkstar@city-net.com>
To: gnats-bugs@netbsd.org
Subject: usb HC detach race condition

>Number:         32011
>Category:       kern
>Synopsis:       usb HC detach race condition
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    kern-bug-people
>State:          closed
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Nov 07 21:26:00 +0000 2005
>Closed-Date:    Sun Mar 08 09:18:38 +0000 2015
>Last-Modified:  Sun Mar 08 09:18:38 +0000 2015
>Originator:     darkstar@city-net.com
>Release:        NetBSD 2.0_RC4
>Organization:
>Environment:

>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();


>Release-Note:

>Audit-Trail:
From: Matthew Orgass <darkstar@city-net.com>
To: gnats-bugs@netbsd.org
Cc: 
Subject: Re: kern/32011 usb HC detach race condition
Date: Tue, 21 Nov 2006 10:59:42 -0500 (EST)

   An alternate patch that also add sc_dying setting to uhci, moves it in
 ehci and ohci, removes it from usb_detach (where it is unnecessary),
 and includes the patch above.

 Index: sys/dev/usb/uhci.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/usb/uhci.c,v
 retrieving revision 1.204
 diff -u -p -r1.204 uhci.c
 --- sys/dev/usb/uhci.c	31 Oct 2006 20:43:31 -0000	1.204
 +++ sys/dev/usb/uhci.c	21 Nov 2006 16:34:54 -0000
 @@ -573,6 +573,7 @@ uhci_activate(device_ptr_t self, enum de
  		return (EOPNOTSUPP);

  	case DVACT_DEACTIVATE:
 +		sc->sc_dying = 1;
  		if (sc->sc_child != NULL)
  			rv = config_deactivate(sc->sc_child);
  		break;
 Index: sys/dev/usb/ehci.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/usb/ehci.c,v
 retrieving revision 1.114
 diff -u -p -r1.114 ehci.c
 --- sys/dev/usb/ehci.c	31 Oct 2006 20:43:31 -0000	1.114
 +++ sys/dev/usb/ehci.c	21 Nov 2006 16:34:59 -0000
 @@ -983,9 +983,9 @@ ehci_activate(device_ptr_t self, enum de
  		return (EOPNOTSUPP);

  	case DVACT_DEACTIVATE:
 +		sc->sc_dying = 1;
  		if (sc->sc_child != NULL)
  			rv = config_deactivate(sc->sc_child);
 -		sc->sc_dying = 1;
  		break;
  	}
  	return (rv);
 Index: sys/dev/usb/ohci.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/usb/ohci.c,v
 retrieving revision 1.178
 diff -u -p -r1.178 ohci.c
 --- sys/dev/usb/ohci.c	31 Oct 2006 20:43:31 -0000	1.178
 +++ sys/dev/usb/ohci.c	21 Nov 2006 16:35:04 -0000
 @@ -374,9 +374,9 @@ ohci_activate(device_ptr_t self, enum de
  		return (EOPNOTSUPP);

  	case DVACT_DEACTIVATE:
 +		sc->sc_dying = 1;
  		if (sc->sc_child != NULL)
  			rv = config_deactivate(sc->sc_child);
 -		sc->sc_dying = 1;
  		break;
  	}
  	return (rv);
 Index: sys/dev/usb/usb.c
 ===================================================================
 RCS file: /cvsroot/src/sys/dev/usb/usb.c,v
 retrieving revision 1.90
 diff -u -p -r1.90 usb.c
 --- sys/dev/usb/usb.c	31 Oct 2006 20:43:31 -0000	1.90
 +++ sys/dev/usb/usb.c	21 Nov 2006 16:35:06 -0000
 @@ -848,21 +848,17 @@ usb_detach(device_ptr_t self, int flags

  	DPRINTF(("usb_detach: start\n"));

 -	sc->sc_dying = 1;
 +	/* Kill off event thread. */
 +	while (sc->sc_event_thread != NULL) {
 +		wakeup(&sc->sc_bus->needs_explore);
 +		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);

 -	/* Kill off event thread. */
 -	if (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"));
 -	}
 -
  	usbd_finish();

  #ifdef USB_USE_SOFTINTR

State-Changed-From-To: open->closed
State-Changed-By: skrll@NetBSD.org
State-Changed-When: Sun, 08 Mar 2015 09:18:38 +0000
State-Changed-Why:
Fixed with
Module Name:	src
Committed By:	kiyohara
Date:		Wed Aug 15 04:00:35 UTC 2007

Modified Files:
	src/sys/dev/usb: ehci.c ohci.c uhci.c usb.c usbdi.c usbdi.h

Log Message:
* splsoftusb, IPL_SOFTUSB, and IPL_HARDUSB defines in usbdi.h
-> the current names are confusing (didn't change other drivers)
* fix invalid memory access in usbd_transfer (kern/24636)
-> needed for this driver
* fix USB HC detach race condition (kern/32011)
-> main patch needed for this driver, sc_dying changes in other drivers
not necessary but seem right to me

Patch from Matthew Orgass.
  http://mail-index.netbsd.org/tech-kern/2007/06/26/0001.html


To generate a diff of this commit:
cvs rdiff -r1.123 -r1.124 src/sys/dev/usb/ehci.c
cvs rdiff -r1.182 -r1.183 src/sys/dev/usb/ohci.c
cvs rdiff -r1.209 -r1.210 src/sys/dev/usb/uhci.c
cvs rdiff -r1.98 -r1.99 src/sys/dev/usb/usb.c
cvs rdiff -r1.119 -r1.120 src/sys/dev/usb/usbdi.c
cvs rdiff -r1.74 -r1.75 src/sys/dev/usb/usbdi.h


>Unformatted:

NetBSD Home
NetBSD PR Database Search

(Contact us) $NetBSD: query-full-pr,v 1.39 2013/11/01 18:47:49 spz Exp $
$NetBSD: gnats_config.sh,v 1.8 2006/05/07 09:23:38 tsutsui Exp $
Copyright © 1994-2007 The NetBSD Foundation, Inc. ALL RIGHTS RESERVED.