From: Martin Pieuchot <mpi@grenadille.net>
Subject: uvm_fault_unwire() & truncation
To: tech@openbsd.org
Date: Mon, 21 Oct 2024 13:09:22 +0200

Syzkaller reports [0] a race in uvmspace_free() where the page `pg' given
to uvm_pageunwire() is not associated to the `entry'.

Diff below adapts NetBSD's 416427e0 that fixed a similar syzkaller report [2].

- in uvm_fault_check(), if the map entry is wired, handle the fault the same
  way that we would handle UVM_FAULT_WIRE.  faulting on wired mappings is valid
  if the mapped object was truncated and then later grown again.

- in uvm_fault_unwire_locked(), we must hold the locks for the vm_map_entry
  while calling pmap_extract() in order to avoid races with the mapped object
  being truncated while we are unwiring it.

Ok?

[0] https://syzkaller.appspot.com/bug?extid=78e55b3fe74e144f033b
[1] https://github.com/IIJ-NetBSD/netbsd-src/commit/416427e0
[2] https://syzkaller.appspot.com/bug?id=8840dce484094a926e1ec388ffb83acb2fa291c9

Index: uvm/uvm_fault.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
diff -u -p -r1.135 uvm_fault.c
--- uvm/uvm_fault.c	5 Sep 2023 05:08:26 -0000	1.135
+++ uvm/uvm_fault.c	21 Oct 2024 11:02:33 -0000
@@ -552,7 +552,7 @@ struct uvm_faultctx {
 
 int		uvm_fault_check(
 		    struct uvm_faultinfo *, struct uvm_faultctx *,
-		    struct vm_anon ***);
+		    struct vm_anon ***, vm_fault_t);
 
 int		uvm_fault_upper(
 		    struct uvm_faultinfo *, struct uvm_faultctx *,
@@ -585,11 +585,6 @@ uvm_fault(vm_map_t orig_map, vaddr_t vad
 	ufi.orig_map = orig_map;
 	ufi.orig_rvaddr = trunc_page(vaddr);
 	ufi.orig_size = PAGE_SIZE;	/* can't get any smaller than this */
-	if (fault_type == VM_FAULT_WIRE)
-		flt.narrow = TRUE;	/* don't look for neighborhood
-					 * pages on wire */
-	else
-		flt.narrow = FALSE;	/* normal fault */
 	flt.access_type = access_type;
 
 
@@ -597,7 +592,7 @@ uvm_fault(vm_map_t orig_map, vaddr_t vad
 	while (error == ERESTART) { /* ReFault: */
 		anons = anons_store;
 
-		error = uvm_fault_check(&ufi, &flt, &anons);
+		error = uvm_fault_check(&ufi, &flt, &anons, fault_type);
 		if (error != 0)
 			continue;
 
@@ -660,7 +655,7 @@ uvm_fault(vm_map_t orig_map, vaddr_t vad
  */
 int
 uvm_fault_check(struct uvm_faultinfo *ufi, struct uvm_faultctx *flt,
-    struct vm_anon ***ranons)
+    struct vm_anon ***ranons, vm_fault_t fault_type)
 {
 	struct vm_amap *amap;
 	struct uvm_object *uobj;
@@ -694,12 +689,14 @@ uvm_fault_check(struct uvm_faultinfo *uf
 	 * be more strict than ufi->entry->protection.  "wired" means either
 	 * the entry is wired or we are fault-wiring the pg.
 	 */
-
 	flt->enter_prot = ufi->entry->protection;
 	flt->pa_flags = UVM_ET_ISWC(ufi->entry) ? PMAP_WC : 0;
-	flt->wired = VM_MAPENT_ISWIRED(ufi->entry) || (flt->narrow == TRUE);
-	if (flt->wired)
+	if (VM_MAPENT_ISWIRED(ufi->entry) || (fault_type == VM_FAULT_WIRE)) {
+		flt->wired = TRUE;
 		flt->access_type = flt->enter_prot; /* full access for wired */
+		/*  don't look for neighborhood * pages on "wire" fault */
+		flt->narrow = TRUE;
+	}
 
 	* handle "needs_copy" case. */
 	if (UVM_ET_ISNEEDSCOPY(ufi->entry)) {
@@ -1654,9 +1651,6 @@ uvm_fault_unwire_locked(vm_map_t map, va
 		panic("uvm_fault_unwire_locked: address not in map");
 
 	for (va = start; va < end ; va += PAGE_SIZE) {
-		if (pmap_extract(pmap, va, &pa) == FALSE)
-			continue;
-
 		/*
 		 * find the map entry for the current address.
 		 */
@@ -1681,6 +1675,9 @@ uvm_fault_unwire_locked(vm_map_t map, va
 			uvm_map_lock_entry(entry);
 			oentry = entry;
 		}
+
+		if (!pmap_extract(pmap, va, &pa))
+			continue;
 
 		/*
 		 * if the entry is no longer wired, tell the pmap.