git: 4c9e89c85d7c - releng/14.4 - device_pager: Avoid double-insertion of pages into the pager list

From: Mark Johnston <markj_at_FreeBSD.org>
Date: Tue, 30 Jun 2026 17:20:54 UTC
The branch releng/14.4 has been updated by markj:

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

commit 4c9e89c85d7c6cf50b09865de4dacedd015c5f39
Author:     Mark Johnston <markj@FreeBSD.org>
AuthorDate: 2026-06-22 14:40:22 +0000
Commit:     Mark Johnston <markj@FreeBSD.org>
CommitDate: 2026-06-29 19:17:29 +0000

    device_pager: Avoid double-insertion of pages into the pager list
    
    Unmanaged device pager objects maintain a linked list of pages that were
    returned from the fault handler.  Initially, such pages are valid, but
    msync(MS_INVALIDATE) can mark them invalid (and clean).  They are not
    removed from page tables (since they are unmanaged), but a subsequent
    mlock() call can trigger a page fault that is handled by the pager.
    dev_pager_getpages() then re-inserts the page into the linked list even
    though it's already present there.
    
    This patch fixes the problem by removing the linked list.  OBJ_PG_DTOR
    is set, so vm_object_terminate_pages() does nothing, and
    dev_pager_dealloc() instead handles cleanup of the object.
    
    Add a regression test case which triggers a queue.h assertion failure in
    unpatched kernels.
    
    Note, in stable branches we should avoid changing the layout of struct
    vm_object.
    
    Approved by:    so
    Security:       FreeBSD-SA-26:37.vm
    Security:       CVE-2026-49418
    Reported by:    slidybat
    Reviewed by:    kib
    Sponsored by:   The FreeBSD Foundation
    Differential Revision:  https://reviews.freebsd.org/D57743
---
 sys/vm/device_pager.c    | 34 +++++++++++++++++-----------------
 sys/vm/vm_object.h       |  2 +-
 sys/vm/vm_page.c         |  6 ++++--
 tests/sys/vm/mmap_test.c | 30 ++++++++++++++++++++++++++++++
 4 files changed, 52 insertions(+), 20 deletions(-)

diff --git a/sys/vm/device_pager.c b/sys/vm/device_pager.c
index bceff38a41c3..b32d18fc3354 100644
--- a/sys/vm/device_pager.c
+++ b/sys/vm/device_pager.c
@@ -210,7 +210,8 @@ again:
 			object1 = NULL;
 			object->handle = handle;
 			object->un_pager.devp.ops = ops;
-			TAILQ_INIT(&object->un_pager.devp.devp_pglist);
+			if (object->type == OBJT_DEVICE)
+				vm_object_set_flag(object, OBJ_PG_DTOR);
 			TAILQ_INSERT_TAIL(&dev_pager_object_list, object,
 			    pager_object_list);
 			mtx_unlock(&dev_pager_mtx);
@@ -282,15 +283,12 @@ dev_pager_free_page(vm_object_t object, vm_page_t m)
 	KASSERT((object->type == OBJT_DEVICE &&
 	    (m->oflags & VPO_UNMANAGED) != 0),
 	    ("Managed device or page obj %p m %p", object, m));
-	TAILQ_REMOVE(&object->un_pager.devp.devp_pglist, m, plinks.q);
 	vm_page_putfake(m);
 }
 
 static void
 dev_pager_dealloc(vm_object_t object)
 {
-	vm_page_t m;
-
 	VM_OBJECT_WUNLOCK(object);
 	object->un_pager.devp.ops->cdev_pg_dtor(object->un_pager.devp.handle);
 
@@ -300,15 +298,21 @@ dev_pager_dealloc(vm_object_t object)
 	VM_OBJECT_WLOCK(object);
 
 	if (object->type == OBJT_DEVICE) {
-		/*
-		 * Free up our fake pages.
-		 */
-		while ((m = TAILQ_FIRST(&object->un_pager.devp.devp_pglist))
-		    != NULL) {
-			if (vm_page_busy_acquire(m, VM_ALLOC_WAITFAIL) == 0)
-				continue;
-
-			dev_pager_free_page(object, m);
+		vm_page_t m, mtmp;
+
+restart:
+		TAILQ_FOREACH_SAFE(m, &object->memq, listq, mtmp) {
+			if (!vm_page_busy_acquire(m, VM_ALLOC_WAITFAIL))
+				goto restart;
+			if (vm_page_remove(m)) {
+				/*
+				 * We could end up with invalid pages installed
+				 * by the generic page fault handler.  Typically
+				 * these are replaced by the device pager.
+				 */
+				vm_page_free(m);
+			} else if ((m->flags & PG_FICTITIOUS) != 0)
+				dev_pager_free_page(object, m);
 		}
 	}
 	object->handle = NULL;
@@ -337,10 +341,6 @@ dev_pager_getpages(vm_object_t object, vm_page_t *ma, int count, int *rbehind,
 		    (object->type == OBJT_MGTDEVICE &&
 		     (ma[0]->oflags & VPO_UNMANAGED) == 0),
 		    ("Wrong page type %p %p", ma[0], object));
-		if (object->type == OBJT_DEVICE) {
-			TAILQ_INSERT_TAIL(&object->un_pager.devp.devp_pglist,
-			    ma[0], plinks.q);
-		}
 		if (rbehind)
 			*rbehind = 0;
 		if (rahead)
diff --git a/sys/vm/vm_object.h b/sys/vm/vm_object.h
index 9a3bba4ef396..75a44fb66be4 100644
--- a/sys/vm/vm_object.h
+++ b/sys/vm/vm_object.h
@@ -137,7 +137,7 @@ struct vm_object {
 		 *	devp_pglist - list of allocated pages
 		 */
 		struct {
-			TAILQ_HEAD(, vm_page) devp_pglist;
+			void *spare[2];
 			const struct cdev_pager_ops *ops;
 			void *handle;
 		} devp;
diff --git a/sys/vm/vm_page.c b/sys/vm/vm_page.c
index b23c44ae083d..178b104ab95f 100644
--- a/sys/vm/vm_page.c
+++ b/sys/vm/vm_page.c
@@ -1301,8 +1301,10 @@ vm_page_putfake(vm_page_t m)
 	KASSERT((m->oflags & VPO_UNMANAGED) != 0, ("managed %p", m));
 	KASSERT((m->flags & PG_FICTITIOUS) != 0,
 	    ("vm_page_putfake: bad page %p", m));
-	vm_page_assert_xbusied(m);
-	vm_page_busy_free(m);
+	if (m->object != NULL) {
+		vm_page_assert_xbusied(m);
+		vm_page_busy_free(m);
+	}
 	uma_zfree(fakepg_zone, m);
 }
 
diff --git a/tests/sys/vm/mmap_test.c b/tests/sys/vm/mmap_test.c
index 6bc30f73ca95..a94fb56c46cf 100644
--- a/tests/sys/vm/mmap_test.c
+++ b/tests/sys/vm/mmap_test.c
@@ -362,6 +362,35 @@ ATF_TC_BODY(mmap__maxprot_shm, tc)
 	ATF_REQUIRE(close(fd) == 0);
 }
 
+/* A regression test for a bug in the device pager. */
+ATF_TC_WITHOUT_HEAD(mmap__device_reinsert);
+ATF_TC_BODY(mmap__device_reinsert, tc)
+{
+	void *p;
+	int fd;
+
+	fd = open("/dev/devstat", O_RDONLY);
+	ATF_REQUIRE(fd >= 0);
+
+	p = mmap(NULL, getpagesize(), PROT_READ, MAP_SHARED, fd, 0);
+	ATF_REQUIRE(p != MAP_FAILED);
+
+	/*
+	 * Use mlock() to trigger a fault, since msync() will not actually
+	 * remove the page from the process page tables.
+	 */
+	ATF_REQUIRE(mlock(p, getpagesize()) == 0);
+	ATF_REQUIRE(munlock(p, getpagesize()) == 0);
+	ATF_REQUIRE(msync(p, getpagesize(), MS_INVALIDATE) == 0);
+	ATF_REQUIRE(mlock(p, getpagesize()) == 0);
+	ATF_REQUIRE(munlock(p, getpagesize()) == 0);
+	ATF_REQUIRE(msync(p, getpagesize(), MS_INVALIDATE) == 0);
+	ATF_REQUIRE(mlock(p, getpagesize()) == 0);
+	ATF_REQUIRE(munlock(p, getpagesize()) == 0);
+
+	ATF_REQUIRE(munmap(p, getpagesize()) == 0);
+}
+
 ATF_TP_ADD_TCS(tp)
 {
 	ATF_TP_ADD_TC(tp, mmap__map_at_zero);
@@ -371,6 +400,7 @@ ATF_TP_ADD_TCS(tp)
 	ATF_TP_ADD_TC(tp, mmap__write_only);
 	ATF_TP_ADD_TC(tp, mmap__maxprot_basic);
 	ATF_TP_ADD_TC(tp, mmap__maxprot_shm);
+	ATF_TP_ADD_TC(tp, mmap__device_reinsert);
 
 	return (atf_no_error());
 }