diff --git a/Kernel/Syscall.cpp b/Kernel/Syscall.cpp index 4f37b67383d..38a6fa77f7c 100644 --- a/Kernel/Syscall.cpp +++ b/Kernel/Syscall.cpp @@ -199,39 +199,7 @@ NEVER_INLINE void syscall_handler(TrapFrame* trap) // NOTE: We take the big process lock before inspecting memory regions. process.big_lock().lock(); - VirtualAddress userspace_sp; -#if ARCH(I386) - userspace_sp = VirtualAddress { regs.userspace_esp }; -#else - userspace_sp = VirtualAddress { regs.userspace_rsp }; -#endif - if (!MM.validate_user_stack(process.space(), userspace_sp)) { - dbgln("Invalid stack pointer: {:p}", userspace_sp); - handle_crash(regs, "Bad stack on syscall entry", SIGSTKFLT); - } - - VirtualAddress ip; -#if ARCH(I386) - ip = VirtualAddress { regs.eip }; -#else - ip = VirtualAddress { regs.rip }; -#endif - - auto* calling_region = MM.find_user_region_from_vaddr(process.space(), ip); - if (!calling_region) { - dbgln("Syscall from {:p} which has no associated region", ip); - handle_crash(regs, "Syscall from unknown region", SIGSEGV); - } - - if (calling_region->is_writable()) { - dbgln("Syscall from writable memory at {:p}", ip); - handle_crash(regs, "Syscall from writable memory", SIGSEGV); - } - - if (process.space().enforces_syscall_regions() && !calling_region->is_syscall_region()) { - dbgln("Syscall from non-syscall region"); - handle_crash(regs, "Syscall from non-syscall region", SIGSEGV); - } + MM.validate_syscall_preconditions(process.space(), regs); FlatPtr function; FlatPtr arg1; diff --git a/Kernel/VM/MemoryManager.cpp b/Kernel/VM/MemoryManager.cpp index 30f8a54fc7c..46767ac923c 100644 --- a/Kernel/VM/MemoryManager.cpp +++ b/Kernel/VM/MemoryManager.cpp @@ -624,10 +624,55 @@ Region* MemoryManager::kernel_region_from_vaddr(VirtualAddress vaddr) return nullptr; } +Region* MemoryManager::find_user_region_from_vaddr_no_lock(Space& space, VirtualAddress vaddr) +{ + VERIFY(space.get_lock().own_lock()); + return space.find_region_containing({ vaddr, 1 }); +} + Region* MemoryManager::find_user_region_from_vaddr(Space& space, VirtualAddress vaddr) { ScopedSpinLock lock(space.get_lock()); - return space.find_region_containing({ vaddr, 1 }); + return find_user_region_from_vaddr_no_lock(space, vaddr); +} + +void MemoryManager::validate_syscall_preconditions(Space& space, RegisterState& regs) +{ + // We take the space lock once here and then use the no_lock variants + // to avoid excessive spinlock recursion in this extemely common path. + ScopedSpinLock lock(space.get_lock()); + + auto unlock_and_handle_crash = [&lock, ®s](const char* description, int signal) { + lock.unlock(); + handle_crash(regs, description, signal); + }; + + { + VirtualAddress userspace_sp = VirtualAddress { regs.userspace_sp() }; + if (!MM.validate_user_stack_no_lock(space, userspace_sp)) { + dbgln("Invalid stack pointer: {:p}", userspace_sp); + unlock_and_handle_crash("Bad stack on syscall entry", SIGSTKFLT); + } + } + + { + VirtualAddress ip = VirtualAddress { regs.ip() }; + auto* calling_region = MM.find_user_region_from_vaddr_no_lock(space, ip); + if (!calling_region) { + dbgln("Syscall from {:p} which has no associated region", ip); + unlock_and_handle_crash("Syscall from unknown region", SIGSEGV); + } + + if (calling_region->is_writable()) { + dbgln("Syscall from writable memory at {:p}", ip); + unlock_and_handle_crash("Syscall from writable memory", SIGSEGV); + } + + if (space.enforces_syscall_regions() && !calling_region->is_syscall_region()) { + dbgln("Syscall from non-syscall region"); + unlock_and_handle_crash("Syscall from non-syscall region", SIGSEGV); + } + } } Region* MemoryManager::find_region_from_vaddr(VirtualAddress vaddr) @@ -1039,7 +1084,7 @@ bool MemoryManager::validate_user_stack_no_lock(Space& space, VirtualAddress vad if (!is_user_address(vaddr)) return false; - auto* region = find_user_region_from_vaddr(space, vaddr); + auto* region = find_user_region_from_vaddr_no_lock(space, vaddr); return region && region->is_user() && region->is_stack(); } diff --git a/Kernel/VM/MemoryManager.h b/Kernel/VM/MemoryManager.h index 2e3acd1aa68..dbb31faae5b 100644 --- a/Kernel/VM/MemoryManager.h +++ b/Kernel/VM/MemoryManager.h @@ -189,6 +189,8 @@ public: } static Region* find_user_region_from_vaddr(Space&, VirtualAddress); + static Region* find_user_region_from_vaddr_no_lock(Space&, VirtualAddress); + static void validate_syscall_preconditions(Space&, RegisterState&); void dump_kernel_regions();