From: Martin Pieuchot <mpi@grenadille.net>
Subject: Missing lock in uvm_unmap_kill_entry()
To: tech@openbsd.org
Date: Tue, 4 Feb 2025 12:16:59 +0100

Syzkaller reported [1] an assertion when tearing down a VM space with wired
entries:

  panic: kernel diagnostic assertion "uvm_page_owner_locked_p(pg, TRUE)" failed: file "/syzkaller/managers/main/kernel/sys/uvm/uvm_page.c", line 1248
  Stopped at      db_enter+0x25:  addq    $0x8,%rsp
      TID    PID    UID     PRFLAGS     PFLAGS  CPU  COMMAND
  *153883   5762      0     0x14000      0x200    0  reaper
  db_enter() at db_enter+0x25 sys/arch/amd64/amd64/db_interface.c:437
  panic(ffffffff8311d783) at panic+0x1cf sys/kern/subr_prf.c:198
  __assert(ffffffff830d7289,ffffffff83025989,4e0,ffffffff8305b6f1) at __assert+0x29
  uvm_pageunwire(fffffd8006c5df00) at uvm_pageunwire+0x1dd sys/uvm/uvm_page.c:1248
  uvm_fault_unwire_locked(fffffd807aee3cb0,2a346811000,2a346c10000) at uvm_fault_unwire_locked+0x33c sys/uvm/uvm_fault.c:1774
  uvm_unmap_kill_entry_withlock(fffffd807aee3cb0,fffffd807ac2dd48,0) at uvm_unmap_kill_entry_withlock+0x81 sys/uvm/uvm_map.c:1860
  uvm_map_teardown(fffffd807aee3cb0) at uvm_map_teardown+0x1c7 sys/uvm/uvm_map.c:2496
  uvmspace_free(fffffd807aee3cb0) at uvmspace_free+0xbd sys/uvm/uvm_map.c:3420
  reaper(ffff80002a8376d0) at reaper+0x225 sys/kern/kern_exit.c:478

Wiring and unwiring a page are operations serialized by its owner lock.

Diff below prevents this panic by grabbing the lock before unwiring the
page.

ok?

[1] https://syzkaller.appspot.com/bug?extid=09beba1b131af8d8235e

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	4 Feb 2025 11:10:50 -0000
@@ -1854,14 +1854,14 @@ void
 uvm_unmap_kill_entry_withlock(struct vm_map *map, struct vm_map_entry *entry,
     int needlock)
 {
+	if (needlock)
+		uvm_map_lock_entry(entry);
+
 	/* Unwire removed map entry. */
 	if (VM_MAPENT_ISWIRED(entry)) {
 		entry->wired_count = 0;
 		uvm_fault_unwire_locked(map, entry->start, entry->end);
 	}
-
-	if (needlock)
-		uvm_map_lock_entry(entry);
 
 	/* Entry-type specific code. */
 	if (UVM_ET_ISHOLE(entry)) {