DragonFlyBSD Kernel Audit
DF-0774 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/vfs/devfs/devfs_core.c b/sys/vfs/devfs/devfs_core.c
index 0000000..1111111 100644
--- a/sys/vfs/devfs/devfs_core.c
+++ b/sys/vfs/devfs/devfs_core.c
@@ -2318,30 +2318,50 @@
 	int error;
 	struct devfs_clone_handler *chandler;
 	struct dev_clone_args ap;
+	d_clone_t *clone_fn;
 
+again:
 	TAILQ_FOREACH(chandler, &devfs_chandler_list, link) {
 		if (chandler->namlen != len)
 			continue;
+		/*
+		 * Capture the clone handler function pointer while we still
+		 * hold devfs_lock.  After lockmgr(LK_RELEASE) below, a
+		 * concurrent devfs_clone_handler_del() (e.g. from driver
+		 * detach / module unload) may free the chandler struct via
+		 * the core thread.  Using chandler->nhandler after the
+		 * unlock would be a use-after-free.  Copy into a local.
+		 *
+		 * Also note: we intentionally do NOT call devfs_config()
+		 * after dropping the lock.  That SYNC forces the core
+		 * thread to drain its message queue — including any
+		 * pending CHANDLER_DEL — before we proceed, which is
+		 * exactly what makes the free observable from this path.
+		 */
 		if ((!memcmp(chandler->name, name, len)) &&
-		    (chandler->nhandler)) {
+		    (clone_fn = chandler->nhandler)) {
 			/*
-			 * We have to unlock across the config and the
-			 * callback to avoid deadlocking.  The device is
-			 * likely to obtain its own lock in the callback
-			 * and might then call into devfs.
+			 * We have to unlock across the callback to avoid
+			 * deadlocking.  The device is likely to obtain its
+			 * own lock in the callback and might then call
+			 * into devfs.
 			 */
 			lockmgr(&devfs_lock, LK_RELEASE);
-			devfs_config();
 			ap.a_head.a_dev = dev;
 			ap.a_dev = NULL;
 			ap.a_name = name;
 			ap.a_namelen = len;
 			ap.a_mode = mode;
 			ap.a_cred = cred;
-			error = (chandler->nhandler)(&ap);
+			error = clone_fn(&ap);
 			lockmgr(&devfs_lock, LK_SHARED);
+			/*
+			 * The chandler list may have changed while the lock
+			 * was released.  Do NOT dereference chandler->link;
+			 * restart the scan from the head.
+			 */
 			if (error)
-				continue;
+				goto again;
 
 			return ap.a_dev;
 		}

diff --git a/sys/net/tap/if_tap.c b/sys/net/tap/if_tap.c
index 0000000..1111111 100644
--- a/sys/net/tap/if_tap.c
+++ b/sys/net/tap/if_tap.c
@@ -191,13 +191,20 @@
 		if (taprefcnt > 0)
 			return (EBUSY);
 
+		/*
+		 * Remove the clone handler FIRST so that no new clone
+		 * callbacks can race against the teardown below.
+		 * devfs_clone_handler_del() is synchronous: by the time
+		 * destroy_autoclone_dev() returns the handler is gone.
+		 */
+		destroy_autoclone_dev(dev, &DEVFS_CLONE_BITMAP(tap));
+
 		if_clone_detach(&tap_cloner);
 
 		SLIST_FOREACH_MUTABLE(sc, &tap_listhead, tap_link, sc_tmp)
 			tapdestroy(sc);
 
 		dev_ops_remove_all(&tap_ops);
-		destroy_autoclone_dev(dev, &DEVFS_CLONE_BITMAP(tap));
 		break;
 
 	default: