From ede1483e48aa99bb349455c9197dc9a90925793d Mon Sep 17 00:00:00 2001 From: Brian Gianforcaro Date: Fri, 14 May 2021 04:55:43 -0700 Subject: [PATCH] Kernel: Make Process creation APIs OOM safe This change looks more involved than it actually is. This simply reshuffles the previous Process constructor and splits out the parts which can fail (resource allocation) into separate methods which can be called from a factory method. The factory is then used everywhere instead of the constructor. --- Kernel/Process.cpp | 30 +++++++++++++++++++++++++----- Kernel/Process.h | 4 +++- Kernel/Syscalls/fork.cpp | 4 ++-- 3 files changed, 30 insertions(+), 8 deletions(-) diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 413e25e89e2..bc92dc94e46 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -144,7 +144,7 @@ RefPtr Process::create_user_process(RefPtr& first_thread, const if (!cwd) cwd = VFS::the().root_custody(); - auto process = adopt_ref(*new Process(first_thread, parts.take_last(), uid, gid, parent_pid, false, move(cwd), nullptr, tty)); + auto process = Process::create(first_thread, parts.take_last(), uid, gid, parent_pid, false, move(cwd), nullptr, tty); if (!first_thread) return {}; if (!process->m_fds.try_resize(m_max_open_file_descriptors)) { @@ -175,8 +175,8 @@ RefPtr Process::create_user_process(RefPtr& first_thread, const RefPtr Process::create_kernel_process(RefPtr& first_thread, String&& name, void (*entry)(void*), void* entry_data, u32 affinity) { - auto process = adopt_ref(*new Process(first_thread, move(name), (uid_t)0, (gid_t)0, ProcessID(0), true)); - if (!first_thread) + auto process = Process::create(first_thread, move(name), (uid_t)0, (gid_t)0, ProcessID(0), true); + if (!first_thread || !process) return {}; first_thread->tss().eip = (FlatPtr)entry; first_thread->tss().esp = FlatPtr(entry_data); // entry function argument is expected to be in tss.esp @@ -203,7 +203,18 @@ void Process::unprotect_data() MM.set_page_writable_direct(VirtualAddress { this }, true); } -Process::Process(RefPtr& first_thread, const String& name, uid_t uid, gid_t gid, ProcessID ppid, bool is_kernel_process, RefPtr cwd, RefPtr executable, TTY* tty, Process* fork_parent) +RefPtr Process::create(RefPtr& first_thread, const String& name, uid_t uid, gid_t gid, ProcessID ppid, bool is_kernel_process, RefPtr cwd, RefPtr executable, TTY* tty, Process* fork_parent) +{ + auto process = adopt_ref_if_nonnull(new Process(name, uid, gid, ppid, is_kernel_process, move(cwd), move(executable), tty)); + if (!process) + return {}; + auto result = process->attach_resources(first_thread, fork_parent); + if (result.is_error()) + return {}; + return process; +} + +Process::Process(const String& name, uid_t uid, gid_t gid, ProcessID ppid, bool is_kernel_process, RefPtr cwd, RefPtr executable, TTY* tty) : m_name(move(name)) , m_is_kernel_process(is_kernel_process) , m_executable(move(executable)) @@ -224,19 +235,28 @@ Process::Process(RefPtr& first_thread, const String& name, uid_t uid, gi m_sgid = gid; dbgln_if(PROCESS_DEBUG, "Created new process {}({})", m_name, this->pid().value()); +} +KResult Process::attach_resources(RefPtr& first_thread, Process* fork_parent) +{ m_space = Space::create(*this, fork_parent ? &fork_parent->space() : nullptr); + if (!m_space) + return ENOMEM; if (fork_parent) { // NOTE: fork() doesn't clone all threads; the thread that called fork() becomes the only thread in the new process. first_thread = Thread::current()->clone(*this); + if (!first_thread) + return ENOMEM; } else { // NOTE: This non-forked code path is only taken when the kernel creates a process "manually" (at boot.) auto thread_or_error = Thread::try_create(*this); - VERIFY(!thread_or_error.is_error()); + if (thread_or_error.is_error()) + return thread_or_error.error(); first_thread = thread_or_error.release_value(); first_thread->detach(); } + return KSuccess; } Process::~Process() diff --git a/Kernel/Process.h b/Kernel/Process.h index 7a8881e37e3..5ffcc47c2b2 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -499,7 +499,9 @@ private: bool add_thread(Thread&); bool remove_thread(Thread&); - Process(RefPtr& first_thread, const String& name, uid_t, gid_t, ProcessID ppid, bool is_kernel_process, RefPtr cwd = nullptr, RefPtr executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr); + Process(const String& name, uid_t uid, gid_t gid, ProcessID ppid, bool is_kernel_process, RefPtr cwd, RefPtr executable, TTY* tty); + static RefPtr create(RefPtr& first_thread, const String& name, uid_t, gid_t, ProcessID ppid, bool is_kernel_process, RefPtr cwd = nullptr, RefPtr executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr); + KResult attach_resources(RefPtr& first_thread, Process* fork_parent); static ProcessID allocate_pid(); void kill_threads_except_self(); diff --git a/Kernel/Syscalls/fork.cpp b/Kernel/Syscalls/fork.cpp index 274d449f2d3..5da7afc28f6 100644 --- a/Kernel/Syscalls/fork.cpp +++ b/Kernel/Syscalls/fork.cpp @@ -17,8 +17,8 @@ KResultOr Process::sys$fork(RegisterState& regs) { REQUIRE_PROMISE(proc); RefPtr child_first_thread; - auto child = adopt_ref(*new Process(child_first_thread, m_name, uid(), gid(), pid(), m_is_kernel_process, m_cwd, m_executable, m_tty, this)); - if (!child_first_thread) + auto child = Process::create(child_first_thread, m_name, uid(), gid(), pid(), m_is_kernel_process, m_cwd, m_executable, m_tty, this); + if (!child || !child_first_thread) return ENOMEM; child->m_root_directory = m_root_directory; child->m_root_directory_relative_to_global_root = m_root_directory_relative_to_global_root;