From 09129782de3ba4d4afec953e77311e6264b08afe Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 25 Dec 2020 02:31:04 +0100 Subject: [PATCH] Kernel: Simplify ELF loading logic in sys$execve() somewhat Get rid of the lambda functions and put the logic inline in the program header traversal loop instead. This makes the code quite a bit shorter and hopefully makes it easier to see what's going on. --- Kernel/Syscalls/execve.cpp | 116 +++++++++++++++---------------------- 1 file changed, 46 insertions(+), 70 deletions(-) diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index 02e09659531..ae047973e31 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -83,66 +83,32 @@ KResultOr Process::load_elf_object(FileDescription& object_ FlatPtr load_base_address = 0; MM.enter_process_paging_scope(*this); - String object_name = LexicalPath(object_description.absolute_path()).basename(); + String elf_name = object_description.absolute_path(); auto elf_image = ELF::Image(region->vaddr().as_ptr(), loader_metadata.size); - auto map_section_hook = [&](VirtualAddress vaddr, size_t size, size_t alignment, size_t offset_in_image, bool is_readable, bool is_writable, bool is_executable, const String& name) -> u8* { - ASSERT(size); - ASSERT(alignment == PAGE_SIZE); - int prot = 0; - if (is_readable) - prot |= PROT_READ; - if (is_writable) - prot |= PROT_WRITE; - if (is_executable) - prot |= PROT_EXEC; - if (auto* region = allocate_region_with_vmobject(vaddr.offset(load_offset), size, *vmobject, offset_in_image, String(name), prot)) { - region->set_shared(true); - if (offset_in_image == 0) - load_base_address = (FlatPtr)region->vaddr().as_ptr(); - return region->vaddr().as_ptr(); - } - return nullptr; - }; - - auto alloc_section_hook = [&](VirtualAddress vaddr, size_t size, size_t alignment, bool is_readable, bool is_writable, const String& name) -> u8* { - ASSERT(size); - ASSERT(alignment == PAGE_SIZE); - int prot = 0; - if (is_readable) - prot |= PROT_READ; - if (is_writable) - prot |= PROT_WRITE; - if (auto* region = allocate_region(vaddr.offset(load_offset), size, String(name), prot)) - return region->vaddr().as_ptr(); - return nullptr; - }; - - auto tls_section_hook = [&](size_t size, size_t alignment) { - ASSERT(should_allocate_tls == ShouldAllocateTls::Yes); - ASSERT(size); - master_tls_region = allocate_region({}, size, String(), PROT_READ | PROT_WRITE); - master_tls_size = size; - master_tls_alignment = alignment; - return master_tls_region->vaddr().as_ptr(); - }; - ASSERT(!Processor::current().in_critical()); bool failed = false; elf_image.for_each_program_header([&](const ELF::Image::ProgramHeader& program_header) { if (program_header.type() == PT_TLS) { - auto* tls_image = tls_section_hook(program_header.size_in_memory(), program_header.alignment()); - if (!tls_image) { - failed = true; - return; - } + ASSERT(should_allocate_tls == ShouldAllocateTls::Yes); + ASSERT(program_header.size_in_memory()); + if (!elf_image.is_within_image(program_header.raw_data(), program_header.size_in_image())) { dbg() << "Shenanigans! ELF PT_TLS header sneaks outside of executable."; failed = true; return; } - if (!copy_to_user(tls_image, program_header.raw_data(), program_header.size_in_image())) { + + master_tls_region = allocate_region({}, program_header.size_in_memory(), String::formatted("{} (master-tls)", elf_name), PROT_READ | PROT_WRITE); + if (!master_tls_region) { + failed = true; + return; + } + master_tls_size = program_header.size_in_memory(); + master_tls_alignment = program_header.alignment(); + + if (!copy_to_user(master_tls_region->vaddr().as_ptr(), program_header.raw_data(), program_header.size_in_image())) { failed = false; return; } @@ -151,22 +117,27 @@ KResultOr Process::load_elf_object(FileDescription& object_ if (program_header.type() != PT_LOAD) return; if (program_header.is_writable()) { - auto* allocated_section = alloc_section_hook( - program_header.vaddr(), - program_header.size_in_memory(), - program_header.alignment(), - program_header.is_readable(), - program_header.is_writable(), - String::format("%s-alloc-%s%s", m_name.is_empty() ? "elf" : m_name.characters(), program_header.is_readable() ? "r" : "", program_header.is_writable() ? "w" : "")); - if (!allocated_section) { - failed = true; - return; - } + ASSERT(program_header.size_in_memory()); + ASSERT(program_header.alignment() == PAGE_SIZE); + if (!elf_image.is_within_image(program_header.raw_data(), program_header.size_in_image())) { dbg() << "Shenanigans! Writable ELF PT_LOAD header sneaks outside of executable."; failed = true; return; } + + int prot = 0; + if (program_header.is_readable()) + prot |= PROT_READ; + if (program_header.is_writable()) + prot |= PROT_WRITE; + auto region_name = String::formatted("{} (data-{}{})", elf_name, program_header.is_readable() ? "r" : "", program_header.is_writable() ? "w" : ""); + auto* region = allocate_region(program_header.vaddr().offset(load_offset), program_header.size_in_memory(), move(region_name), prot); + if (!region) { + failed = true; + return; + } + // It's not always the case with PIE executables (and very well shouldn't be) that the // virtual address in the program header matches the one we end up giving the process. // In order to copy the data image correctly into memory, we need to copy the data starting at @@ -176,23 +147,28 @@ KResultOr Process::load_elf_object(FileDescription& object_ // Accessing it would definitely be a bug. auto page_offset = program_header.vaddr(); page_offset.mask(~PAGE_MASK); - if (!copy_to_user((u8*)allocated_section + page_offset.get(), program_header.raw_data(), program_header.size_in_image())) { + if (!copy_to_user((u8*)region->vaddr().as_ptr() + page_offset.get(), program_header.raw_data(), program_header.size_in_image())) { failed = false; return; } } else { - auto* mapped_section = map_section_hook( - program_header.vaddr(), - program_header.size_in_memory(), - program_header.alignment(), - program_header.offset(), - program_header.is_readable(), - program_header.is_writable(), - program_header.is_executable(), - String::format("%s-map-%s%s%s", m_name.is_empty() ? "elf" : m_name.characters(), program_header.is_readable() ? "r" : "", program_header.is_writable() ? "w" : "", program_header.is_executable() ? "x" : "")); - if (!mapped_section) { + ASSERT(program_header.size_in_memory()); + ASSERT(program_header.alignment() == PAGE_SIZE); + int prot = 0; + if (program_header.is_readable()) + prot |= PROT_READ; + if (program_header.is_writable()) + prot |= PROT_WRITE; + if (program_header.is_executable()) + prot |= PROT_EXEC; + auto* region = allocate_region_with_vmobject(program_header.vaddr().offset(load_offset), program_header.size_in_memory(), *vmobject, program_header.offset(), elf_name, prot); + if (!region) { failed = true; + return; } + region->set_shared(true); + if (program_header.offset() == 0) + load_base_address = (FlatPtr)region->vaddr().as_ptr(); } });