DragonFlyBSD Kernel Audit
DF-0949 / fix.diff
← back to finding ↓ download raw
diff --git a/sys/vm/vm_mmap.c b/sys/vm/vm_mmap.c
--- a/sys/vm/vm_mmap.c
+++ b/sys/vm/vm_mmap.c
@@ -1049,7 +1049,6 @@
 	struct thread *td = curthread;
 	struct proc *p = td->td_proc;
 	vm_map_t map = &p->p_vmspace->vm_map;
-	vm_map_entry_t entry;
 	int how = uap->how;
 	int rc = KERN_SUCCESS;
 
@@ -1060,43 +1059,41 @@
 	if (rc) 
 		return (rc);
 
-	vm_map_lock(map);
-	do {
-		if (how & MCL_CURRENT) {
-			RB_FOREACH(entry, vm_map_rb_tree, &map->rb_root) {
-				/* Only writeable VM_MAPTYPE_NORMAL entries handled */
-				if ((entry->eflags & MAP_ENTRY_USER_WIRED) ||
-					entry->maptype != VM_MAPTYPE_NORMAL ||
-					(entry->max_protection & VM_PROT_WRITE) == 0) {
-					continue;
-				}
-
-				if (entry->wired_count != 0) {
-					entry->wired_count++;
-					entry->eflags |= MAP_ENTRY_USER_WIRED;
-					continue;
-				}
-
-				entry->wired_count++;
-				rc = vm_fault_wire(map, entry, TRUE, 0);
-				if (rc)
-					goto done;
-				entry->eflags |= MAP_ENTRY_USER_WIRED;
-			}
-		}
-		if (how & MCL_FUTURE)
-			map->flags |= MAP_WIREFUTURE;
-	} while(0);
-
-done:
-	RB_FOREACH(entry, vm_map_rb_tree, &map->rb_root) {
-		if (entry->eflags & MAP_ENTRY_USER_WIRED) {
-			entry->eflags &= ~MAP_ENTRY_USER_WIRED;
-			vm_fault_unwire(map, entry);
-		}
+	/*
+	 * DF-0949: the previous implementation walked the rb_root with
+	 * RB_FOREACH and called vm_fault_wire() per entry while holding
+	 * the map lock.  vm_fault_wire() (vm_fault.c:2624-2648) drops
+	 * and reacquires the map lock to fault pages in, but sys_mlockall
+	 * did not set MAP_ENTRY_IN_TRANSITION on the entry first, so a
+	 * concurrent munmap() in another thread could delete the entry
+	 * out from under us during that window.  On resume we wrote to
+	 * freed memory (`entry->eflags |= MAP_ENTRY_USER_WIRED`) and
+	 * RB_NEXT dereferenced stale rb_node pointers.
+	 *
+	 * Delegate to vm_map_user_wiring() for the MCL_CURRENT phase,
+	 * exactly like sys_mlock() does.  vm_map_user_wiring() uses
+	 * vm_map_clip_range()/vm_map_unclip_range() to set
+	 * MAP_ENTRY_IN_TRANSITION on the whole range atomically, which
+	 * makes vm_map_delete() sleep via vm_map_transition_wait()
+	 * instead of freeing entries we are iterating over.
+	 *
+	 * vm_map_user_wiring() takes the lock itself, so we must not
+	 * hold it across the call.  MCL_FUTURE is a separate flag on
+	 * the map and is handled under our own lock below.
+	 */
+	if (how & MCL_CURRENT) {
+		rc = vm_map_user_wiring(map,
+					VM_MIN_USER_ADDRESS,
+					VM_MAX_USER_ADDRESS, FALSE);
+		if (rc != KERN_SUCCESS)
+			return (rc);
 	}
 
-	vm_map_unlock(map);
+	if (how & MCL_FUTURE) {
+		vm_map_lock(map);
+		map->flags |= MAP_WIREFUTURE;
+		vm_map_unlock(map);
+	}
 
 	return (rc);
 }