DF-4671 / fix.diff
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 | 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. |