⬢ DragonFlyBSD Kernel Audit
DF-4671 / fix.diff
← back to finding ↓ download raw
fix.diff for DF-4671 / DF-4672 / DF-4673 (authored after on-guest
verification; applies with `patch -p1` from the repo root to
sys/dev/video/bktr/{bktr_core.c,bktr_os.c})

Root cause: the METEORSETGEO grow block (bktr_core.c:1664-1681) releases
the contigmalloc'd frame buffer with a bare kmem_free() -- for the
object-less pmap_kenter'd entry that neither returns the wired pages to
the VM system nor to the low-memory DMA-reserve alist, nor removes the
kernel PTEs (sys/vm/vm_kern.c:311-313 warns about exactly this misuse);
installs the new buffer UNZEROED (the attach path bzero()s,
bktr_core.c:432); and swaps bigbuf/alloc_pages with no synchronization
against bktr_mmap (bktr_os.c:680-683), which the device pager
re-executes on first fault of every page index
(sys/vm/device_pager.c:348-390).

--- a/sys/dev/video/bktr/bktr_core.c	2026-10-15 00:47:09.153563126 +0000
+++ b/sys/dev/video/bktr/bktr_core.c	2026-10-15 00:51:29.146211265 +0000
@@ -98,6 +98,7 @@
 #include <sys/proc.h>
 #include <sys/signalvar.h>
 #include <sys/vnode.h>
+#include <sys/malloc.h>	/* DF-4672 fix: contigfree/M_DEVBUF */
 #include <sys/bus.h>		/* used by smbus and newbus */
 #include <sys/thread2.h>
 
@@ -1663,14 +1664,40 @@
 			temp = btoc(temp);
 			if ((int) temp > bktr->alloc_pages
 			    && bktr->video.addr == 0) {
+				vm_offset_t obuf;
+				int opgs;
 
 				buf = get_bktr_mem(unit, temp*PAGE_SIZE);
 				if (buf != 0) {
-					kmem_free(kernel_map, bktr->bigbuf,
-					    (bktr->alloc_pages * PAGE_SIZE));
-
+					/*
+					 * DF-4672: contigmalloc'd memory must
+					 * be released with contigfree(); a
+					 * bare kmem_free() leaks the wired
+					 * pages and the DMA-reserve alist
+					 * block and leaves stale kernel
+					 * PTEs (vm_kern.c:311-313).
+					 *
+					 * DF-4671: install the new buffer
+					 * BEFORE freeing the old one so the
+					 * unlocked bktr_mmap fault path can
+					 * never observe a freed KVA in
+					 * bigbuf.
+					 *
+					 * DF-4673: the attach path zeroes
+					 * the initial buffer (bktr_core.c
+					 * :432); zero the grown buffer too
+					 * or recycled PQ_FREE/PQ_CACHE
+					 * pages disclose stale kernel
+					 * memory via bktr_mmap.
+					 */
+					obuf = bktr->bigbuf;
+					opgs = bktr->alloc_pages;
 					bktr->bigbuf = buf;
 					bktr->alloc_pages = temp;
+					bzero((caddr_t) buf, temp * PAGE_SIZE);
+					if (obuf != 0 && opgs != 0)
+						contigfree((void *) obuf,
+						    opgs * PAGE_SIZE, M_DEVBUF);
 					if (bootverbose)
 						kprintf(
 				"%s: ioctl: Allocating %d bytes\n",
--- a/sys/dev/video/bktr/bktr_os.c	2026-10-15 00:47:09.153563126 +0000
+++ b/sys/dev/video/bktr/bktr_os.c	2026-10-15 00:47:09.173562869 +0000
@@ -680,7 +680,39 @@
 	if (ap->a_offset >= bktr->alloc_pages * PAGE_SIZE)
 		return(EINVAL);
 
-	ap->a_result = atop(vtophys(bktr->bigbuf) + ap->a_offset);
+	/*
+	 * DF-4671: the METEORSETGEO grow path swaps bigbuf/alloc_pages with
+	 * no synchronization against this fault-time re-query (the device
+	 * pager re-executes d_mmap on the first fault of every page index,
+	 * device_pager.c old_dev_pager_fault).  Sample the pair
+	 * consistently and re-check the bound against the sampled pair so a
+	 * fault never computes vtophys() through a bigbuf sampled from a
+	 * different generation than alloc_pages.
+	 */
+	{
+		vm_offset_t buf;
+		vm_offset_t pa;
+		int npages;
+
+		do {
+			buf = bktr->bigbuf;
+			npages = bktr->alloc_pages;
+		} while (buf != bktr->bigbuf ||
+			 npages != bktr->alloc_pages);
+		if (ap->a_offset >= (vm_offset_t)npages * PAGE_SIZE)
+			return (EINVAL);
+
+		/*
+		 * Fail closed: if the buffer was released between the
+		 * consistent sample and here, contigfree() has torn the
+		 * PTEs down and vtophys() returns 0; mapping atop(0+offset)
+		 * would alias low physical memory.
+		 */
+		pa = vtophys(buf + ap->a_offset);
+		if (pa == 0)
+			return (EINVAL);
+		ap->a_result = atop(pa);
+	}
 	return(0);
 }
 

Residual (documented, out of scope for this diff): mappings established
before a buffer replacement keep aliasing the old pages afterwards (the
old-style device pager pins nothing; verified on-guest with
poc_race --holdcheck: a held RW mapping stayed a live alias onto the
orphaned pages across 10 grows).  Fully closing that requires refcounted
buffer lifetimes freed on last unmap (vm_object references), tracked as
follow-up work.  With contigfree() the released pages at least return to
the VM allocator once the pager object dies with its cdev.