From: Martin Pieuchot <mpi@grenadille.net>
Subject: Re: uvm_fault_unwire() & truncation
To: Miod Vallat <miod@online.fr>
Cc: tech@openbsd.org
Date: Sat, 2 Nov 2024 10:51:07 +0100

On 02/11/24(Sat) 08:02, Miod Vallat wrote:
> > 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 */
> 
> You should keep a default initialization of flt.narrow to FALSE, or at
> least make sure uvm_fault_check() initializes it to FALSE in the
> non-narrow code paths (e.g. around line 700 where all the fields are
> initialized).

Fixed version below.

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	2 Nov 2024 08:24:15 -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,19 +585,14 @@ 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;
-
+	flt.narrow = FALSE;		/* normal fault */
 
 	error = ERESTART;
 	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.