From dd82f6832612a03ad51f577b60d289fea4bee169 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Thu, 9 Sep 2021 11:36:40 +0200 Subject: [PATCH] Kernel: Use KString all the way in sys$execve() This patch converts all the usage of AK::String around sys$execve() to using KString instead, allowing us to catch and propagate OOM errors. It also required changing the kernel CommandLine helper class to return a vector of KString for the userspace init program arguments. --- Kernel/CommandLine.cpp | 13 +++++++---- Kernel/CommandLine.h | 4 +++- Kernel/Coredump.cpp | 4 ++-- Kernel/Process.cpp | 6 +++-- Kernel/Process.h | 14 +++++------ Kernel/Syscalls/execve.cpp | 48 +++++++++++++++++++------------------- 6 files changed, 48 insertions(+), 41 deletions(-) diff --git a/Kernel/CommandLine.cpp b/Kernel/CommandLine.cpp index 0e3cc065e7c..8e73d808efe 100644 --- a/Kernel/CommandLine.cpp +++ b/Kernel/CommandLine.cpp @@ -223,13 +223,16 @@ StringView CommandLine::userspace_init() const return lookup("init"sv).value_or("/bin/SystemServer"sv); } -Vector CommandLine::userspace_init_args() const +NonnullOwnPtrVector CommandLine::userspace_init_args() const { - auto init_args = lookup("init_args"sv).value_or(""sv).to_string().split(';'); - if (!init_args.is_empty()) - init_args.prepend(userspace_init()); + NonnullOwnPtrVector args; - return init_args; + auto init_args = lookup("init_args"sv).value_or(""sv).split_view(';'); + if (!init_args.is_empty()) + args.prepend(KString::must_create(userspace_init())); + for (auto& init_arg : init_args) + args.append(KString::must_create(init_arg)); + return args; } UNMAP_AFTER_INIT size_t CommandLine::switch_to_tty() const diff --git a/Kernel/CommandLine.h b/Kernel/CommandLine.h index ac22043cdc6..5e3b6b7a6e7 100644 --- a/Kernel/CommandLine.h +++ b/Kernel/CommandLine.h @@ -7,9 +7,11 @@ #pragma once #include +#include #include #include #include +#include namespace Kernel { @@ -76,7 +78,7 @@ public: [[nodiscard]] bool disable_virtio() const; [[nodiscard]] AHCIResetMode ahci_reset_mode() const; [[nodiscard]] StringView userspace_init() const; - [[nodiscard]] Vector userspace_init_args() const; + [[nodiscard]] NonnullOwnPtrVector userspace_init_args() const; [[nodiscard]] StringView root_device() const; [[nodiscard]] size_t switch_to_tty() const; diff --git a/Kernel/Coredump.cpp b/Kernel/Coredump.cpp index 7b938d97ef3..88300eb9324 100644 --- a/Kernel/Coredump.cpp +++ b/Kernel/Coredump.cpp @@ -194,13 +194,13 @@ KResult Coredump::create_notes_process_data(auto& builder) const { auto arguments_array = process_obj.add_array("arguments"sv); for (auto& argument : m_process->arguments()) - arguments_array.add(argument); + arguments_array.add(argument.view()); } { auto environment_array = process_obj.add_array("environment"sv); for (auto& variable : m_process->environment()) - environment_array.add(variable); + environment_array.add(variable.view()); } } diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index fb512ea5467..62014dfcc95 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -143,11 +143,13 @@ void Process::register_new(Process& process) }); } -KResultOr> Process::try_create_user_process(RefPtr& first_thread, StringView path, UserID uid, GroupID gid, Vector arguments, Vector environment, TTY* tty) +KResultOr> Process::try_create_user_process(RefPtr& first_thread, StringView path, UserID uid, GroupID gid, NonnullOwnPtrVector arguments, NonnullOwnPtrVector environment, TTY* tty) { auto parts = path.split_view('/'); if (arguments.is_empty()) { - arguments.append(parts.last()); + auto last_part = TRY(KString::try_create(parts.last())); + if (!arguments.try_append(move(last_part))) + return ENOMEM; } auto path_string = TRY(KString::try_create(path)); diff --git a/Kernel/Process.h b/Kernel/Process.h index b728792d852..4ed9107869c 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -179,7 +179,7 @@ public: } static RefPtr create_kernel_process(RefPtr& first_thread, NonnullOwnPtr name, void (*entry)(void*), void* entry_data = nullptr, u32 affinity = THREAD_AFFINITY_DEFAULT, RegisterProcess do_register = RegisterProcess::Yes); - static KResultOr> try_create_user_process(RefPtr& first_thread, StringView path, UserID, GroupID, Vector arguments, Vector environment, TTY*); + static KResultOr> try_create_user_process(RefPtr& first_thread, StringView path, UserID, GroupID, NonnullOwnPtrVector arguments, NonnullOwnPtrVector environment, TTY*); static void register_new(Process&); bool unref() const; @@ -438,10 +438,10 @@ public: Custody* executable() { return m_executable.ptr(); } const Custody* executable() const { return m_executable.ptr(); } - const Vector& arguments() const { return m_arguments; }; - const Vector& environment() const { return m_environment; }; + NonnullOwnPtrVector const& arguments() const { return m_arguments; }; + NonnullOwnPtrVector const& environment() const { return m_environment; }; - KResult exec(NonnullOwnPtr path, Vector arguments, Vector environment, int recusion_depth = 0); + KResult exec(NonnullOwnPtr path, NonnullOwnPtrVector arguments, NonnullOwnPtrVector environment, int recusion_depth = 0); KResultOr load(NonnullRefPtr main_program_description, RefPtr interpreter_description, const ElfW(Ehdr) & main_program_header); @@ -534,7 +534,7 @@ private: bool create_perf_events_buffer_if_needed(); void delete_perf_events_buffer(); - KResult do_exec(NonnullRefPtr main_program_description, Vector arguments, Vector environment, RefPtr interpreter_description, Thread*& new_main_thread, u32& prev_flags, const ElfW(Ehdr) & main_program_header); + KResult do_exec(NonnullRefPtr main_program_description, NonnullOwnPtrVector arguments, NonnullOwnPtrVector environment, RefPtr interpreter_description, Thread*& new_main_thread, u32& prev_flags, const ElfW(Ehdr) & main_program_header); KResultOr do_write(OpenFileDescription&, const UserOrKernelBuffer&, size_t); KResultOr do_statvfs(StringView path, statvfs* buf); @@ -763,8 +763,8 @@ private: RefPtr m_executable; RefPtr m_cwd; - Vector m_arguments; - Vector m_environment; + NonnullOwnPtrVector m_arguments; + NonnullOwnPtrVector m_environment; RefPtr m_tty; diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index 1c71e88b3d9..477c92464f1 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -42,7 +42,7 @@ struct LoadResult { static Vector generate_auxiliary_vector(FlatPtr load_base, FlatPtr entry_eip, UserID uid, UserID euid, GroupID gid, GroupID egid, StringView executable_path, Optional const& main_program_fd_allocation); -static bool validate_stack_size(const Vector& arguments, const Vector& environment) +static bool validate_stack_size(NonnullOwnPtrVector const& arguments, NonnullOwnPtrVector& environment) { size_t total_arguments_size = 0; size_t total_environment_size = 0; @@ -68,8 +68,8 @@ static bool validate_stack_size(const Vector& arguments, const Vector make_userspace_context_for_main_thread([[maybe_unused]] ThreadRegisters& regs, Memory::Region& region, Vector arguments, - Vector environment, Vector auxiliary_values) +static KResultOr make_userspace_context_for_main_thread([[maybe_unused]] ThreadRegisters& regs, Memory::Region& region, NonnullOwnPtrVector const& arguments, + NonnullOwnPtrVector const& environment, Vector auxiliary_values) { FlatPtr new_sp = region.range().end().get(); @@ -99,14 +99,14 @@ static KResultOr make_userspace_context_for_main_thread([[maybe_unused] Vector argv_entries; for (auto& argument : arguments) { - push_string_on_new_stack(argument); + push_string_on_new_stack(argument.view()); if (!argv_entries.try_append(new_sp)) return ENOMEM; } Vector env_entries; for (auto& variable : environment) { - push_string_on_new_stack(variable); + push_string_on_new_stack(variable.view()); if (!env_entries.try_append(new_sp)) return ENOMEM; } @@ -433,7 +433,7 @@ Process::load(NonnullRefPtr main_program_description, return interpreter_load_result; } -KResult Process::do_exec(NonnullRefPtr main_program_description, Vector arguments, Vector environment, +KResult Process::do_exec(NonnullRefPtr main_program_description, NonnullOwnPtrVector arguments, NonnullOwnPtrVector environment, RefPtr interpreter_description, Thread*& new_main_thread, u32& prev_flags, const ElfW(Ehdr) & main_program_header) { VERIFY(is_user_process()); @@ -514,8 +514,8 @@ KResult Process::do_exec(NonnullRefPtr main_program_descrip Memory::MemoryManager::enter_address_space(*m_space); m_executable = main_program_description->custody(); - m_arguments = arguments; - m_environment = environment; + m_arguments = move(arguments); + m_environment = move(environment); m_veil_state = VeilState::None; m_unveiled_paths.clear(); @@ -554,7 +554,7 @@ KResult Process::do_exec(NonnullRefPtr main_program_descrip // NOTE: We create the new stack before disabling interrupts since it will zero-fault // and we don't want to deal with faults after this point. - auto make_stack_result = make_userspace_context_for_main_thread(new_main_thread->regs(), *load_result.stack_region.unsafe_ptr(), move(arguments), move(environment), move(auxv)); + auto make_stack_result = make_userspace_context_for_main_thread(new_main_thread->regs(), *load_result.stack_region.unsafe_ptr(), m_arguments, m_environment, move(auxv)); if (make_stack_result.is_error()) return make_stack_result.error(); FlatPtr new_userspace_sp = make_stack_result.value(); @@ -672,12 +672,12 @@ static Vector generate_auxiliary_vector(FlatPtr load_base, return auxv; } -static KResultOr> find_shebang_interpreter_for_executable(char const first_page[], size_t nread) +static KResultOr> find_shebang_interpreter_for_executable(char const first_page[], size_t nread) { int word_start = 2; - int word_length = 0; + size_t word_length = 0; if (nread > 2 && first_page[0] == '#' && first_page[1] == '!') { - Vector interpreter_words; + NonnullOwnPtrVector interpreter_words; for (size_t i = 2; i < nread; ++i) { if (first_page[i] == '\n') { @@ -690,15 +690,18 @@ static KResultOr> find_shebang_interpreter_for_executable(char co if (first_page[i] == ' ') { if (word_length > 0) { - interpreter_words.append(String(&first_page[word_start], word_length)); + auto word = TRY(KString::try_create(StringView { &first_page[word_start], word_length })); + interpreter_words.append(move(word)); } word_length = 0; word_start = i + 1; } } - if (word_length > 0) - interpreter_words.append(String(&first_page[word_start], word_length)); + if (word_length > 0) { + auto word = TRY(KString::try_create(StringView { &first_page[word_start], word_length })); + interpreter_words.append(move(word)); + } if (!interpreter_words.is_empty()) return interpreter_words; @@ -772,7 +775,7 @@ KResultOr> Process::find_elf_interpreter_for_executa return nullptr; } -KResult Process::exec(NonnullOwnPtr path, Vector arguments, Vector environment, int recursion_depth) +KResult Process::exec(NonnullOwnPtr path, NonnullOwnPtrVector arguments, NonnullOwnPtrVector environment, int recursion_depth) { if (recursion_depth > 2) { dbgln("exec({}): SHENANIGANS! recursed too far trying to find #! interpreter", path); @@ -807,9 +810,8 @@ KResult Process::exec(NonnullOwnPtr path, Vector arguments, Vec auto shebang_result = find_shebang_interpreter_for_executable(first_page, nread); if (!shebang_result.is_error()) { auto shebang_words = shebang_result.release_value(); - auto shebang_path = TRY(KString::try_create(shebang_words.first())); - // FIXME: Don't use String. - arguments[0] = path->view(); + auto shebang_path = TRY(shebang_words.first().try_clone()); + arguments.ptr_at(0) = move(path); if (!arguments.try_prepend(move(shebang_words))) return ENOMEM; return exec(move(shebang_path), move(arguments), move(environment), ++recursion_depth); @@ -886,18 +888,16 @@ KResultOr Process::sys$execve(Userspaceview()); - if (!output.try_append(move(ak_string))) + if (!output.try_append(move(string))) return ENOMEM; } return KSuccess; }; - Vector arguments; + NonnullOwnPtrVector arguments; TRY(copy_user_strings(params.arguments, arguments)); - Vector environment; + NonnullOwnPtrVector environment; TRY(copy_user_strings(params.environment, environment)); auto result = exec(move(path), move(arguments), move(environment));