From: Martin Pieuchot <mpi@grenadille.net>
Subject: Re: VMMAP_DEBUG fix
To: David Higgs <higgsd@gmail.com>
Cc: OpenBSD Tech <tech@openbsd.org>
Date: Thu, 13 Mar 2025 10:16:44 +0100

On 10/03/25(Mon) 18:07, David Higgs wrote:
> On Mon, Mar 10, 2025 at 3:20 PM Martin Pieuchot <mpi@grenadille.net> wrote:
> 
> > Hello David,
> >
> > On 19/02/25(Wed) 21:11, David Higgs wrote:
> > > Is it possible that VMMAP_DEBUG is never enabled?  I think I found a
> > > bug in vmspace_validate that panics because it ignores anon mmap pages
> > > rather than counting them.
> >
> > VMMAP_DEBUG is indeed only enabled by people that hack on the code.
> >
> > I applied your diff, which fix the first panic, and now I get the
> > following,
> >  did you see it as well?  Do you have a fix?
> 
> 
> My project is very rudimentary and does not emulate the reaper or the
> kernel map. Unfortunately, I have not seen this (yet).
> 
> panic: uvm_tree_sanity 0xfffffd800780a720 (/sys/uvm/uvm_map.c 2504):
> > (iter->ety
> > pe & UVM_ET_FREEMAPPED) == 0
> > Stopped at      db_enter+0x14:  popq    %rbp
> >     TID    PID    UID     PRFLAGS     PFLAGS  CPU  COMMAND
> > *381413  34081      0     0x14000      0x200    0  reaper
> > db_enter() at db_enter+0x14
> > panic(ffffffff81d9b559) at panic+0xdd
> > uvm_tree_sanity(fffffd800780a720,ffffffff81d1062e,9c8) at
> > uvm_tree_sanity+0x428
> >
> > vm_map_unlock_ln(fffffd800780a720,ffffffff81d1062e,9c8) at
> > vm_map_unlock_ln+0x4
> > 5
> > uvm_map_teardown(fffffd800780a720) at uvm_map_teardown+0x231
> > uvmspace_free(fffffd800780a720) at uvmspace_free+0x5b
> > reaper(ffff8000fffff480) at reaper+0x198

This panic is fixed by destroy the address selectors after releasing the
lock.  This is fine because uvm_map_teardown() is called with the last
reference on the vm_map and the lock is there only to satisfy assertions. 

Here's the full diff I intended to commit.  With it enabling VMMAP_DEBUG
works again.

Ok?

Index: uvm/uvm_map.c
===================================================================
RCS file: /cvs/src/sys/uvm/uvm_map.c,v
diff -u -p -r1.338 uvm_map.c
--- uvm/uvm_map.c	29 Jan 2025 15:25:31 -0000	1.338
+++ uvm/uvm_map.c	13 Mar 2025 09:10:49 -0000
@@ -2460,16 +2460,6 @@ uvm_map_teardown(struct vm_map *map)
 
 	vm_map_lock(map);
 
-	/* Remove address selectors. */
-	uvm_addr_destroy(map->uaddr_exe);
-	map->uaddr_exe = NULL;
-	for (i = 0; i < nitems(map->uaddr_any); i++) {
-		uvm_addr_destroy(map->uaddr_any[i]);
-		map->uaddr_any[i] = NULL;
-	}
-	uvm_addr_destroy(map->uaddr_brk_stack);
-	map->uaddr_brk_stack = NULL;
-
 	/*
 	 * Remove entries.
 	 *
@@ -2503,6 +2493,16 @@ uvm_map_teardown(struct vm_map *map)
 
 	vm_map_unlock(map);
 
+	/* Remove address selectors. */
+	uvm_addr_destroy(map->uaddr_exe);
+	map->uaddr_exe = NULL;
+	for (i = 0; i < nitems(map->uaddr_any); i++) {
+		uvm_addr_destroy(map->uaddr_any[i]);
+		map->uaddr_any[i] = NULL;
+	}
+	uvm_addr_destroy(map->uaddr_brk_stack);
+	map->uaddr_brk_stack = NULL;
+
 #ifdef VMMAP_DEBUG
 	numt = numq = 0;
 	RBT_FOREACH(entry, uvm_map_addr, &map->addr)
@@ -2744,7 +2744,7 @@ vmspace_validate(struct vm_map *map)
 		imin = imax = iter->start;
 
 		if (UVM_ET_ISHOLE(iter) || iter->object.uvm_obj != NULL ||
-		    iter->protection != PROT_NONE)
+		    iter->protection == PROT_NONE)
 			continue;
 
 		/*