From 2fd23745a90966c9a9a7cffb7e6963f176ce7ee2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?kleines=20Filmr=C3=B6llchen?= Date: Mon, 10 Jul 2023 00:20:48 +0200 Subject: [PATCH] Kernel: Allow relaxing cleanup task rules during system shutdown Once we move to a more proper shutdown procedure, processes other than the finalizer task must be able to perform cleanup and finalization duties, not only because the finalizer task itself needs to be cleaned up by someone. This global variable, mirroring the early boot flags, allows a future shutdown process to perform cleanup on its own. Note that while this *could* be considered a weakening in security, the attack surface is minimal and the results are not dramatic. To exploit this, an attacker would have to gain a Kernel write primitive to this global variable (bypassing KASLR among other things) and then gain some way of calling the relevant functions, all of this only to destroy some other running process. The same effect can be achieved with LPE which can often be gained with significantly simpler userspace exploits (e.g. of setuid binaries). --- Kernel/CMakeLists.txt | 1 + Kernel/Memory/AddressSpace.cpp | 5 ++++- Kernel/Tasks/PowerStateSwitchTask.cpp | 11 +++++++++++ Kernel/Tasks/PowerStateSwitchTask.h | 11 +++++++++++ Kernel/Tasks/Process.cpp | 12 +++++++++--- Kernel/Tasks/Thread.cpp | 4 +++- 6 files changed, 39 insertions(+), 5 deletions(-) create mode 100644 Kernel/Tasks/PowerStateSwitchTask.cpp create mode 100644 Kernel/Tasks/PowerStateSwitchTask.h diff --git a/Kernel/CMakeLists.txt b/Kernel/CMakeLists.txt index a0bc372404..6bbdb27fff 100644 --- a/Kernel/CMakeLists.txt +++ b/Kernel/CMakeLists.txt @@ -350,6 +350,7 @@ set(KERNEL_SOURCES Tasks/FinalizerTask.cpp Tasks/FutexQueue.cpp Tasks/PerformanceEventBuffer.cpp + Tasks/PowerStateSwitchTask.cpp Tasks/Process.cpp Tasks/ProcessGroup.cpp Tasks/ProcessList.cpp diff --git a/Kernel/Memory/AddressSpace.cpp b/Kernel/Memory/AddressSpace.cpp index 0d972cd1cc..9ea53b88a1 100644 --- a/Kernel/Memory/AddressSpace.cpp +++ b/Kernel/Memory/AddressSpace.cpp @@ -14,6 +14,7 @@ #include #include #include +#include #include #include @@ -323,7 +324,9 @@ void AddressSpace::dump_regions() void AddressSpace::remove_all_regions(Badge) { - VERIFY(Thread::current() == g_finalizer); + if (!g_in_system_shutdown) + VERIFY(Thread::current() == g_finalizer); + { SpinlockLocker pd_locker(m_page_directory->get_lock()); for (auto& region : m_region_tree.regions()) diff --git a/Kernel/Tasks/PowerStateSwitchTask.cpp b/Kernel/Tasks/PowerStateSwitchTask.cpp new file mode 100644 index 0000000000..9e1f9c25cd --- /dev/null +++ b/Kernel/Tasks/PowerStateSwitchTask.cpp @@ -0,0 +1,11 @@ +/* + * Copyright (c) 2023, kleines Filmröllchen + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +namespace Kernel { + +bool g_in_system_shutdown { false }; + +} diff --git a/Kernel/Tasks/PowerStateSwitchTask.h b/Kernel/Tasks/PowerStateSwitchTask.h new file mode 100644 index 0000000000..5f84a080be --- /dev/null +++ b/Kernel/Tasks/PowerStateSwitchTask.h @@ -0,0 +1,11 @@ +/* + * Copyright (c) 2023, kleines Filmröllchen + * + * SPDX-License-Identifier: BSD-2-Clause + */ + +namespace Kernel { + +extern bool g_in_system_shutdown; + +} diff --git a/Kernel/Tasks/Process.cpp b/Kernel/Tasks/Process.cpp index ebef656bc5..9834b7c98b 100644 --- a/Kernel/Tasks/Process.cpp +++ b/Kernel/Tasks/Process.cpp @@ -46,6 +46,7 @@ namespace Kernel { static void create_signal_trampoline(); extern ProcessID g_init_pid; +extern bool g_in_system_shutdown; RecursiveSpinlock g_profiling_lock {}; static Atomic next_pid; @@ -749,7 +750,8 @@ ErrorOr Process::dump_perfcore() void Process::finalize() { - VERIFY(Thread::current() == g_finalizer); + if (!g_in_system_shutdown) + VERIFY(Thread::current() == g_finalizer); dbgln_if(PROCESS_DEBUG, "Finalizing process {}", *this); @@ -759,8 +761,12 @@ void Process::finalize() }); } - if (g_init_pid != 0 && pid() == g_init_pid) - PANIC("Init process quit unexpectedly. Exit code: {}", termination_status()); + if (g_init_pid != 0 && pid() == g_init_pid) { + if (g_in_system_shutdown) + dbgln("Init process quitting for shutdown."); + else + PANIC("Init process quit unexpectedly. Exit code: {}", termination_status()); + } if (is_dumpable()) { if (m_should_generate_coredump) { diff --git a/Kernel/Tasks/Thread.cpp b/Kernel/Tasks/Thread.cpp index f0a9b6bf6b..c47ccc145a 100644 --- a/Kernel/Tasks/Thread.cpp +++ b/Kernel/Tasks/Thread.cpp @@ -23,6 +23,7 @@ #include #include #include +#include #include #include #include @@ -537,7 +538,8 @@ StringView Thread::state_string() const void Thread::finalize() { - VERIFY(Thread::current() == g_finalizer); + if (!g_in_system_shutdown) + VERIFY(Thread::current() == g_finalizer); VERIFY(Thread::current() != this); #if LOCK_DEBUG