From 69578254449769c2c429827c0e1eea787dd35ebb Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Fri, 17 May 2019 04:39:22 +0200 Subject: [PATCH] Kernel: Factor out range allocation from Process::allocate_region*(). These functions were doing exactly the same thing for range allocation, so share that code in an allocate_range() helper. Region allocation will now also fail if range allocation fails, which means that mmap() can actually fail without falling apart. Exciting times! --- Kernel/FileSystem/FileDescriptor.cpp | 2 ++ Kernel/Process.cpp | 40 +++++++++++----------------- Kernel/Process.h | 2 ++ 3 files changed, 20 insertions(+), 24 deletions(-) diff --git a/Kernel/FileSystem/FileDescriptor.cpp b/Kernel/FileSystem/FileDescriptor.cpp index e0242af169..f8483de195 100644 --- a/Kernel/FileSystem/FileDescriptor.cpp +++ b/Kernel/FileSystem/FileDescriptor.cpp @@ -346,6 +346,8 @@ KResultOr FileDescriptor::mmap(Process& process, LinearAddress laddr, s // FIXME: Implement mapping at a client-specified address. Most of the support is already in plcae. ASSERT(laddr.as_ptr() == nullptr); auto* region = process.allocate_file_backed_region(LinearAddress(), size, inode(), move(region_name), prot & PROT_READ, prot & PROT_WRITE); + if (!region) + return KResult(-ENOMEM); region->page_in(); return region; } diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 0275cc83c5..3cd7c93ddf 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -68,17 +68,20 @@ bool Process::in_group(gid_t gid) const return m_gids.contains(gid); } -Region* Process::allocate_region(LinearAddress laddr, size_t size, String&& name, bool is_readable, bool is_writable, bool commit) +Range Process::allocate_range(LinearAddress laddr, size_t size) { laddr.mask(PAGE_MASK); size = PAGE_ROUND_UP(size); - - Range range; if (laddr.is_null()) - range = m_range_allocator.allocate_anywhere(size); - else - range = m_range_allocator.allocate_specific(laddr, size); + return m_range_allocator.allocate_anywhere(size); + return m_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) +{ + 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))); MM.map_region(*this, *m_regions.last()); if (commit) @@ -88,15 +91,9 @@ Region* Process::allocate_region(LinearAddress laddr, size_t size, String&& name Region* Process::allocate_file_backed_region(LinearAddress laddr, size_t size, RetainPtr&& inode, String&& name, bool is_readable, bool is_writable) { - laddr.mask(PAGE_MASK); - size = PAGE_ROUND_UP(size); - - Range range; - if (laddr.is_null()) - range = m_range_allocator.allocate_anywhere(size); - else - range = m_range_allocator.allocate_specific(laddr, size); - + 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))); MM.map_region(*this, *m_regions.last()); return m_regions.last().ptr(); @@ -104,15 +101,9 @@ Region* Process::allocate_file_backed_region(LinearAddress laddr, size_t size, R Region* Process::allocate_region_with_vmo(LinearAddress laddr, size_t size, Retained&& vmo, size_t offset_in_vmo, String&& name, bool is_readable, bool is_writable) { - laddr.mask(PAGE_MASK); - size = PAGE_ROUND_UP(size); - - Range range; - if (laddr.is_null()) - range = m_range_allocator.allocate_anywhere(size); - else - range = m_range_allocator.allocate_specific(laddr, size); - + auto range = allocate_range(laddr, size); + if (!range.is_valid()) + return nullptr; offset_in_vmo &= PAGE_MASK; size = ceil_div(size, PAGE_SIZE) * PAGE_SIZE; m_regions.append(adopt(*new Region(range, move(vmo), offset_in_vmo, move(name), is_readable, is_writable))); @@ -328,6 +319,7 @@ int Process::do_exec(String path, Vector arguments, Vector envir vmo->set_name("ELF image"); #endif RetainPtr region = allocate_region_with_vmo(LinearAddress(), descriptor->metadata().size, vmo.copy_ref(), 0, "executable", true, false); + ASSERT(region); if (this != ¤t->process()) { // FIXME: Don't force-load the entire executable at once, let the on-demand pager take care of it. diff --git a/Kernel/Process.h b/Kernel/Process.h index 6d408c4165..444b40825c 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -258,6 +258,8 @@ private: Process(String&& name, uid_t, gid_t, pid_t ppid, RingLevel, RetainPtr&& cwd = nullptr, RetainPtr&& executable = nullptr, TTY* = nullptr, Process* fork_parent = nullptr); + Range allocate_range(LinearAddress, size_t); + int do_exec(String path, Vector arguments, Vector environment); ssize_t do_write(FileDescriptor&, const byte*, int data_size);