Kernel+DynamicLoader: Various fixes around mmap & mprotect syscalls

Kernel/API/prctl_numbers.h

@@ -15,3 +15,4 @@
 #define PR_GET_PROCESS_NAME 7
 #define PR_SET_THREAD_NAME 8
 #define PR_GET_THREAD_NAME 9
+#define PR_SET_NO_TRANSITION_TO_EXECUTABLE_FROM_WRITABLE_PROT 10

Kernel/Memory/AddressSpace.h

@@ -8,6 +8,7 @@
 #pragma once
 
 #include <AK/RedBlackTree.h>
+#include <AK/SetOnce.h>
 #include <AK/Vector.h>
 #include <Kernel/Arch/PageDirectory.h>
 #include <Kernel/Library/LockWeakPtr.h>
@@ -48,8 +49,8 @@ class AddressSpace {
 
     ErrorOr<Vector<Region*, 4>> find_regions_intersecting(VirtualRange const&);
 
-    bool enforces_syscall_regions() const { return m_enforces_syscall_regions; }
-    void set_enforces_syscall_regions(bool b) { m_enforces_syscall_regions = b; }
+    bool enforces_syscall_regions() const { return m_enforces_syscall_regions.was_set(); }
+    void set_enforces_syscall_regions() { m_enforces_syscall_regions.set(); }
 
     void remove_all_regions(Badge<Process>);
 
@@ -68,7 +69,7 @@ class AddressSpace {
 
     RegionTree m_region_tree;
 
-    bool m_enforces_syscall_regions { false };
+    SetOnce m_enforces_syscall_regions;
 };
 
 }

Kernel/Memory/Region.h

@@ -43,9 +43,6 @@ class Region final
         Read = 1,
         Write = 2,
         Execute = 4,
-        HasBeenReadable = 16,
-        HasBeenWritable = 32,
-        HasBeenExecutable = 64,
         ReadOnly = Read,
         ReadWrite = Read | Write,
         ReadWriteExecute = Read | Write | Execute,
@@ -69,9 +66,9 @@ class Region final
     [[nodiscard]] bool is_writable() const { return (m_access & Access::Write) == Access::Write; }
     [[nodiscard]] bool is_executable() const { return (m_access & Access::Execute) == Access::Execute; }
 
-    [[nodiscard]] bool has_been_readable() const { return (m_access & Access::HasBeenReadable) == Access::HasBeenReadable; }
-    [[nodiscard]] bool has_been_writable() const { return (m_access & Access::HasBeenWritable) == Access::HasBeenWritable; }
-    [[nodiscard]] bool has_been_executable() const { return (m_access & Access::HasBeenExecutable) == Access::HasBeenExecutable; }
+    [[nodiscard]] bool has_been_readable() const { return m_has_been_readable.was_set(); }
+    [[nodiscard]] bool has_been_writable() const { return m_has_been_writable.was_set(); }
+    [[nodiscard]] bool has_been_executable() const { return m_has_been_executable.was_set(); }
 
     [[nodiscard]] bool is_cacheable() const { return m_cacheable; }
     [[nodiscard]] StringView name() const { return m_name ? m_name->view() : StringView {}; }
@@ -188,9 +185,24 @@ class Region final
 
     [[nodiscard]] size_t cow_pages() const;
 
-    void set_readable(bool b) { set_access_bit(Access::Read, b); }
-    void set_writable(bool b) { set_access_bit(Access::Write, b); }
-    void set_executable(bool b) { set_access_bit(Access::Execute, b); }
+    void set_readable(bool b)
+    {
+        set_access_bit(Access::Read, b);
+        if (b)
+            m_has_been_readable.set();
+    }
+    void set_writable(bool b)
+    {
+        set_access_bit(Access::Write, b);
+        if (b)
+            m_has_been_writable.set();
+    }
+    void set_executable(bool b)
+    {
+        set_access_bit(Access::Execute, b);
+        if (b)
+            m_has_been_executable.set();
+    }
 
     void unsafe_clear_access() { m_access = Region::None; }
 
@@ -224,7 +236,7 @@ class Region final
     void set_access_bit(Access access, bool b)
     {
         if (b)
-            m_access |= access | (access << 4);
+            m_access |= access;
         else
             m_access &= ~access;
     }
@@ -254,6 +266,9 @@ class Region final
 
     SetOnce m_immutable;
     SetOnce m_initially_loaded_executable_segment;
+    SetOnce m_has_been_readable;
+    SetOnce m_has_been_writable;
+    SetOnce m_has_been_executable;
 
     IntrusiveRedBlackTreeNode<FlatPtr, Region, RawPtr<Region>> m_tree_node;
     IntrusiveListNode<Region> m_vmobject_list_node;

Kernel/Syscalls/fork.cpp

@@ -164,7 +164,8 @@ ErrorOr<FlatPtr> Process::sys$fork(RegisterState& regs)
 
     TRY(address_space().with([&](auto& parent_space) {
         return child->address_space().with([&](auto& child_space) -> ErrorOr<void> {
-            child_space->set_enforces_syscall_regions(parent_space->enforces_syscall_regions());
+            if (parent_space->enforces_syscall_regions())
+                child_space->set_enforces_syscall_regions();
             for (auto& region : parent_space->region_tree().regions()) {
                 dbgln_if(FORK_DEBUG, "fork: cloning Region '{}' @ {}", region.name(), region.vaddr());
                 auto region_clone = TRY(region.try_clone());

Kernel/Syscalls/mmap.cpp

@@ -28,55 +28,8 @@
 
 namespace Kernel {
 
-static bool should_make_executable_exception_for_dynamic_loader(bool make_readable, bool make_writable, bool make_executable, Memory::Region const& region)
-{
-    // Normally we don't allow W -> X transitions, but we have to make an exception
-    // for the dynamic loader, which needs to do this after performing text relocations.
-
-    // FIXME: Investigate whether we could get rid of all text relocations entirely.
-
-    // The exception is only made if all the following criteria is fulfilled:
-
-    // The region must be RW
-    if (!(region.is_readable() && region.is_writable() && !region.is_executable()))
-        return false;
-
-    // The region wants to become RX
-    if (!(make_readable && !make_writable && make_executable))
-        return false;
-
-    // The region is backed by a file
-    if (!region.vmobject().is_inode())
-        return false;
-
-    // The file mapping is private, not shared (no relocations in a shared mapping!)
-    if (!region.vmobject().is_private_inode())
-        return false;
-
-    auto const& inode_vm = static_cast<Memory::InodeVMObject const&>(region.vmobject());
-    auto const& inode = inode_vm.inode();
-
-    Elf_Ehdr header;
-    auto buffer = UserOrKernelBuffer::for_kernel_buffer((u8*)&header);
-    auto result = inode.read_bytes(0, sizeof(header), buffer, nullptr);
-    if (result.is_error() || result.value() != sizeof(header))
-        return false;
-
-    // The file is a valid ELF binary
-    if (!ELF::validate_elf_header(header, inode.size()))
-        return false;
-
-    // The file is an ELF shared object
-    if (header.e_type != ET_DYN)
-        return false;
-
-    // FIXME: Are there any additional checks/validations we could do here?
-    return true;
-}
-
 ErrorOr<void> Process::validate_mmap_prot(int prot, bool map_stack, bool map_anonymous, Memory::Region const* region) const
 {
-    bool make_readable = prot & PROT_READ;
     bool make_writable = prot & PROT_WRITE;
     bool make_executable = prot & PROT_EXEC;
 
@@ -96,13 +49,8 @@ ErrorOr<void> Process::validate_mmap_prot(int prot, bool map_stack, bool map_ano
         if (make_writable && region->has_been_executable())
             return EINVAL;
 
-        if (make_executable && region->has_been_writable()) {
-            if (should_make_executable_exception_for_dynamic_loader(make_readable, make_writable, make_executable, *region)) {
-                return {};
-            } else {
-                return EINVAL;
-            };
-        }
+        if (make_executable && region->has_been_writable() && should_reject_transition_to_executable_from_writable_prot())
+            return EINVAL;
     }
 
     return {};

Kernel/Syscalls/prctl.cpp

@@ -26,14 +26,10 @@ ErrorOr<FlatPtr> Process::sys$prctl(int option, FlatPtr arg1, FlatPtr arg2, Flat
                 return space->enforces_syscall_regions();
             });
         case PR_SET_NO_NEW_SYSCALL_REGION_ANNOTATIONS: {
-            if (arg1 != 0 && arg1 != 1)
+            if (arg1 != 0)
                 return EINVAL;
-            bool prohibit_new_annotated_syscall_regions = (arg1 == 1);
             return address_space().with([&](auto& space) -> ErrorOr<FlatPtr> {
-                if (space->enforces_syscall_regions() && !prohibit_new_annotated_syscall_regions)
-                    return EPERM;
-
-                space->set_enforces_syscall_regions(prohibit_new_annotated_syscall_regions);
+                space->set_enforces_syscall_regions();
                 return 0;
             });
             return 0;
@@ -96,6 +92,13 @@ ErrorOr<FlatPtr> Process::sys$prctl(int option, FlatPtr arg1, FlatPtr arg2, Flat
             }));
             return 0;
         }
+        case PR_SET_NO_TRANSITION_TO_EXECUTABLE_FROM_WRITABLE_PROT: {
+            TRY(require_promise(Pledge::prot_exec));
+            with_mutable_protected_data([](auto& protected_data) {
+                return protected_data.reject_transition_to_executable_from_writable_prot.set();
+            });
+            return 0;
+        }
         }
 
         return EINVAL;

Kernel/Tasks/Process.h

@@ -143,6 +143,7 @@ class Process final
         Atomic<u32> thread_count { 0 };
         u8 termination_status { 0 };
         u8 termination_signal { 0 };
+        SetOnce reject_transition_to_executable_from_writable_prot;
     };
 
 public:
@@ -624,6 +625,13 @@ class Process final
     ErrorOr<void> require_promise(Pledge);
     ErrorOr<void> require_no_promises() const;
 
+    bool should_reject_transition_to_executable_from_writable_prot() const
+    {
+        return with_protected_data([](auto& protected_data) {
+            return protected_data.reject_transition_to_executable_from_writable_prot.was_set();
+        });
+    }
+
     ErrorOr<void> validate_mmap_prot(int prot, bool map_stack, bool map_anonymous, Memory::Region const* region = nullptr) const;
     ErrorOr<void> validate_inode_mmap_prot(int prot, bool description_readable, bool description_writable, bool map_shared) const;
 

Userland/Libraries/LibELF/DynamicLinker.cpp

@@ -739,7 +739,12 @@ Examples of static-pie ELF objects are ELF packers, and the system dynamic loade
         entry_point = entry_point.offset(main_executable_loader->base_address().get());
     auto entry_point_function = reinterpret_cast<EntryPointFunction>(entry_point.as_ptr());
 
-    int rc = syscall(SC_prctl, PR_SET_NO_NEW_SYSCALL_REGION_ANNOTATIONS, 1, 0, nullptr);
+    int rc = syscall(SC_prctl, PR_SET_NO_NEW_SYSCALL_REGION_ANNOTATIONS, 0, 0, nullptr);
+    if (rc < 0) {
+        VERIFY_NOT_REACHED();
+    }
+
+    rc = syscall(SC_prctl, PR_SET_NO_TRANSITION_TO_EXECUTABLE_FROM_WRITABLE_PROT, 0, 0, nullptr);
     if (rc < 0) {
         VERIFY_NOT_REACHED();
     }