⬢ DragonFlyBSD Kernel Audit
DF-5513 / fix.diff
← back to finding ↓ download raw
--- sys/bus/ppbus/ppbconf.c	2026-06-29 12:51:18.000000000 +0000
+++ findings/poc/DF-5510/fix_patched_ppbconf.c	2026-10-06 16:48:38.238897894 +0000
@@ -275,7 +275,18 @@
 	if (class)
 		/* identify class ident */
 		for (i = 0; pnp_tokens[i] != NULL; i++) {
-			if (search_token(class, len, pnp_tokens[i]) != NULL) {
+			/*
+			 * DF-5510: `class' is an interior pointer into
+			 * str[], so the scan length must be the length of
+			 * the class field only.  Passing the absolute
+			 * `len' here scans past the end of the 257-byte
+			 * stack object str[] (OOB read) and across the
+			 * following PnP fields (misclassification).
+			 */
+			int clen = 0;
+			while (class[clen] != '\0' && &class[clen] < &str[len])
+				clen++;
+			if (search_token(class, clen, pnp_tokens[i]) != NULL) {
 				class_id = i;
 				goto end_detect;
 			}
@@ -415,6 +426,13 @@
 	/* a device driver must own the bus to register an interrupt */
 	if (ppb->ppb_owner != child)
 		return (EINVAL);
+	/*
+	 * DF-5512: only one registration per device, otherwise the
+	 * cookie/resource bookkeeping (used to force unregistration on
+	 * bus release) silently loses the earlier registration.
+	 */
+	if (ppbdev->intr_cookie != NULL)
+		return (EBUSY);
 
 	if ((error = BUS_SETUP_INTR(device_get_parent(bus), child, r, flags,
 					ihand, arg, cookiep, serializer, NULL)))
@@ -463,6 +481,15 @@
 
 	while (!error) {
 		crit_enter();
+		/*
+		 * DF-5513: requesting the bus we already own must not
+		 * sleep (an owner re-request would wait for itself
+		 * forever; pcfclock_open() has no HAVEBUS guard).
+		 */
+		if (ppb->ppb_owner == dev) {
+			crit_exit();
+			return (0);
+		}
 		if (ppb->ppb_owner) {
 			crit_exit();
 			switch (how) {
@@ -525,15 +552,16 @@
 		crit_exit();
 		return (EACCES);
 	}
-	ppb->ppb_owner = 0;
-	crit_exit();
 
-	/* save the context of the device */
+	/* save the context of the device while we still own the bus, so a
+	 * device acquiring the bus right after the owner clear cannot
+	 * pollute our snapshot (DF-5511) */
 	ppbdev->ctx.mode = ppb_get_mode(bus);
-
-	/* ok, now the context of the device is valid */
 	ppbdev->ctx.valid = 1;
 
+	ppb->ppb_owner = 0;
+	crit_exit();
+
 	/* wakeup waiting processes */
 	wakeup(ppb);