git: b4c700fa7cca - main - new-bus: Fix some shortcomings in disabling devices via hints

From: John Baldwin <jhb_at_FreeBSD.org>
Date: Sat, 23 Nov 2024 16:39:37 UTC
The branch main has been updated by jhb:

URL: https://cgit.FreeBSD.org/src/commit/?id=b4c700fa7cca7d3e4da84dd417fc3efc42788a52

commit b4c700fa7cca7d3e4da84dd417fc3efc42788a52
Author:     John Baldwin <jhb@FreeBSD.org>
AuthorDate: 2024-11-23 16:39:02 +0000
Commit:     John Baldwin <jhb@FreeBSD.org>
CommitDate: 2024-11-23 16:39:02 +0000

    new-bus: Fix some shortcomings in disabling devices via hints
    
    A device can be disabled via a hint after it is probed (but before it
    is attached).  The initial version of this marked the device disabled,
    but left the device "alive" meaning that dev->driver and dev->desc
    were untouched and still pointed into the driver that probed the
    device.  If that driver lives in a kernel module that is later
    unloaded, device_detach() called from devclass_delete_driver() doesn't
    do anything (the device's state is DS_ALIVE).  In particular, it
    doesn't call device_set_driver(dev, NULL) to disassociate the device
    from the driver that is being unloaded.
    
    There are several places where these stale pointers can be tripped
    over.  After kldunload, invoking the sysctl to fetch device info can
    dereference dev->desc and dev->driver causing panics.  Even without
    kldunload, a system suspend request will call the device_suspend and
    device_resume DEVMETHODs of the driver in question even though the
    device is not attached which can cause some excitement.
    
    To clean this up, more fully detach a device that is disabled by a
    hint by clearing the driver and setting the state to DS_NOTPRESENT.
    However, to keep the device name+unit combination reserved, leave the
    device attached to its devclass.
    
    This requires a change to 'devctl enable' handling to deal with this
    updated state.  It now checks for a non-NULL devclass to determine if
    a disabled device is in this state and if so it clears the hint.
    However, it also now clears the devclass before attaching the device.
    This gives all drivers an opportunity to attach to the now-enabled
    device.
    
    Reported by:    adrian
    Discussed with: imp
    Reviewed by:    imp
    MFC after:      1 week
    Differential Revision:  https://reviews.freebsd.org/D47691
---
 sys/kern/subr_bus.c | 21 +++++++++++++++------
 1 file changed, 15 insertions(+), 6 deletions(-)

diff --git a/sys/kern/subr_bus.c b/sys/kern/subr_bus.c
index 2dbe1072aa83..32ddcaa9f643 100644
--- a/sys/kern/subr_bus.c
+++ b/sys/kern/subr_bus.c
@@ -2591,7 +2591,13 @@ device_attach(device_t dev)
 	int error;
 
 	if (resource_disabled(dev->driver->name, dev->unit)) {
+		/*
+		 * Mostly detach the device, but leave it attached to
+		 * the devclass to reserve the name and unit.
+		 */
 		device_disable(dev);
+		(void)device_set_driver(dev, NULL);
+		dev->state = DS_NOTPRESENT;
 		if (bootverbose)
 			 device_printf(dev, "disabled via hints entry\n");
 		return (ENXIO);
@@ -5759,17 +5765,20 @@ devctl2_ioctl(struct cdev *cdev, u_long cmd, caddr_t data, int fflag,
 		 * attach the device rather than doing a full probe.
 		 */
 		device_enable(dev);
-		if (device_is_alive(dev)) {
+		if (dev->devclass != NULL) {
 			/*
 			 * If the device was disabled via a hint, clear
 			 * the hint.
 			 */
-			if (resource_disabled(dev->driver->name, dev->unit))
-				resource_unset_value(dev->driver->name,
+			if (resource_disabled(dev->devclass->name, dev->unit))
+				resource_unset_value(dev->devclass->name,
 				    dev->unit, "disabled");
-			error = device_attach(dev);
-		} else
-			error = device_probe_and_attach(dev);
+
+			/* Allow any drivers to rebid. */
+			if (!(dev->flags & DF_FIXEDCLASS))
+				devclass_delete_device(dev->devclass, dev);
+		}
+		error = device_probe_and_attach(dev);
 		break;
 	case DEV_DISABLE:
 		if (!device_is_enabled(dev)) {