From 1d08b671ea2dd3186443b19232ec2d5215ebed59 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 23 Dec 2021 00:42:30 +0100 Subject: [PATCH] Kernel: Enter new address space before destroying old in sys$execve() Previously we were assigning to Process::m_space before actually entering the new address space (assigning it to CR3.) If a thread was preempted by the scheduler while destroying the old address space, we'd then attempt to resume the thread with CR3 pointing at a partially destroyed address space. We could then crash immediately in write_cr3(), right after assigning the new value to CR3. I am hopeful that this may have been the bug haunting our CI for months. :^) --- Kernel/Syscalls/execve.cpp | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index a825c9b72c..1ca443ba48 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -503,14 +503,11 @@ ErrorOr Process::do_exec(NonnullRefPtr main_program_d set_dumpable(!executable_is_setid); - { - // We must disable global profiling (especially kfree tracing) here because - // we might otherwise end up walking the stack into the process' space that - // is about to be destroyed. - TemporaryChange global_profiling_disabler(g_profiling_all_threads, false); - m_space = load_result.space.release_nonnull(); - } - Memory::MemoryManager::enter_address_space(*m_space); + // We make sure to enter the new address space before destroying the old one. + // This ensures that the process always has a valid page directory. + Memory::MemoryManager::enter_address_space(*load_result.space); + + m_space = load_result.space.release_nonnull(); m_executable = main_program_description->custody(); m_arguments = move(arguments);