From cbaa3465a88256dec3700d77a591d07b41b9f5c0 Mon Sep 17 00:00:00 2001 From: Liav A Date: Sat, 9 Sep 2023 18:09:42 +0300 Subject: [PATCH] Kernel: Add jail semantics to methods iterating over thread lists We should consider whether the selected Thread is within the same jail or not. Therefore let's make it clear to callers with jail semantics if a called method checks if the desired Thread object is within the same jail. As for Thread::for_each_* methods, currently nothing in the kernel codebase needs iteration with consideration for jails, so the old Thread::for_each* were simply renamed to include "ignoring_jails" suffix in their names. --- Kernel/FileSystem/ProcFS/ProcessExposed.cpp | 2 +- Kernel/Syscalls/ptrace.cpp | 2 +- Kernel/Syscalls/sched.cpp | 2 +- Kernel/Tasks/Process.cpp | 2 +- Kernel/Tasks/Scheduler.cpp | 2 +- Kernel/Tasks/Thread.cpp | 22 +++++++++++++++++++-- Kernel/Tasks/Thread.h | 21 ++++++++++---------- 7 files changed, 36 insertions(+), 17 deletions(-) diff --git a/Kernel/FileSystem/ProcFS/ProcessExposed.cpp b/Kernel/FileSystem/ProcFS/ProcessExposed.cpp index 99793c38f7..389d6cc7e4 100644 --- a/Kernel/FileSystem/ProcFS/ProcessExposed.cpp +++ b/Kernel/FileSystem/ProcFS/ProcessExposed.cpp @@ -41,7 +41,7 @@ ErrorOr> Process::lookup_as_directory(ProcFS& procfs, Strin ErrorOr Process::procfs_get_thread_stack(ThreadID thread_id, KBufferBuilder& builder) const { auto array = TRY(JsonArraySerializer<>::try_create(builder)); - auto thread = Thread::from_tid(thread_id); + auto thread = Thread::from_tid_in_same_jail(thread_id); if (!thread) return ESRCH; auto current_process_credentials = Process::current().credentials(); diff --git a/Kernel/Syscalls/ptrace.cpp b/Kernel/Syscalls/ptrace.cpp index 5528e3b28c..2403ea2b46 100644 --- a/Kernel/Syscalls/ptrace.cpp +++ b/Kernel/Syscalls/ptrace.cpp @@ -34,7 +34,7 @@ static ErrorOr handle_ptrace(Kernel::Syscall::SC_ptrace_params const& p if (params.tid == caller.pid().value()) return EINVAL; - auto peer = Thread::from_tid(params.tid); + auto peer = Thread::from_tid_in_same_jail(params.tid); if (!peer) return ESRCH; diff --git a/Kernel/Syscalls/sched.cpp b/Kernel/Syscalls/sched.cpp index 12ed9d6e71..62cc1b7b26 100644 --- a/Kernel/Syscalls/sched.cpp +++ b/Kernel/Syscalls/sched.cpp @@ -27,7 +27,7 @@ ErrorOr> Process::get_thread_from_pid_or_tid(pid_t pid_or_ case Syscall::SchedulerParametersMode::Thread: { peer = Thread::current(); if (pid_or_tid != 0) - peer = Thread::from_tid(pid_or_tid); + peer = Thread::from_tid_in_same_jail(pid_or_tid); // Only superuser can access other processes' threads. if (!credentials()->is_superuser() && peer && &peer->process() != this) diff --git a/Kernel/Tasks/Process.cpp b/Kernel/Tasks/Process.cpp index 8c4ae838c2..e9ef93a187 100644 --- a/Kernel/Tasks/Process.cpp +++ b/Kernel/Tasks/Process.cpp @@ -952,7 +952,7 @@ ErrorOr Process::send_signal(u8 signal, Process* sender) { VERIFY(is_user_process()); // Try to send it to the "obvious" main thread: - auto receiver_thread = Thread::from_tid(pid().value()); + auto receiver_thread = Thread::from_tid_in_same_jail(pid().value()); // If the main thread has died, there may still be other threads: if (!receiver_thread) { // The first one should be good enough. diff --git a/Kernel/Tasks/Scheduler.cpp b/Kernel/Tasks/Scheduler.cpp index 86b42000fb..f60ad7915a 100644 --- a/Kernel/Tasks/Scheduler.cpp +++ b/Kernel/Tasks/Scheduler.cpp @@ -513,7 +513,7 @@ void dump_thread_list(bool with_stack_traces) return thread.get_register_dump_from_stack().ip(); }; - Thread::for_each([&](Thread& thread) { + Thread::for_each_ignoring_jails([&](Thread& thread) { auto color = thread.process().is_kernel_process() ? "\x1b[34;1m"sv : "\x1b[33;1m"sv; switch (thread.state()) { case Thread::State::Dying: diff --git a/Kernel/Tasks/Thread.cpp b/Kernel/Tasks/Thread.cpp index 813284fb5b..62685a18f2 100644 --- a/Kernel/Tasks/Thread.cpp +++ b/Kernel/Tasks/Thread.cpp @@ -589,7 +589,7 @@ void Thread::finalize_dying_threads() Vector dying_threads; { SpinlockLocker lock(g_scheduler_lock); - for_each_in_state(Thread::State::Dying, [&](Thread& thread) { + for_each_in_state_ignoring_jails(Thread::State::Dying, [&](Thread& thread) { if (!thread.is_finalizable()) return; auto result = dying_threads.try_append(&thread); @@ -1398,7 +1398,25 @@ ErrorOr Thread::make_thread_specific_region(Badge) }); } -RefPtr Thread::from_tid(ThreadID tid) +RefPtr Thread::from_tid_in_same_jail(ThreadID tid) +{ + return Thread::all_instances().with([&](auto& list) -> RefPtr { + for (Thread& thread : list) { + if (thread.tid() == tid) { + return Process::current().jail().with([&thread](auto const& my_jail) -> RefPtr { + return thread.process().jail().with([&thread, my_jail](auto const& other_thread_process_jail) -> RefPtr { + if (my_jail && my_jail.ptr() != other_thread_process_jail.ptr()) + return nullptr; + return thread; + }); + }); + } + } + return nullptr; + }); +} + +RefPtr Thread::from_tid_ignoring_jails(ThreadID tid) { return Thread::all_instances().with([&](auto& list) -> RefPtr { for (Thread& thread : list) { diff --git a/Kernel/Tasks/Thread.h b/Kernel/Tasks/Thread.h index cec430fe66..15f79f6bde 100644 --- a/Kernel/Tasks/Thread.h +++ b/Kernel/Tasks/Thread.h @@ -71,7 +71,8 @@ public: static ErrorOr> create(NonnullRefPtr); ~Thread(); - static RefPtr from_tid(ThreadID); + static RefPtr from_tid_ignoring_jails(ThreadID); + static RefPtr from_tid_in_same_jail(ThreadID); static void finalize_dying_threads(); ThreadID tid() const { return m_tid; } @@ -967,14 +968,14 @@ public: ErrorOr> clone(NonnullRefPtr); template Callback> - static IterationDecision for_each_in_state(State, Callback); + static IterationDecision for_each_in_state_ignoring_jails(State, Callback); template Callback> - static IterationDecision for_each(Callback); + static IterationDecision for_each_ignoring_jails(Callback); template Callback> - static IterationDecision for_each_in_state(State, Callback); + static IterationDecision for_each_in_state_ignoring_jails(State, Callback); template Callback> - static IterationDecision for_each(Callback); + static IterationDecision for_each_ignoring_jails(Callback); static constexpr u32 default_kernel_stack_size = 65536; static constexpr u32 default_userspace_stack_size = 1 * MiB; @@ -1264,7 +1265,7 @@ public: AK_ENUM_BITWISE_OPERATORS(Thread::FileBlocker::BlockFlags); template Callback> -inline IterationDecision Thread::for_each(Callback callback) +inline IterationDecision Thread::for_each_ignoring_jails(Callback callback) { return Thread::all_instances().with([&](auto& list) -> IterationDecision { for (auto& thread : list) { @@ -1277,7 +1278,7 @@ inline IterationDecision Thread::for_each(Callback callback) } template Callback> -inline IterationDecision Thread::for_each_in_state(State state, Callback callback) +inline IterationDecision Thread::for_each_in_state_ignoring_jails(State state, Callback callback) { return Thread::all_instances().with([&](auto& list) -> IterationDecision { for (auto& thread : list) { @@ -1292,7 +1293,7 @@ inline IterationDecision Thread::for_each_in_state(State state, Callback callbac } template Callback> -inline IterationDecision Thread::for_each(Callback callback) +inline IterationDecision Thread::for_each_ignoring_jails(Callback callback) { return Thread::all_instances().with([&](auto& list) { for (auto& thread : list) { @@ -1304,9 +1305,9 @@ inline IterationDecision Thread::for_each(Callback callback) } template Callback> -inline IterationDecision Thread::for_each_in_state(State state, Callback callback) +inline IterationDecision Thread::for_each_in_state_ignoring_jails(State state, Callback callback) { - return for_each_in_state(state, [&](auto& thread) { + return for_each_in_state_ignoring_jails(state, [&](auto& thread) { callback(thread); return IterationDecision::Continue; });