Kernel: Make the Process allocate_region* API's understand "int prot".

Instead of having to inspect 'prot' at every call site, make the Process
API's take care of that so we can just pass it through.

AK/ELF/ELFLoader.cpp

@@ -54,7 +54,8 @@ bool ELFLoader::layout()
                 program_header.offset(),
                 program_header.is_readable(),
                 program_header.is_writable(),
-                String::format("elf-map-%s%s", program_header.is_readable() ? "r" : "", program_header.is_writable() ? "w" : "")
+                program_header.is_executable(),
+                String::format("elf-map-%s%s%s", program_header.is_readable() ? "r" : "", program_header.is_writable() ? "w" : "", program_header.is_writable() ? "x" : "")
             );
         }
     });

AK/ELF/ELFLoader.h

@@ -17,7 +17,7 @@ public:
     bool load();
 #if defined(KERNEL)
     Function<void*(LinearAddress, size_t, size_t, bool, bool, const String&)> alloc_section_hook;
-    Function<void*(LinearAddress, size_t, size_t, size_t, bool, bool, const String&)> map_section_hook;
+    Function<void*(LinearAddress, size_t, size_t, size_t, bool r, bool w, bool x, const String&)> map_section_hook;
     LinearAddress entry() const { return m_image.entry(); }
 #endif
     char* symbol_ptr(const char* name);

Kernel/Devices/BXVGADevice.cpp

@@ -95,8 +95,7 @@ KResultOr<Region*> BXVGADevice::mmap(Process& process, LinearAddress preferred_l
         move(vmo),
         0,
         "BXVGA Framebuffer",
-        prot & PROT_READ,
-        prot & PROT_WRITE
+        prot
     );
     kprintf("BXVGA: %s(%u) created Region{%p} with size %u for framebuffer P%x with laddr L%x\n",
             process.name().characters(), process.pid(),

Kernel/FileSystem/InodeFile.cpp

@@ -35,7 +35,7 @@ KResultOr<Region*> InodeFile::mmap(Process& process, LinearAddress preferred_lad
     region_name = "Memory-mapped file";
 #endif
     InterruptDisabler disabler;
-    auto* region = process.allocate_file_backed_region(preferred_laddr, size, inode(), move(region_name), prot & PROT_READ, prot & PROT_WRITE);
+    auto* region = process.allocate_file_backed_region(preferred_laddr, size, inode(), move(region_name), prot);
     if (!region)
         return KResult(-ENOMEM);
     return region;

Kernel/Process.cpp

@@ -79,35 +79,47 @@ Range Process::allocate_range(LinearAddress laddr, size_t size)
     return page_directory().range_allocator().allocate_specific(laddr, size);
 }
 
-Region* Process::allocate_region(LinearAddress laddr, size_t size, String&& name, bool is_readable, bool is_writable, bool commit)
+static unsigned prot_to_region_access_flags(int prot)
+{
+    unsigned access = 0;
+    if (prot & PROT_READ)
+        access |= Region::Access::Read;
+    if (prot & PROT_WRITE)
+        access |= Region::Access::Write;
+    if (prot & PROT_EXEC)
+        access |= Region::Access::Execute;
+    return access;
+}
+
+Region* Process::allocate_region(LinearAddress laddr, size_t size, String&& name, int prot, bool commit)
 {
     auto range = allocate_range(laddr, size);
     if (!range.is_valid())
         return nullptr;
-    m_regions.append(adopt(*new Region(range, move(name), is_readable, is_writable)));
+    m_regions.append(adopt(*new Region(range, move(name), prot_to_region_access_flags(prot))));
     MM.map_region(*this, *m_regions.last());
     if (commit)
         m_regions.last()->commit();
     return m_regions.last().ptr();
 }
 
-Region* Process::allocate_file_backed_region(LinearAddress laddr, size_t size, RetainPtr<Inode>&& inode, String&& name, bool is_readable, bool is_writable)
+Region* Process::allocate_file_backed_region(LinearAddress laddr, size_t size, RetainPtr<Inode>&& inode, String&& name, int prot)
 {
     auto range = allocate_range(laddr, size);
     if (!range.is_valid())
         return nullptr;
-    m_regions.append(adopt(*new Region(range, move(inode), move(name), is_readable, is_writable)));
+    m_regions.append(adopt(*new Region(range, move(inode), move(name), prot_to_region_access_flags(prot))));
     MM.map_region(*this, *m_regions.last());
     return m_regions.last().ptr();
 }
 
-Region* Process::allocate_region_with_vmo(LinearAddress laddr, size_t size, Retained<VMObject>&& vmo, size_t offset_in_vmo, String&& name, bool is_readable, bool is_writable)
+Region* Process::allocate_region_with_vmo(LinearAddress laddr, size_t size, Retained<VMObject>&& vmo, size_t offset_in_vmo, String&& name, int prot)
 {
     auto range = allocate_range(laddr, size);
     if (!range.is_valid())
         return nullptr;
     offset_in_vmo &= PAGE_MASK;
-    m_regions.append(adopt(*new Region(range, move(vmo), offset_in_vmo, move(name), is_readable, is_writable)));
+    m_regions.append(adopt(*new Region(range, move(vmo), offset_in_vmo, move(name), prot_to_region_access_flags(prot))));
     MM.map_region(*this, *m_regions.last());
     return m_regions.last().ptr();
 }
@@ -165,7 +177,7 @@ void* Process::sys$mmap(const Syscall::SC_mmap_params* params)
     if ((dword)addr & ~PAGE_MASK)
         return (void*)-EINVAL;
     if (flags & MAP_ANONYMOUS) {
-        auto* region = allocate_region(LinearAddress((dword)addr), size, "mmap", prot & PROT_READ, prot & PROT_WRITE, false);
+        auto* region = allocate_region(LinearAddress((dword)addr), size, "mmap", prot, false);
         if (!region)
             return (void*)-ENOMEM;
         if (flags & MAP_SHARED)
@@ -326,7 +338,7 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir
 #else
     vmo->set_name("ELF image");
 #endif
-    RetainPtr<Region> region = allocate_region_with_vmo(LinearAddress(), descriptor->metadata().size, vmo.copy_ref(), 0, "executable", true, false);
+    RetainPtr<Region> region = allocate_region_with_vmo(LinearAddress(), descriptor->metadata().size, vmo.copy_ref(), 0, "executable", PROT_READ);
     ASSERT(region);
 
     if (this != &current->process()) {
@@ -340,16 +352,28 @@ int Process::do_exec(String path, Vector<String> arguments, Vector<String> envir
         auto old_regions = move(m_regions);
         m_regions.append(*region);
         loader = make<ELFLoader>(region->laddr().as_ptr());
-        loader->map_section_hook = [&] (LinearAddress laddr, size_t size, size_t alignment, size_t offset_in_image, bool is_readable, bool is_writable, const String& name) {
+        loader->map_section_hook = [&] (LinearAddress laddr, size_t size, size_t alignment, size_t offset_in_image, bool is_readable, bool is_writable, bool is_executable, const String& name) {
             ASSERT(size);
             ASSERT(alignment == PAGE_SIZE);
-            (void) allocate_region_with_vmo(laddr, size, vmo.copy_ref(), offset_in_image, String(name), is_readable, is_writable);
+            int prot = 0;
+            if (is_readable)
+                prot |= PROT_READ;
+            if (is_writable)
+                prot |= PROT_WRITE;
+            if (is_executable)
+                prot |= PROT_EXEC;
+            (void) allocate_region_with_vmo(laddr, size, vmo.copy_ref(), offset_in_image, String(name), prot);
             return laddr.as_ptr();
         };
         loader->alloc_section_hook = [&] (LinearAddress laddr, size_t size, size_t alignment, bool is_readable, bool is_writable, const String& name) {
             ASSERT(size);
             ASSERT(alignment == PAGE_SIZE);
-            (void) allocate_region(laddr, size, String(name), is_readable, is_writable);
+            int prot = 0;
+            if (is_readable)
+                prot |= PROT_READ;
+            if (is_writable)
+                prot |= PROT_WRITE;
+            (void) allocate_region(laddr, size, String(name), prot);
             return laddr.as_ptr();
         };
         bool success = loader->load();
@@ -657,7 +681,7 @@ void Process::create_signal_trampolines_if_needed()
         return;
     // FIXME: This should be a global trampoline shared by all processes, not one created per process!
     // FIXME: Remap as read-only after setup.
-    auto* region = allocate_region(LinearAddress(), PAGE_SIZE, "Signal trampolines", true, true);
+    auto* region = allocate_region(LinearAddress(), PAGE_SIZE, "Signal trampolines", PROT_READ | PROT_WRITE | PROT_EXEC);
     m_return_to_ring3_from_signal_trampoline = region->laddr();
     byte* code_ptr = m_return_to_ring3_from_signal_trampoline.as_ptr();
     *code_ptr++ = 0x58; // pop eax (Argument to signal handler (ignored here))
@@ -2379,14 +2403,14 @@ struct SharedBuffer {
         if (m_pid1 == process.pid()) {
             ++m_pid1_retain_count;
             if (!m_pid1_region) {
-                m_pid1_region = process.allocate_region_with_vmo(LinearAddress(), size(), m_vmo.copy_ref(), 0, "SharedBuffer", true, m_pid1_writable);
+                m_pid1_region = process.allocate_region_with_vmo(LinearAddress(), size(), m_vmo.copy_ref(), 0, "SharedBuffer", PROT_READ | (m_pid1_writable ? PROT_WRITE : 0));
                 m_pid1_region->set_shared(true);
             }
             return m_pid1_region->laddr().as_ptr();
         } else if (m_pid2 == process.pid()) {
             ++m_pid2_retain_count;
             if (!m_pid2_region) {
-                m_pid2_region = process.allocate_region_with_vmo(LinearAddress(), size(), m_vmo.copy_ref(), 0, "SharedBuffer", true, m_pid2_writable);
+                m_pid2_region = process.allocate_region_with_vmo(LinearAddress(), size(), m_vmo.copy_ref(), 0, "SharedBuffer", PROT_READ | (m_pid2_writable ? PROT_WRITE : 0));
                 m_pid2_region->set_shared(true);
             }
             return m_pid2_region->laddr().as_ptr();
@@ -2515,7 +2539,7 @@ int Process::sys$create_shared_buffer(pid_t peer_pid, int size, void** buffer)
     auto shared_buffer = make<SharedBuffer>(m_pid, peer_pid, size);
     shared_buffer->m_shared_buffer_id = shared_buffer_id;
     ASSERT(shared_buffer->size() >= size);
-    shared_buffer->m_pid1_region = allocate_region_with_vmo(LinearAddress(), shared_buffer->size(), shared_buffer->m_vmo.copy_ref(), 0, "SharedBuffer", true, true);
+    shared_buffer->m_pid1_region = allocate_region_with_vmo(LinearAddress(), shared_buffer->size(), shared_buffer->m_vmo.copy_ref(), 0, "SharedBuffer", PROT_READ | PROT_WRITE);
     shared_buffer->m_pid1_region->set_shared(true);
     *buffer = shared_buffer->m_pid1_region->laddr().as_ptr();
 #ifdef SHARED_BUFFER_DEBUG

Kernel/Process.h

@@ -248,9 +248,9 @@ public:
 
     bool is_superuser() const { return m_euid == 0; }
 
-    Region* allocate_region_with_vmo(LinearAddress, size_t, Retained<VMObject>&&, size_t offset_in_vmo, String&& name, bool is_readable, bool is_writable);
-    Region* allocate_file_backed_region(LinearAddress, size_t, RetainPtr<Inode>&&, String&& name, bool is_readable, bool is_writable);
-    Region* allocate_region(LinearAddress, size_t, String&& name, bool is_readable = true, bool is_writable = true, bool commit = true);
+    Region* allocate_region_with_vmo(LinearAddress, size_t, Retained<VMObject>&&, size_t offset_in_vmo, String&& name, int prot);
+    Region* allocate_file_backed_region(LinearAddress, size_t, RetainPtr<Inode>&&, String&& name, int prot);
+    Region* allocate_region(LinearAddress, size_t, String&& name, int prot = PROT_READ | PROT_WRITE, bool commit = true);
     bool deallocate_region(Region& region);
 
     void set_being_inspected(bool b) { m_being_inspected = b; }

Kernel/SharedMemory.cpp

@@ -93,5 +93,5 @@ KResultOr<Region*> SharedMemory::mmap(Process& process, LinearAddress laddr, siz
 {
     if (!vmo())
         return KResult(-ENODEV);
-    return process.allocate_region_with_vmo(laddr, size, *vmo(), offset, name(), prot & PROT_READ, prot & PROT_WRITE);
+    return process.allocate_region_with_vmo(laddr, size, *vmo(), offset, name(), prot);
 }

Kernel/VM/MemoryManager.cpp

@@ -403,7 +403,7 @@ RetainPtr<Region> MemoryManager::allocate_kernel_region(size_t size, String&& na
     ASSERT(!(size % PAGE_SIZE));
     auto range = kernel_page_directory().range_allocator().allocate_anywhere(size);
     ASSERT(range.is_valid());
-    auto region = adopt(*new Region(range, move(name), true, true, false));
+    auto region = adopt(*new Region(range, move(name), PROT_READ | PROT_WRITE | PROT_EXEC, false));
     MM.map_region_at_address(*m_kernel_page_directory, *region, range.base(), false);
     // FIXME: It would be cool if these could zero-fill on demand instead.
     region->commit();

Kernel/VM/Region.cpp

@@ -4,36 +4,33 @@
 #include <Kernel/Process.h>
 #include <Kernel/Thread.h>
 
-Region::Region(const Range& range, String&& n, bool r, bool w, bool cow)
+Region::Region(const Range& range, String&& n, byte access, bool cow)
     : m_range(range)
     , m_vmo(VMObject::create_anonymous(size()))
     , m_name(move(n))
-    , m_readable(r)
-    , m_writable(w)
+    , m_access(access)
     , m_cow_map(Bitmap::create(m_vmo->page_count(), cow))
 {
     m_vmo->set_name(m_name);
     MM.register_region(*this);
 }
 
-Region::Region(const Range& range, RetainPtr<Inode>&& inode, String&& n, bool r, bool w)
+Region::Region(const Range& range, RetainPtr<Inode>&& inode, String&& n, byte access)
     : m_range(range)
     , m_vmo(VMObject::create_file_backed(move(inode)))
     , m_name(move(n))
-    , m_readable(r)
-    , m_writable(w)
+    , m_access(access)
     , m_cow_map(Bitmap::create(m_vmo->page_count()))
 {
     MM.register_region(*this);
 }
 
-Region::Region(const Range& range, Retained<VMObject>&& vmo, size_t offset_in_vmo, String&& n, bool r, bool w, bool cow)
+Region::Region(const Range& range, Retained<VMObject>&& vmo, size_t offset_in_vmo, String&& n, byte access, bool cow)
     : m_range(range)
     , m_offset_in_vmo(offset_in_vmo)
     , m_vmo(move(vmo))
     , m_name(move(n))
-    , m_readable(r)
-    , m_writable(w)
+    , m_access(access)
     , m_cow_map(Bitmap::create(m_vmo->page_count(), cow))
 {
     MM.register_region(*this);
@@ -71,7 +68,7 @@ bool Region::page_in()
 Retained<Region> Region::clone()
 {
     ASSERT(current);
-    if (m_shared || (m_readable && !m_writable)) {
+    if (m_shared || (is_readable() && !is_writable())) {
 #ifdef MM_DEBUG
         dbgprintf("%s<%u> Region::clone(): sharing %s (L%x)\n",
                   current->process().name().characters(),
@@ -80,7 +77,7 @@ Retained<Region> Region::clone()
                   laddr().get());
 #endif
         // Create a new region backed by the same VMObject.
-        return adopt(*new Region(m_range, m_vmo.copy_ref(), m_offset_in_vmo, String(m_name), m_readable, m_writable));
+        return adopt(*new Region(m_range, m_vmo.copy_ref(), m_offset_in_vmo, String(m_name), m_access));
     }
 
 #ifdef MM_DEBUG
@@ -93,7 +90,7 @@ Retained<Region> Region::clone()
     // Set up a COW region. The parent (this) region becomes COW as well!
     m_cow_map.fill(true);
     MM.remap_region(current->process().page_directory(), *this);
-    return adopt(*new Region(m_range, m_vmo->clone(), m_offset_in_vmo, String(m_name), m_readable, m_writable, true));
+    return adopt(*new Region(m_range, m_vmo->clone(), m_offset_in_vmo, String(m_name), m_access, true));
 }
 
 int Region::commit()

Kernel/VM/Region.h

@@ -10,17 +10,24 @@ class VMObject;
 
 class Region : public Retainable<Region> {
     friend class MemoryManager;
-
 public:
-    Region(const Range&, String&&, bool r, bool w, bool cow = false);
-    Region(const Range&, Retained<VMObject>&&, size_t offset_in_vmo, String&&, bool r, bool w, bool cow = false);
-    Region(const Range&, RetainPtr<Inode>&&, String&&, bool r, bool w);
+    enum Access
+    {
+        Read = 1,
+        Write = 2,
+        Execute = 4,
+    };
+
+    Region(const Range&, String&&, byte access, bool cow = false);
+    Region(const Range&, Retained<VMObject>&&, size_t offset_in_vmo, String&&, byte access, bool cow = false);
+    Region(const Range&, RetainPtr<Inode>&&, String&&, byte access);
     ~Region();
 
     LinearAddress laddr() const { return m_range.base(); }
     size_t size() const { return m_range.size(); }
-    bool is_readable() const { return m_readable; }
-    bool is_writable() const { return m_writable; }
+    bool is_readable() const { return m_access & Access::Read; }
+    bool is_writable() const { return m_access & Access::Write; }
+    bool is_executable() const { return m_access & Access::Execute; }
     String name() const { return m_name; }
 
     void set_name(String&& name) { m_name = move(name); }
@@ -81,7 +88,13 @@ public:
     bool should_cow(size_t page_index) const { return m_cow_map.get(page_index); }
     void set_should_cow(size_t page_index, bool cow) { m_cow_map.set(page_index, cow); }
 
-    void set_writable(bool b) { m_writable = b; }
+    void set_writable(bool b)
+    {
+        if (b)
+            m_access |= Access::Read;
+        else
+            m_access &= ~Access::Write;
+    }
 
 private:
     RetainPtr<PageDirectory> m_page_directory;
@@ -89,8 +102,7 @@ private:
     size_t m_offset_in_vmo { 0 };
     Retained<VMObject> m_vmo;
     String m_name;
-    bool m_readable { true };
-    bool m_writable { true };
+    byte m_access { 0 };
     bool m_shared { false };
     Bitmap m_cow_map;
 };