⬢ DragonFlyBSD Kernel Audit
DF-5789 / fix.diff
← back to finding ↓ download raw
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -855,7 +855,8 @@
 	devclass_t dc = devclass_find(driver->name);
 	driverlink_t dl;
 	device_t dev;
-	int i;
+	device_t *devs;
+	int count, i, j;
 	int error;
 
 	PDEBUG(("%s from devclass %s", driver->name, DEVCLANAME(busclass)));
@@ -885,16 +886,30 @@
 	 * should not detach devices which are not children of devices in
 	 * the affected devclass.
 	 */
-	for (i = 0; i < dc->maxunit; i++)
-		if (dc->devices[i]) {
-			dev = dc->devices[i];
-			if (dev->driver == driver && dev->parent &&
-			    dev->parent->devclass == busclass) {
-				if ((error = device_detach(dev)) != 0)
-					return(error);
-				device_set_driver(dev, NULL);
-		    	}
+	/*
+	 * Snapshot the device list first: device_detach() irreversibly
+	 * destroys each device's driver binding (softc freed, sysctl tree
+	 * torn down, devclass entry removed for non-fixed-class children),
+	 * so bailing out mid-loop on the first detach veto used to leave
+	 * every already-detached unit permanently dead while this driver
+	 * remained registered and the module unload wedged.  On a veto,
+	 * roll back by re-probing the units already detached.
+	 */
+	devs = NULL;
+	count = 0;
+	if (devclass_get_devices(dc, &devs, &count) != 0)
+		devs = NULL;
+	for (i = 0; i < count && devs != NULL; i++) {
+		dev = devs[i];
+		if (dev->driver == driver && dev->parent &&
+		    dev->parent->devclass == busclass) {
+			if ((error = device_detach(dev)) != 0)
+				goto rollback;
+			device_set_driver(dev, NULL);
 		}
+	}
+	if (devs != NULL)
+		kfree(devs, M_TEMP);
 
 	TAILQ_REMOVE(&busclass->drivers, dl, link);
 	kfree(dl, M_BUS);
@@ -903,6 +918,26 @@
 
 	bus_data_generation_update();
 	return(0);
+
+rollback:
+	/*
+	 * Best-effort rollback: revive (re-probe/re-attach) the units of
+	 * this devclass that the aborted loop already detached.  A device
+	 * may end up bound to a different driver if one now matches
+	 * better, which is still preferable to leaving it dead while the
+	 * module stays loaded and the unload is wedged.
+	 */
+	if (devs != NULL) {
+		for (j = 0; j < count; j++) {
+			dev = devs[j];
+			if (dev != NULL && dev->driver == NULL && dev->parent &&
+			    dev->parent->devclass == busclass &&
+			    dev->state == DS_NOTPRESENT)
+				device_probe_and_attach(dev);
+		}
+		kfree(devs, M_TEMP);
+	}
+	return (error);
 }
 
 static driverlink_t
@@ -1292,6 +1327,17 @@
 	if ((error = device_detach(child)) != 0)
 		return(error);
 
+	/*
+	 * A child that was probed but never attached (state DS_ALIVE)
+	 * still holds a driver binding and an allocated softc:
+	 * device_detach() short-circuits for state != DS_ATTACHED
+	 * without releasing them.  Release the driver (which frees the
+	 * softc via device_set_driver) before the device object is
+	 * destroyed, otherwise the softc is leaked.
+	 */
+	if (child->driver != NULL)
+		device_set_driver(child, NULL);
+
 	/* remove children second */
 	while ( (grandchild = TAILQ_FIRST(&child->children)) ) {
         	error = device_delete_child(child, grandchild);