From 9d6372ff07b55cb41878bca40429417d55dc98ad Mon Sep 17 00:00:00 2001 From: Tim Schumacher Date: Mon, 10 Jul 2023 11:13:13 +0200 Subject: [PATCH] Kernel: Consolidate finding the ELF stack size with validation Previously, we started parsing the ELF file again in a completely different place, and without the partial mapping that we do while validating. Instead of doing manual parsing in two places, just capture the requested stack size right after we validated it. --- Kernel/Syscalls/execve.cpp | 63 ++++++++---------------- Kernel/Tasks/Process.h | 6 +-- Userland/Libraries/LibELF/Image.cpp | 2 +- Userland/Libraries/LibELF/Validation.cpp | 5 +- Userland/Libraries/LibELF/Validation.h | 2 +- 5 files changed, 29 insertions(+), 49 deletions(-) diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index 93363ca768..e818c0df74 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -286,6 +286,9 @@ static ErrorOr load_elf_object(Memory::AddressSpace& new_space, Open FlatPtr load_base_address = 0; size_t stack_size = Thread::default_userspace_stack_size; + if (minimum_stack_size.has_value() && minimum_stack_size.value() > stack_size) + stack_size = minimum_stack_size.value(); + auto elf_name = TRY(object_description.pseudo_path()); VERIFY(!Processor::in_critical()); @@ -380,13 +383,6 @@ static ErrorOr load_elf_object(Memory::AddressSpace& new_space, Open if (program_header.type() == PT_LOAD) return load_section(program_header); - if (program_header.type() == PT_GNU_STACK) { - auto new_stack_size = program_header.size_in_memory(); - - if (new_stack_size > stack_size) - stack_size = new_stack_size; - } - // NOTE: We ignore other program header types. return {}; }; @@ -405,9 +401,6 @@ static ErrorOr load_elf_object(Memory::AddressSpace& new_space, Open return ENOEXEC; } - if (minimum_stack_size.has_value() && minimum_stack_size.value() > stack_size) - stack_size = minimum_stack_size.value(); - auto* stack_region = TRY(new_space.allocate_region(Memory::RandomizeVirtualAddress::Yes, {}, stack_size, PAGE_SIZE, "Stack (Main thread)"sv, PROT_READ | PROT_WRITE, AllocationStrategy::Reserve)); stack_region->set_stack(true); @@ -424,42 +417,19 @@ static ErrorOr load_elf_object(Memory::AddressSpace& new_space, Open ErrorOr Process::load(Memory::AddressSpace& new_space, NonnullRefPtr main_program_description, - RefPtr interpreter_description, const ElfW(Ehdr) & main_program_header) + RefPtr interpreter_description, const ElfW(Ehdr) & main_program_header, Optional minimum_stack_size) { auto load_offset = TRY(get_load_offset(main_program_header, main_program_description, interpreter_description)); if (interpreter_description.is_null()) { - auto load_result = TRY(load_elf_object(new_space, main_program_description, load_offset, ShouldAllocateTls::Yes, ShouldAllowSyscalls::No)); + auto load_result = TRY(load_elf_object(new_space, main_program_description, load_offset, ShouldAllocateTls::Yes, ShouldAllowSyscalls::No, minimum_stack_size)); m_master_tls_region = load_result.tls_region; m_master_tls_size = load_result.tls_size; m_master_tls_alignment = load_result.tls_alignment; return load_result; } - Optional requested_main_program_stack_size; - { - auto main_program_size = main_program_description->inode()->size(); - auto main_program_rounded_size = TRY(Memory::page_round_up(main_program_size)); - - auto main_program_vmobject = TRY(Memory::SharedInodeVMObject::try_create_with_inode(*main_program_description->inode())); - auto main_program_region = TRY(MM.allocate_kernel_region_with_vmobject(*main_program_vmobject, main_program_rounded_size, "Loaded Main Program ELF"sv, Memory::Region::Access::Read)); - - auto main_program_image = ELF::Image(main_program_region->vaddr().as_ptr(), main_program_size); - if (!main_program_image.is_valid()) - return EINVAL; - - main_program_image.for_each_program_header([&requested_main_program_stack_size](ELF::Image::ProgramHeader const& program_header) { - if (program_header.type() != PT_GNU_STACK) - return; - - if (program_header.size_in_memory() == 0) - return; - - requested_main_program_stack_size = program_header.size_in_memory(); - }); - } - - auto interpreter_load_result = TRY(load_elf_object(new_space, *interpreter_description, load_offset, ShouldAllocateTls::No, ShouldAllowSyscalls::Yes, requested_main_program_stack_size)); + auto interpreter_load_result = TRY(load_elf_object(new_space, *interpreter_description, load_offset, ShouldAllocateTls::No, ShouldAllowSyscalls::Yes, minimum_stack_size)); // TLS allocation will be done in userspace by the loader VERIFY(!interpreter_load_result.tls_region); @@ -489,7 +459,7 @@ void Process::clear_signal_handlers_for_exec() } ErrorOr Process::do_exec(NonnullRefPtr main_program_description, Vector> arguments, Vector> environment, - RefPtr interpreter_description, Thread*& new_main_thread, InterruptsState& previous_interrupts_state, const ElfW(Ehdr) & main_program_header) + RefPtr interpreter_description, Thread*& new_main_thread, InterruptsState& previous_interrupts_state, const ElfW(Ehdr) & main_program_header, Optional minimum_stack_size) { VERIFY(is_user_process()); VERIFY(!Processor::in_critical()); @@ -537,7 +507,7 @@ ErrorOr Process::do_exec(NonnullRefPtr main_program_d Memory::MemoryManager::enter_process_address_space(*this); }); - auto load_result = TRY(load(new_space, main_program_description, interpreter_description, main_program_header)); + auto load_result = TRY(load(new_space, main_program_description, interpreter_description, main_program_header, minimum_stack_size)); // NOTE: We don't need the interpreter executable description after this point. // We destroy it here to prevent it from getting destroyed when we return from this function. @@ -826,15 +796,18 @@ static ErrorOr>> find_shebang_interpreter_for_exec return ENOEXEC; } -ErrorOr> Process::find_elf_interpreter_for_executable(StringView path, ElfW(Ehdr) const& main_executable_header, size_t main_executable_header_size, size_t file_size) +ErrorOr> Process::find_elf_interpreter_for_executable(StringView path, ElfW(Ehdr) const& main_executable_header, size_t main_executable_header_size, size_t file_size, Optional& minimum_stack_size) { // Not using ErrorOr here because we'll want to do the same thing in userspace in the RTLD StringBuilder interpreter_path_builder; - if (!TRY(ELF::validate_program_headers(main_executable_header, file_size, { &main_executable_header, main_executable_header_size }, &interpreter_path_builder))) { + Optional main_executable_requested_stack_size {}; + if (!TRY(ELF::validate_program_headers(main_executable_header, file_size, { &main_executable_header, main_executable_header_size }, &interpreter_path_builder, &main_executable_requested_stack_size))) { dbgln("exec({}): File has invalid ELF Program headers", path); return ENOEXEC; } auto interpreter_path = interpreter_path_builder.string_view(); + if (main_executable_requested_stack_size.has_value() && (!minimum_stack_size.has_value() || *minimum_stack_size < *main_executable_requested_stack_size)) + minimum_stack_size = main_executable_requested_stack_size; if (!interpreter_path.is_empty()) { dbgln_if(EXEC_DEBUG, "exec({}): Using program interpreter {}", path, interpreter_path); @@ -863,11 +836,14 @@ ErrorOr> Process::find_elf_interpreter_for_executabl // Not using ErrorOr here because we'll want to do the same thing in userspace in the RTLD StringBuilder interpreter_interpreter_path_builder; - if (!TRY(ELF::validate_program_headers(*elf_header, interp_metadata.size, { first_page, nread }, &interpreter_interpreter_path_builder))) { + Optional interpreter_requested_stack_size {}; + if (!TRY(ELF::validate_program_headers(*elf_header, interp_metadata.size, { first_page, nread }, &interpreter_interpreter_path_builder, &interpreter_requested_stack_size))) { dbgln("exec({}): Interpreter ({}) has invalid ELF Program headers", path, interpreter_path); return ENOEXEC; } auto interpreter_interpreter_path = interpreter_interpreter_path_builder.string_view(); + if (interpreter_requested_stack_size.has_value() && (!minimum_stack_size.has_value() || *minimum_stack_size < *interpreter_requested_stack_size)) + minimum_stack_size = interpreter_requested_stack_size; if (!interpreter_interpreter_path.is_empty()) { dbgln("exec({}): Interpreter ({}) has its own interpreter ({})! No thank you!", path, interpreter_path, interpreter_interpreter_path); @@ -945,8 +921,9 @@ ErrorOr Process::exec(NonnullOwnPtr path, Vectorview(), *main_program_header, nread, metadata.size)); - return do_exec(move(description), move(arguments), move(environment), move(interpreter_description), new_main_thread, previous_interrupts_state, *main_program_header); + Optional minimum_stack_size {}; + auto interpreter_description = TRY(find_elf_interpreter_for_executable(path->view(), *main_program_header, nread, metadata.size, minimum_stack_size)); + return do_exec(move(description), move(arguments), move(environment), move(interpreter_description), new_main_thread, previous_interrupts_state, *main_program_header, minimum_stack_size); } ErrorOr Process::sys$execve(Userspace user_params) diff --git a/Kernel/Tasks/Process.h b/Kernel/Tasks/Process.h index fb42f7795c..c1331dfb00 100644 --- a/Kernel/Tasks/Process.h +++ b/Kernel/Tasks/Process.h @@ -494,7 +494,7 @@ public: ErrorOr exec(NonnullOwnPtr path, Vector> arguments, Vector> environment, Thread*& new_main_thread, InterruptsState& previous_interrupts_state, int recursion_depth = 0); - ErrorOr load(Memory::AddressSpace& new_space, NonnullRefPtr main_program_description, RefPtr interpreter_description, const ElfW(Ehdr) & main_program_header); + ErrorOr load(Memory::AddressSpace& new_space, NonnullRefPtr main_program_description, RefPtr interpreter_description, const ElfW(Ehdr) & main_program_header, Optional minimum_stack_size = {}); void terminate_due_to_signal(u8 signal); ErrorOr send_signal(u8 signal, Process* sender); @@ -624,12 +624,12 @@ private: bool create_perf_events_buffer_if_needed(); void delete_perf_events_buffer(); - ErrorOr do_exec(NonnullRefPtr main_program_description, Vector> arguments, Vector> environment, RefPtr interpreter_description, Thread*& new_main_thread, InterruptsState& previous_interrupts_state, const ElfW(Ehdr) & main_program_header); + ErrorOr do_exec(NonnullRefPtr main_program_description, Vector> arguments, Vector> environment, RefPtr interpreter_description, Thread*& new_main_thread, InterruptsState& previous_interrupts_state, const ElfW(Ehdr) & main_program_header, Optional minimum_stack_size = {}); ErrorOr do_write(OpenFileDescription&, UserOrKernelBuffer const&, size_t, Optional = {}); ErrorOr do_statvfs(FileSystem const& path, Custody const*, statvfs* buf); - ErrorOr> find_elf_interpreter_for_executable(StringView path, ElfW(Ehdr) const& main_executable_header, size_t main_executable_header_size, size_t file_size); + ErrorOr> find_elf_interpreter_for_executable(StringView path, ElfW(Ehdr) const& main_executable_header, size_t main_executable_header_size, size_t file_size, Optional& minimum_stack_size); ErrorOr do_kill(Process&, int signal); ErrorOr do_killpg(ProcessGroupID pgrp, int signal); diff --git a/Userland/Libraries/LibELF/Image.cpp b/Userland/Libraries/LibELF/Image.cpp index cf4c382c28..db11bf76f4 100644 --- a/Userland/Libraries/LibELF/Image.cpp +++ b/Userland/Libraries/LibELF/Image.cpp @@ -128,7 +128,7 @@ bool Image::parse() return false; } - auto result_or_error = validate_program_headers(header(), m_size, { m_buffer, m_size }, nullptr, m_verbose_logging); + auto result_or_error = validate_program_headers(header(), m_size, { m_buffer, m_size }, nullptr, nullptr, m_verbose_logging); if (result_or_error.is_error()) { if (m_verbose_logging) dbgln("ELF::Image::parse(): Failed validating ELF Program Headers"); diff --git a/Userland/Libraries/LibELF/Validation.cpp b/Userland/Libraries/LibELF/Validation.cpp index a1fb73f0fe..8e5da1cc4d 100644 --- a/Userland/Libraries/LibELF/Validation.cpp +++ b/Userland/Libraries/LibELF/Validation.cpp @@ -187,7 +187,7 @@ bool validate_elf_header(ElfW(Ehdr) const& elf_header, size_t file_size, bool ve return true; } -ErrorOr validate_program_headers(ElfW(Ehdr) const& elf_header, size_t file_size, ReadonlyBytes buffer, StringBuilder* interpreter_path_builder, bool verbose) +ErrorOr validate_program_headers(ElfW(Ehdr) const& elf_header, size_t file_size, ReadonlyBytes buffer, StringBuilder* interpreter_path_builder, Optional* requested_stack_size, bool verbose) { Checked total_size_of_program_headers = elf_header.e_phnum; total_size_of_program_headers *= elf_header.e_phentsize; @@ -306,6 +306,9 @@ ErrorOr validate_program_headers(ElfW(Ehdr) const& elf_header, size_t file dbgln("PT_GNU_STACK size is not page-aligned."); return false; } + + if (requested_stack_size) + *requested_stack_size = program_header.p_memsz; } break; diff --git a/Userland/Libraries/LibELF/Validation.h b/Userland/Libraries/LibELF/Validation.h index 3583d94ac3..81c25e7bc6 100644 --- a/Userland/Libraries/LibELF/Validation.h +++ b/Userland/Libraries/LibELF/Validation.h @@ -12,6 +12,6 @@ namespace ELF { bool validate_elf_header(ElfW(Ehdr) const& elf_header, size_t file_size, bool verbose = true); -ErrorOr validate_program_headers(ElfW(Ehdr) const& elf_header, size_t file_size, ReadonlyBytes buffer, StringBuilder* interpreter_path_builder = nullptr, bool verbose = true); +ErrorOr validate_program_headers(ElfW(Ehdr) const& elf_header, size_t file_size, ReadonlyBytes buffer, StringBuilder* interpreter_path_builder = nullptr, Optional* requested_stack_size = nullptr, bool verbose = true); } // end namespace ELF