DF-0949 / fix.diff
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); } |