From 0e08763483ba08db18994f5accbfb033c0d77b77 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 13 Jan 2022 23:17:52 +0100 Subject: [PATCH] Kernel: Wrap much of sys$execve() in a block scope Since we don't return normally from this function, let's make it a little extra difficult to accidentally leak something by leaving it on the stack in this function. --- Kernel/Syscalls/execve.cpp | 71 ++++++++++++++++++++------------------ 1 file changed, 38 insertions(+), 33 deletions(-) diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index 5446ec49c2..52fe068611 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -838,41 +838,46 @@ ErrorOr Process::sys$execve(Userspace VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this); TRY(require_promise(Pledge::exec)); - // NOTE: Be extremely careful with allocating any kernel memory in this function. - // On success, the kernel stack will be lost. - auto params = TRY(copy_typed_from_user(user_params)); - - if (params.arguments.length > ARG_MAX || params.environment.length > ARG_MAX) - return E2BIG; - - auto path = TRY(get_syscall_path_argument(params.path)); - - auto copy_user_strings = [](const auto& list, auto& output) -> ErrorOr { - if (!list.length) - return {}; - Checked size = sizeof(*list.strings); - size *= list.length; - if (size.has_overflow()) - return EOVERFLOW; - Vector strings; - TRY(strings.try_resize(list.length)); - TRY(copy_from_user(strings.data(), list.strings, size.value())); - for (size_t i = 0; i < list.length; ++i) { - auto string = TRY(try_copy_kstring_from_user(strings[i])); - TRY(output.try_append(move(string))); - } - return {}; - }; - - NonnullOwnPtrVector arguments; - TRY(copy_user_strings(params.arguments, arguments)); - - NonnullOwnPtrVector environment; - TRY(copy_user_strings(params.environment, environment)); - Thread* new_main_thread = nullptr; u32 prev_flags = 0; - TRY(exec(move(path), move(arguments), move(environment), new_main_thread, prev_flags)); + + // NOTE: Be extremely careful with allocating any kernel memory in this function. + // On success, the kernel stack will be lost. + // The explicit block scope below is specifically placed to minimize the number + // of stack locals in this function. + { + auto params = TRY(copy_typed_from_user(user_params)); + + if (params.arguments.length > ARG_MAX || params.environment.length > ARG_MAX) + return E2BIG; + + auto path = TRY(get_syscall_path_argument(params.path)); + + auto copy_user_strings = [](const auto& list, auto& output) -> ErrorOr { + if (!list.length) + return {}; + Checked size = sizeof(*list.strings); + size *= list.length; + if (size.has_overflow()) + return EOVERFLOW; + Vector strings; + TRY(strings.try_resize(list.length)); + TRY(copy_from_user(strings.data(), list.strings, size.value())); + for (size_t i = 0; i < list.length; ++i) { + auto string = TRY(try_copy_kstring_from_user(strings[i])); + TRY(output.try_append(move(string))); + } + return {}; + }; + + NonnullOwnPtrVector arguments; + TRY(copy_user_strings(params.arguments, arguments)); + + NonnullOwnPtrVector environment; + TRY(copy_user_strings(params.environment, environment)); + + TRY(exec(move(path), move(arguments), move(environment), new_main_thread, prev_flags)); + } // NOTE: If we're here, the exec has succeeded and we've got a new executable image! // We will not return normally from this function. Instead, the next time we