kern/114492: device_attach() doesn't unset devclass on error

[Raaf]

>Number:         114492
>Category:       kern
>Synopsis:       device_attach() doesn't unset devclass on error
>Confidential:   no
>Severity:       non-critical
>Priority:       medium
>Responsible:    freebsd-bugs
>State:          open
>Quarter:        
>Keywords:       
>Date-Required:
>Class:          sw-bug
>Submitter-Id:   current-users
>Arrival-Date:   Wed Jul 11 06:50:01 GMT 2007
>Closed-Date:
>Last-Modified:
>Originator:     Raaf
>Release:        FreeBSD 6.2-STABLE i386
>Organization:
>Environment:
System: FreeBSD zen.mooo.com 6.2-STABLE FreeBSD 6.2-STABLE #10: Wed Jul 11 08:00:54 CEST 2007 raaf at zen.mooo.com:/usr/obj/usr/src/sys/RANI i386

>Description:
device_attach() doesn't unset devclass on error.

I have a Sony Ericsson mobile phone that has 9 USB interfaces.

Interface 1: Is handled by umodem(4)
Interface 2: Is the associated data interface for interfaces 1 and 3
Interface 3: Is also handled by umodem(4)
Interface 8: Is handled by umass(4)

device_attach() fails on interface 3 because interface 3 requires data
interface 2 which is already taken by interface 1.

Thats is not a big problem.

However because device_attach() doesn't unset devclass on error, all
subsequent calls on the remaining device interfaces will be called
with devclass set to that of the umodem driver, and umass match/attach
will never be called for interface 8.

Note that this current code in device_attach() doesn't do anything:
----------------------------------------------------------------------
/* Unset the class; set in device_probe_child */
if (dev->devclass == 0)
	device_set_devclass(dev, 0);
----------------------------------------------------------------------

>How-To-Repeat:
	
>Fix:

The following patch fixes it for me.

--- patch-subr_bus.c begins here ---
--- sys/kern/subr_bus.c.orig	Wed Jul 11 07:50:24 2007
+++ sys/kern/subr_bus.c	Wed Jul 11 07:56:54 2007
@@ -2322,6 +2322,7 @@
 device_probe_and_attach(device_t dev)
 {
 	int error;
+	int hasclass = (dev->devclass != 0);
 
 	GIANT_REQUIRED;
 
@@ -2344,6 +2345,9 @@
 		return (error);
 	}
 	error = device_attach(dev);
+	/* Unset the class; set in device_probe_child */
+	if(error && !hasclass)
+		device_set_devclass(dev, NULL);
 
 	return (error);
 }
@@ -2378,9 +2382,6 @@
 	if ((error = DEVICE_ATTACH(dev)) != 0) {
 		printf("device_attach: %s%d attach returned %d\n",
 		    dev->driver->name, dev->unit, error);
-		/* Unset the class; set in device_probe_child */
-		if (dev->devclass == 0)
-			device_set_devclass(dev, 0);
 		device_set_driver(dev, NULL);
 		device_sysctl_fini(dev);
 		dev->state = DS_NOTPRESENT;
--- patch-subr_bus.c ends here ---



>Release-Note:
>Audit-Trail:
>Unformatted: