From: Martin Pieuchot <mpi@grenadille.net>
Subject: Re: uvm_fault_unwire() & truncation
To: Miod Vallat <miod@online.fr>, tech@openbsd.org
Date: Sun, 3 Nov 2024 14:33:32 +0100

On 02/11/24(Sat) 10:51, Martin Pieuchot wrote:
> 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.

Previous version did not initialize `flt.wired'.  This one includes the
fix.

Index: uvm/uvm_fault.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_fault.c,v
diff -u -p -r1.139 uvm_fault.c
--- uvm/uvm_fault.c	3 Nov 2024 11:33:32 -0000	1.139
+++ uvm/uvm_fault.c	3 Nov 2024 13:28:11 -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,15 @@ 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;		/* assume normal fault for now */
+	flt.wired = FALSE;		/* assume non-wired fault for now */
 
 	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 +656,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 +690,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)) {