From 55490b0115c4653a78bbcd99d48b987aafd98657 Mon Sep 17 00:00:00 2001 From: mpi Date: Fri, 4 Nov 2022 09:36:44 +0000 Subject: [PATCH] Assert the VM map lock is held in function used by mmap/mprotect/munmap. Also grab the lock in uvm_map_teardown() and uvm_map_deallocate() to satisfy the assertions. Grabbing the lock there shouldn't be strictly necessary, because no other reference to the map should exist when the reaper is holding it, but it doesn't hurt and makes our life easier. Inputs & tests from Ivo van der Sangen, tb@, gnezdo@, kn@ kettenis@ and tb@ agree with the direction, ok kn@ --- sys/uvm/uvm_addr.c | 4 +++- sys/uvm/uvm_fault.c | 3 ++- sys/uvm/uvm_map.c | 51 ++++++++++++++++++++++++++++++++++++++++----- sys/uvm/uvm_map.h | 10 ++++++++- 4 files changed, 60 insertions(+), 8 deletions(-) diff --git a/sys/uvm/uvm_addr.c b/sys/uvm/uvm_addr.c index c68a1beea05..6e3b02d5c09 100644 --- a/sys/uvm/uvm_addr.c +++ b/sys/uvm/uvm_addr.c @@ -1,4 +1,4 @@ -/* $OpenBSD: uvm_addr.c,v 1.31 2022/02/21 10:26:20 jsg Exp $ */ +/* $OpenBSD: uvm_addr.c,v 1.32 2022/11/04 09:36:44 mpi Exp $ */ /* * Copyright (c) 2011 Ariane van der Steldt @@ -416,6 +416,8 @@ uvm_addr_invoke(struct vm_map *map, struct uvm_addr_state *uaddr, !(hint >= uaddr->uaddr_minaddr && hint < uaddr->uaddr_maxaddr)) return ENOMEM; + vm_map_assert_anylock(map); + error = (*uaddr->uaddr_functions->uaddr_select)(map, uaddr, entry_out, addr_out, sz, align, offset, prot, hint); diff --git a/sys/uvm/uvm_fault.c b/sys/uvm/uvm_fault.c index 87d8252f936..4548410449a 100644 --- a/sys/uvm/uvm_fault.c +++ b/sys/uvm/uvm_fault.c @@ -1,4 +1,4 @@ -/* $OpenBSD: uvm_fault.c,v 1.132 2022/08/31 01:27:04 guenther Exp $ */ +/* $OpenBSD: uvm_fault.c,v 1.133 2022/11/04 09:36:44 mpi Exp $ */ /* $NetBSD: uvm_fault.c,v 1.51 2000/08/06 00:22:53 thorpej Exp $ */ /* @@ -1626,6 +1626,7 @@ uvm_fault_unwire_locked(vm_map_t map, vaddr_t start, vaddr_t end) struct vm_page *pg; KASSERT((map->flags & VM_MAP_INTRSAFE) == 0); + vm_map_assert_anylock(map); /* * we assume that the area we are unwiring has actually been wired diff --git a/sys/uvm/uvm_map.c b/sys/uvm/uvm_map.c index 1c6f9630413..9ee1fbd30f3 100644 --- a/sys/uvm/uvm_map.c +++ b/sys/uvm/uvm_map.c @@ -1,4 +1,4 @@ -/* $OpenBSD: uvm_map.c,v 1.302 2022/10/31 10:46:24 mpi Exp $ */ +/* $OpenBSD: uvm_map.c,v 1.303 2022/11/04 09:36:44 mpi Exp $ */ /* $NetBSD: uvm_map.c,v 1.86 2000/11/27 08:40:03 chs Exp $ */ /* @@ -491,6 +491,8 @@ uvmspace_dused(struct vm_map *map, vaddr_t min, vaddr_t max) vaddr_t stack_begin, stack_end; /* Position of stack. */ KASSERT(map->flags & VM_MAP_ISVMSPACE); + vm_map_assert_anylock(map); + vm = (struct vmspace *)map; stack_begin = MIN((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr); stack_end = MAX((vaddr_t)vm->vm_maxsaddr, (vaddr_t)vm->vm_minsaddr); @@ -570,6 +572,8 @@ uvm_map_isavail(struct vm_map *map, struct uvm_addr_state *uaddr, if (addr + sz < addr) return 0; + vm_map_assert_anylock(map); + /* * Kernel memory above uvm_maxkaddr is considered unavailable. */ @@ -1457,6 +1461,8 @@ uvm_map_mkentry(struct vm_map *map, struct vm_map_entry *first, entry->guard = 0; entry->fspace = 0; + vm_map_assert_wrlock(map); + /* Reset free space in first. */ free = uvm_map_uaddr_e(map, first); uvm_mapent_free_remove(map, free, first); @@ -1584,6 +1590,8 @@ boolean_t uvm_map_lookup_entry(struct vm_map *map, vaddr_t address, struct vm_map_entry **entry) { + vm_map_assert_anylock(map); + *entry = uvm_map_entrybyaddr(&map->addr, address); return *entry != NULL && !UVM_ET_ISHOLE(*entry) && (*entry)->start <= address && (*entry)->end > address; @@ -1704,6 +1712,8 @@ uvm_map_is_stack_remappable(struct vm_map *map, vaddr_t addr, vaddr_t sz, vaddr_t end = addr + sz; struct vm_map_entry *first, *iter, *prev = NULL; + vm_map_assert_anylock(map); + if (!uvm_map_lookup_entry(map, addr, &first)) { printf("map stack 0x%lx-0x%lx of map %p failed: no mapping\n", addr, end, map); @@ -1868,6 +1878,8 @@ uvm_mapent_mkfree(struct vm_map *map, struct vm_map_entry *entry, vaddr_t addr; /* Start of freed range. */ vaddr_t end; /* End of freed range. */ + UVM_MAP_REQ_WRITE(map); + prev = *prev_ptr; if (prev == entry) *prev_ptr = prev = NULL; @@ -1996,10 +2008,7 @@ uvm_unmap_remove(struct vm_map *map, vaddr_t start, vaddr_t end, if (start >= end) return 0; - if ((map->flags & VM_MAP_INTRSAFE) == 0) - splassert(IPL_NONE); - else - splassert(IPL_VM); + vm_map_assert_wrlock(map); /* Find first affected entry. */ entry = uvm_map_entrybyaddr(&map->addr, start); @@ -2531,6 +2540,8 @@ uvm_map_teardown(struct vm_map *map) KASSERT((map->flags & VM_MAP_INTRSAFE) == 0); + vm_map_lock(map); + /* Remove address selectors. */ uvm_addr_destroy(map->uaddr_exe); map->uaddr_exe = NULL; @@ -2572,6 +2583,8 @@ uvm_map_teardown(struct vm_map *map) entry = TAILQ_NEXT(entry, dfree.deadq); } + vm_map_unlock(map); + #ifdef VMMAP_DEBUG numt = numq = 0; RBT_FOREACH(entry, uvm_map_addr, &map->addr) @@ -4082,6 +4095,8 @@ uvm_map_checkprot(struct vm_map *map, vaddr_t start, vaddr_t end, { struct vm_map_entry *entry; + vm_map_assert_anylock(map); + if (start < map->min_offset || end > map->max_offset || start > end) return FALSE; if (start == end) @@ -4142,8 +4157,10 @@ uvm_map_deallocate(vm_map_t map) */ TAILQ_INIT(&dead); uvm_tree_sanity(map, __FILE__, __LINE__); + vm_map_lock(map); uvm_unmap_remove(map, map->min_offset, map->max_offset, &dead, TRUE, FALSE, FALSE); + vm_map_unlock(map); pmap_destroy(map->pmap); KASSERT(RBT_EMPTY(uvm_map_addr, &map->addr)); free(map, M_VMMAP, sizeof *map); @@ -4980,6 +4997,7 @@ uvm_map_freelist_update(struct vm_map *map, struct uvm_map_deadq *dead, vaddr_t b_start, vaddr_t b_end, vaddr_t s_start, vaddr_t s_end, int flags) { KDASSERT(b_end >= b_start && s_end >= s_start); + vm_map_assert_wrlock(map); /* Clear all free lists. */ uvm_map_freelist_update_clear(map, dead); @@ -5039,6 +5057,8 @@ uvm_map_fix_space(struct vm_map *map, struct vm_map_entry *entry, KDASSERT((entry != NULL && VMMAP_FREE_END(entry) == min) || min == map->min_offset); + UVM_MAP_REQ_WRITE(map); + /* * During the function, entfree will always point at the uaddr state * for entry. @@ -5404,6 +5424,27 @@ vm_map_unbusy_ln(struct vm_map *map, char *file, int line) wakeup(&map->flags); } +void +vm_map_assert_anylock_ln(struct vm_map *map, char *file, int line) +{ + LPRINTF(("map assert read or write locked: %p (at %s %d)\n", map, file, line)); + if ((map->flags & VM_MAP_INTRSAFE) == 0) + rw_assert_anylock(&map->lock); + else + MUTEX_ASSERT_LOCKED(&map->mtx); +} + +void +vm_map_assert_wrlock_ln(struct vm_map *map, char *file, int line) +{ + LPRINTF(("map assert write locked: %p (at %s %d)\n", map, file, line)); + if ((map->flags & VM_MAP_INTRSAFE) == 0) { + splassert(IPL_NONE); + rw_assert_wrlock(&map->lock); + } else + MUTEX_ASSERT_LOCKED(&map->mtx); +} + #ifndef SMALL_KERNEL int uvm_map_fill_vmmap(struct vm_map *map, struct kinfo_vmentry *kve, diff --git a/sys/uvm/uvm_map.h b/sys/uvm/uvm_map.h index 1b9a03fc3d1..fcfbadb9949 100644 --- a/sys/uvm/uvm_map.h +++ b/sys/uvm/uvm_map.h @@ -1,4 +1,4 @@ -/* $OpenBSD: uvm_map.h,v 1.79 2022/10/21 19:13:33 deraadt Exp $ */ +/* $OpenBSD: uvm_map.h,v 1.80 2022/11/04 09:36:44 mpi Exp $ */ /* $NetBSD: uvm_map.h,v 1.24 2001/02/18 21:19:08 chs Exp $ */ /* @@ -420,6 +420,8 @@ void vm_map_downgrade_ln(struct vm_map*, char*, int); void vm_map_upgrade_ln(struct vm_map*, char*, int); void vm_map_busy_ln(struct vm_map*, char*, int); void vm_map_unbusy_ln(struct vm_map*, char*, int); +void vm_map_assert_anylock_ln(struct vm_map*, char*, int); +void vm_map_assert_wrlock_ln(struct vm_map*, char*, int); #ifdef DIAGNOSTIC #define vm_map_lock_try(map) vm_map_lock_try_ln(map, __FILE__, __LINE__) @@ -431,6 +433,10 @@ void vm_map_unbusy_ln(struct vm_map*, char*, int); #define vm_map_upgrade(map) vm_map_upgrade_ln(map, __FILE__, __LINE__) #define vm_map_busy(map) vm_map_busy_ln(map, __FILE__, __LINE__) #define vm_map_unbusy(map) vm_map_unbusy_ln(map, __FILE__, __LINE__) +#define vm_map_assert_anylock(map) \ + vm_map_assert_anylock_ln(map, __FILE__, __LINE__) +#define vm_map_assert_wrlock(map) \ + vm_map_assert_wrlock_ln(map, __FILE__, __LINE__) #else #define vm_map_lock_try(map) vm_map_lock_try_ln(map, NULL, 0) #define vm_map_lock(map) vm_map_lock_ln(map, NULL, 0) @@ -441,6 +447,8 @@ void vm_map_unbusy_ln(struct vm_map*, char*, int); #define vm_map_upgrade(map) vm_map_upgrade_ln(map, NULL, 0) #define vm_map_busy(map) vm_map_busy_ln(map, NULL, 0) #define vm_map_unbusy(map) vm_map_unbusy_ln(map, NULL, 0) +#define vm_map_assert_anylock(map) vm_map_assert_anylock_ln(map, NULL, 0) +#define vm_map_assert_wrlock(map) vm_map_assert_wrlock_ln(map, NULL, 0) #endif void uvm_map_lock_entry(struct vm_map_entry *); -- 2.20.1