From 197e73ee311db09c50e3a48d1e6c0b1e15512664 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 10 Jan 2020 06:57:18 +0100 Subject: Kernel+LibELF: Enable SMAP protection during non-syscall exec() When loading a new executable, we now map the ELF image in kernel-only memory and parse it there. Then we use copy_to_user() when initializing writable regions with data from the executable. Note that the exec() syscall still disables SMAP protection and will require additional work. This patch only affects kernel-originated process spawns. --- Kernel/Process.cpp | 13 ++++++------- Kernel/Process.h | 2 +- Kernel/VM/Region.cpp | 7 +++++++ Kernel/VM/Region.h | 1 + Libraries/LibELF/ELFLoader.cpp | 7 +++++-- 5 files changed, 20 insertions(+), 10 deletions(-) diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index a5643a79e6..c9b0cae3be 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -165,13 +165,16 @@ Region* Process::allocate_file_backed_region(VirtualAddress vaddr, size_t size, return &m_regions.last(); } -Region* Process::allocate_region_with_vmobject(VirtualAddress vaddr, size_t size, NonnullRefPtr vmobject, size_t offset_in_vmobject, const String& name, int prot) +Region* Process::allocate_region_with_vmobject(VirtualAddress vaddr, size_t size, NonnullRefPtr vmobject, size_t offset_in_vmobject, const String& name, int prot, bool user_accessible) { auto range = allocate_range(vaddr, size); if (!range.is_valid()) return nullptr; offset_in_vmobject &= PAGE_MASK; - m_regions.append(Region::create_user_accessible(range, move(vmobject), offset_in_vmobject, name, prot_to_region_access_flags(prot))); + if (user_accessible) + m_regions.append(Region::create_user_accessible(range, move(vmobject), offset_in_vmobject, name, prot_to_region_access_flags(prot))); + else + m_regions.append(Region::create_kernel_only(range, move(vmobject), offset_in_vmobject, name, prot_to_region_access_flags(prot))); m_regions.last().map(page_directory()); return &m_regions.last(); } @@ -669,7 +672,7 @@ int Process::do_exec(String path, Vector arguments, Vector envir ASSERT(description->inode()); auto vmobject = InodeVMObject::create_with_inode(*description->inode()); - auto* region = allocate_region_with_vmobject(VirtualAddress(), metadata.size, vmobject, 0, description->absolute_path(), PROT_READ); + auto* region = allocate_region_with_vmobject(VirtualAddress(), metadata.size, vmobject, 0, description->absolute_path(), PROT_READ, false); ASSERT(region); // NOTE: We yank this out of 'm_regions' since we're about to manipulate the vector @@ -682,7 +685,6 @@ int Process::do_exec(String path, Vector arguments, Vector envir OwnPtr loader; { - SmapDisabler disabler; // Okay, here comes the sleight of hand, pay close attention.. auto old_regions = move(m_regions); m_regions.append(move(executable_region)); @@ -741,9 +743,6 @@ int Process::do_exec(String path, Vector arguments, Vector envir #endif } - region->set_user_accessible(false); - region->remap(); - m_elf_loader = move(loader); m_executable = description->custody(); diff --git a/Kernel/Process.h b/Kernel/Process.h index f8a96d91fa..9f937665d0 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -279,7 +279,7 @@ public: bool is_superuser() const { return m_euid == 0; } - Region* allocate_region_with_vmobject(VirtualAddress, size_t, NonnullRefPtr, size_t offset_in_vmobject, const String& name, int prot); + Region* allocate_region_with_vmobject(VirtualAddress, size_t, NonnullRefPtr, size_t offset_in_vmobject, const String& name, int prot, bool user_accessible = true); Region* allocate_file_backed_region(VirtualAddress, size_t, NonnullRefPtr, const String& name, int prot); Region* allocate_region(VirtualAddress, size_t, const String& name, int prot = PROT_READ | PROT_WRITE, bool commit = true); bool deallocate_region(Region& region); diff --git a/Kernel/VM/Region.cpp b/Kernel/VM/Region.cpp index 85e84bd92e..5a0917d44c 100644 --- a/Kernel/VM/Region.cpp +++ b/Kernel/VM/Region.cpp @@ -189,6 +189,13 @@ NonnullOwnPtr Region::create_kernel_only(const Range& range, const Strin return region; } +NonnullOwnPtr Region::create_kernel_only(const Range& range, NonnullRefPtr vmobject, size_t offset_in_vmobject, const StringView& name, u8 access) +{ + auto region = make(range, move(vmobject), offset_in_vmobject, name, access); + region->m_user_accessible = false; + return region; +} + bool Region::should_cow(size_t page_index) const { if (m_shared) diff --git a/Kernel/VM/Region.h b/Kernel/VM/Region.h index ddf7c4d194..3af580af79 100644 --- a/Kernel/VM/Region.h +++ b/Kernel/VM/Region.h @@ -30,6 +30,7 @@ public: static NonnullOwnPtr create_user_accessible(const Range&, NonnullRefPtr, size_t offset_in_vmobject, const StringView& name, u8 access); static NonnullOwnPtr create_user_accessible(const Range&, NonnullRefPtr, const StringView& name, u8 access); static NonnullOwnPtr create_kernel_only(const Range&, const StringView& name, u8 access); + static NonnullOwnPtr create_kernel_only(const Range&, NonnullRefPtr, size_t offset_in_vmobject, const StringView& name, u8 access); ~Region(); diff --git a/Libraries/LibELF/ELFLoader.cpp b/Libraries/LibELF/ELFLoader.cpp index 3c89246774..95645f553f 100644 --- a/Libraries/LibELF/ELFLoader.cpp +++ b/Libraries/LibELF/ELFLoader.cpp @@ -5,6 +5,9 @@ #ifdef KERNEL #include +#define do_memcpy copy_to_user +#else +#define do_memcpy memcpy #endif //#define ELFLOADER_DEBUG @@ -48,7 +51,7 @@ bool ELFLoader::layout() failed = true; return; } - memcpy(tls_image, program_header.raw_data(), program_header.size_in_image()); + do_memcpy(tls_image, program_header.raw_data(), program_header.size_in_image()); #endif return; } @@ -75,7 +78,7 @@ bool ELFLoader::layout() failed = true; return; } - memcpy(program_header.vaddr().as_ptr(), program_header.raw_data(), program_header.size_in_image()); + do_memcpy(program_header.vaddr().as_ptr(), program_header.raw_data(), program_header.size_in_image()); } else { auto* mapped_section = map_section_hook( program_header.vaddr(), -- cgit v1.2.3