diff --git a/Kernel/Memory/AddressSpace.cpp b/Kernel/Memory/AddressSpace.cpp index 3399c48f14..abcad85fbd 100644 --- a/Kernel/Memory/AddressSpace.cpp +++ b/Kernel/Memory/AddressSpace.cpp @@ -142,7 +142,7 @@ ErrorOr AddressSpace::try_allocate_split_region(Region const& source_re auto new_region = TRY(Region::create_unplaced( source_region.vmobject(), offset_in_vmobject, move(region_name), source_region.access(), source_region.is_cacheable() ? Region::Cacheable::Yes : Region::Cacheable::No, source_region.is_shared())); new_region->set_syscall_region(source_region.is_syscall_region()); - new_region->set_mmap(source_region.is_mmap()); + new_region->set_mmap(source_region.is_mmap(), source_region.mmapped_from_readable(), source_region.mmapped_from_writable()); new_region->set_stack(source_region.is_stack()); size_t page_offset_in_source_region = (offset_in_vmobject - source_region.offset_in_vmobject()) / PAGE_SIZE; for (size_t i = 0; i < new_region->page_count(); ++i) { diff --git a/Kernel/Memory/InodeVMObject.cpp b/Kernel/Memory/InodeVMObject.cpp index 7f609c9d67..25a7541814 100644 --- a/Kernel/Memory/InodeVMObject.cpp +++ b/Kernel/Memory/InodeVMObject.cpp @@ -96,14 +96,4 @@ u32 InodeVMObject::writable_mappings() const return count; } -u32 InodeVMObject::executable_mappings() const -{ - u32 count = 0; - const_cast(*this).for_each_region([&](auto& region) { - if (region.is_executable()) - ++count; - }); - return count; -} - } diff --git a/Kernel/Memory/InodeVMObject.h b/Kernel/Memory/InodeVMObject.h index ee7124caf4..0a7edd2adb 100644 --- a/Kernel/Memory/InodeVMObject.h +++ b/Kernel/Memory/InodeVMObject.h @@ -26,7 +26,6 @@ public: int try_release_clean_pages(int page_amount); u32 writable_mappings() const; - u32 executable_mappings() const; protected: explicit InodeVMObject(Inode&, FixedArray>&&, Bitmap dirty_pages); diff --git a/Kernel/Memory/Region.cpp b/Kernel/Memory/Region.cpp index 883e813867..9ea6f1b7ad 100644 --- a/Kernel/Memory/Region.cpp +++ b/Kernel/Memory/Region.cpp @@ -106,7 +106,7 @@ ErrorOr> Region::try_clone() auto region = TRY(Region::try_create_user_accessible( m_range, vmobject(), m_offset_in_vmobject, move(region_name), access(), m_cacheable ? Cacheable::Yes : Cacheable::No, m_shared)); - region->set_mmap(m_mmap); + region->set_mmap(m_mmap, m_mmapped_from_readable, m_mmapped_from_writable); region->set_shared(m_shared); region->set_syscall_region(is_syscall_region()); return region; @@ -133,7 +133,7 @@ ErrorOr> Region::try_clone() clone_region->set_stack(true); } clone_region->set_syscall_region(is_syscall_region()); - clone_region->set_mmap(m_mmap); + clone_region->set_mmap(m_mmap, m_mmapped_from_readable, m_mmapped_from_writable); return clone_region; } diff --git a/Kernel/Memory/Region.h b/Kernel/Memory/Region.h index dfe660d1c0..3f26509261 100644 --- a/Kernel/Memory/Region.h +++ b/Kernel/Memory/Region.h @@ -89,7 +89,13 @@ public: void set_stack(bool stack) { m_stack = stack; } [[nodiscard]] bool is_mmap() const { return m_mmap; } - void set_mmap(bool mmap) { m_mmap = mmap; } + + void set_mmap(bool mmap, bool description_was_readable, bool description_was_writable) + { + m_mmap = mmap; + m_mmapped_from_readable = description_was_readable; + m_mmapped_from_writable = description_was_writable; + } [[nodiscard]] bool is_write_combine() const { return m_write_combine; } ErrorOr set_write_combine(bool); @@ -194,6 +200,9 @@ public: [[nodiscard]] bool is_syscall_region() const { return m_syscall_region; } void set_syscall_region(bool b) { m_syscall_region = b; } + [[nodiscard]] bool mmapped_from_readable() const { return m_mmapped_from_readable; } + [[nodiscard]] bool mmapped_from_writable() const { return m_mmapped_from_writable; } + private: Region(); Region(NonnullLockRefPtr, size_t offset_in_vmobject, OwnPtr, Region::Access access, Cacheable, bool shared); @@ -228,6 +237,8 @@ private: bool m_mmap : 1 { false }; bool m_syscall_region : 1 { false }; bool m_write_combine : 1 { false }; + bool m_mmapped_from_readable : 1 { false }; + bool m_mmapped_from_writable : 1 { false }; IntrusiveRedBlackTreeNode> m_tree_node; IntrusiveListNode m_vmobject_list_node; diff --git a/Kernel/Process.h b/Kernel/Process.h index 7c5024f178..71d40572f0 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -570,7 +570,7 @@ public: ErrorOr require_no_promises() const; ErrorOr validate_mmap_prot(int prot, bool map_stack, bool map_anonymous, Memory::Region const* region = nullptr) const; - ErrorOr validate_inode_mmap_prot(int prot, Inode const& inode, bool map_shared) const; + ErrorOr validate_inode_mmap_prot(int prot, bool description_readable, bool description_writable, bool map_shared) const; private: friend class MemoryManager; diff --git a/Kernel/Syscalls/mmap.cpp b/Kernel/Syscalls/mmap.cpp index 1e5be912ab..4ef204f1b6 100644 --- a/Kernel/Syscalls/mmap.cpp +++ b/Kernel/Syscalls/mmap.cpp @@ -103,25 +103,18 @@ ErrorOr Process::validate_mmap_prot(int prot, bool map_stack, bool map_ano return {}; } -ErrorOr Process::validate_inode_mmap_prot(int prot, Inode const& inode, bool map_shared) const +ErrorOr Process::validate_inode_mmap_prot(int prot, bool readable_description, bool description_writable, bool map_shared) const { auto credentials = this->credentials(); - auto metadata = inode.metadata(); - if ((prot & PROT_READ) && !metadata.may_read(credentials)) + if ((prot & PROT_READ) && !readable_description) return EACCES; if (map_shared) { // FIXME: What about readonly filesystem mounts? We cannot make a // decision here without knowing the mount flags, so we would need to // keep a Custody or something from mmap time. - if ((prot & PROT_WRITE) && !metadata.may_write(credentials)) + if ((prot & PROT_WRITE) && !description_writable) return EACCES; - if (auto shared_vmobject = inode.shared_vmobject()) { - if ((prot & PROT_EXEC) && shared_vmobject->writable_mappings()) - return EACCES; - if ((prot & PROT_WRITE) && shared_vmobject->executable_mappings()) - return EACCES; - } } return {}; } @@ -227,7 +220,7 @@ ErrorOr Process::sys$mmap(Userspace use return EACCES; } if (description->inode()) - TRY(validate_inode_mmap_prot(prot, *description->inode(), map_shared)); + TRY(validate_inode_mmap_prot(prot, description->is_readable(), description->is_writable(), map_shared)); vmobject = TRY(description->vmobject_for_mmap(*this, requested_range, used_offset, map_shared)); } @@ -251,7 +244,11 @@ ErrorOr Process::sys$mmap(Userspace use if (!region) return ENOMEM; - region->set_mmap(true); + if (description) + region->set_mmap(true, description->is_readable(), description->is_writable()); + else + region->set_mmap(true, false, false); + if (map_shared) region->set_shared(true); if (map_stack) @@ -289,7 +286,7 @@ ErrorOr Process::sys$mprotect(Userspace addr, size_t size, int p if (whole_region->access() == Memory::prot_to_region_access_flags(prot)) return 0; if (whole_region->vmobject().is_inode()) - TRY(validate_inode_mmap_prot(prot, static_cast(whole_region->vmobject()).inode(), whole_region->is_shared())); + TRY(validate_inode_mmap_prot(prot, whole_region->mmapped_from_readable(), whole_region->mmapped_from_writable(), whole_region->is_shared())); whole_region->set_readable(prot & PROT_READ); whole_region->set_writable(prot & PROT_WRITE); whole_region->set_executable(prot & PROT_EXEC); @@ -306,7 +303,7 @@ ErrorOr Process::sys$mprotect(Userspace addr, size_t size, int p if (old_region->access() == Memory::prot_to_region_access_flags(prot)) return 0; if (old_region->vmobject().is_inode()) - TRY(validate_inode_mmap_prot(prot, static_cast(old_region->vmobject()).inode(), old_region->is_shared())); + TRY(validate_inode_mmap_prot(prot, old_region->mmapped_from_readable(), old_region->mmapped_from_writable(), old_region->is_shared())); // Remove the old region from our regions tree, since were going to add another region // with the exact same start address. @@ -339,7 +336,8 @@ ErrorOr Process::sys$mprotect(Userspace addr, size_t size, int p return EPERM; TRY(validate_mmap_prot(prot, region->is_stack(), region->vmobject().is_anonymous(), region)); if (region->vmobject().is_inode()) - TRY(validate_inode_mmap_prot(prot, static_cast(region->vmobject()).inode(), region->is_shared())); + TRY(validate_inode_mmap_prot(prot, region->mmapped_from_readable(), region->mmapped_from_writable(), region->is_shared())); + full_size_found += region->range().intersect(range_to_mprotect).size(); } @@ -490,11 +488,14 @@ ErrorOr Process::sys$mremap(Userspace auto new_vmobject = TRY(Memory::PrivateInodeVMObject::try_create_with_inode(inode)); auto old_name = old_region->take_name(); + bool old_region_was_mmapped_from_readable = old_region->mmapped_from_readable(); + bool old_region_was_mmapped_from_writable = old_region->mmapped_from_writable(); + old_region->unmap(); space->deallocate_region(*old_region); auto* new_region = TRY(space->allocate_region_with_vmobject(range, move(new_vmobject), old_offset, old_name->view(), old_prot, false)); - new_region->set_mmap(true); + new_region->set_mmap(true, old_region_was_mmapped_from_readable, old_region_was_mmapped_from_writable); return new_region->vaddr().get(); }