Kernel: Make Region::try_create_user_accessible() OOM-safe

Previously we would simply assume that Region allocation always
succeeded. There is still one such assumption when splitting user
regions inside a Space. That will be dealt with in a separate commit.

3 files changed, 33 insertions, 14 deletions

diff --git a/Kernel/VM/Region.cpp b/Kernel/VM/Region.cpp
index bd1fa22e2b..fa93fd15ad 100644
--- a/Kernel/VM/Region.cpp
+++ b/Kernel/VM/Region.cpp
@@ -83,8 +83,12 @@ OwnPtr<Region> Region::clone(Process& new_owner)
             VERIFY(vmobject().is_shared_inode());
 
         // Create a new region backed by the same VMObject.
-        auto region = Region::create_user_accessible(
+        auto region = Region::try_create_user_accessible(
             &new_owner, m_range, m_vmobject, m_offset_in_vmobject, m_name ? m_name->try_clone() : OwnPtr<KString> {}, access(), m_cacheable ? Cacheable::Yes : Cacheable::No, m_shared);
+        if (!region) {
+            dbgln("Region::clone: Unable to allocate new Region");
+            return nullptr;
+        }
         if (m_vmobject->is_anonymous())
             region->copy_purgeable_page_ranges(*this);
         region->set_mmap(m_mmap);
@@ -102,8 +106,12 @@ OwnPtr<Region> Region::clone(Process& new_owner)
 
     // Set up a COW region. The parent (this) region becomes COW as well!
     remap();
-    auto clone_region = Region::create_user_accessible(
+    auto clone_region = Region::try_create_user_accessible(
         &new_owner, m_range, vmobject_clone.release_nonnull(), m_offset_in_vmobject, m_name ? m_name->try_clone() : OwnPtr<KString> {}, access(), m_cacheable ? Cacheable::Yes : Cacheable::No, m_shared);
+    if (!clone_region) {
+        dbgln("Region::clone: Unable to allocate new Region for COW");
+        return nullptr;
+    }
     if (m_vmobject->is_anonymous())
         clone_region->copy_purgeable_page_ranges(*this);
     if (m_stack) {
@@ -208,13 +216,14 @@ size_t Region::amount_shared() const
     return bytes;
 }
 
-NonnullOwnPtr<Region> Region::create_user_accessible(Process* owner, const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, OwnPtr<KString> name, Region::Access access, Cacheable cacheable, bool shared)
+OwnPtr<Region> Region::try_create_user_accessible(Process* owner, const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, OwnPtr<KString> name, Region::Access access, Cacheable cacheable, bool shared)
 {
     auto region = adopt_own_if_nonnull(new (nothrow) Region(range, move(vmobject), offset_in_vmobject, move(name), access, cacheable, shared));
-    if (region && owner)
+    if (!region)
+        return nullptr;
+    if (owner)
         region->m_owner = owner->make_weak_ptr();
-    // FIXME: Return OwnPtr and propagate failure, currently there are too many assumptions made by down stream callers.
-    return region.release_nonnull();
+    return region;
 }
 
 OwnPtr<Region> Region::create_kernel_only(const Range& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, OwnPtr<KString> name, Region::Access access, Cacheable cacheable)
diff --git a/Kernel/VM/Region.h b/Kernel/VM/Region.h
index ed7bc9aff9..dd38a319aa 100644
--- a/Kernel/VM/Region.h
+++ b/Kernel/VM/Region.h
@@ -49,7 +49,7 @@ public:
         Yes,
     };
 
-    static NonnullOwnPtr<Region> create_user_accessible(Process*, const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, OwnPtr<KString> name, Region::Access access, Cacheable, bool shared);
+    static OwnPtr<Region> try_create_user_accessible(Process*, const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, OwnPtr<KString> name, Region::Access access, Cacheable, bool shared);
     static OwnPtr<Region> create_kernel_only(const Range&, NonnullRefPtr<VMObject>, size_t offset_in_vmobject, OwnPtr<KString> name, Region::Access access, Cacheable = Cacheable::Yes);
 
     ~Region();
diff --git a/Kernel/VM/Space.cpp b/Kernel/VM/Space.cpp
index 851bb072a1..a7cc4ca8d5 100644
--- a/Kernel/VM/Space.cpp
+++ b/Kernel/VM/Space.cpp
@@ -145,8 +145,11 @@ Optional<Range> Space::allocate_range(VirtualAddress vaddr, size_t size, size_t
 
 Region& Space::allocate_split_region(const Region& source_region, const Range& range, size_t offset_in_vmobject)
 {
-    auto& region = add_region(Region::create_user_accessible(
-        m_process, range, source_region.vmobject(), offset_in_vmobject, KString::try_create(source_region.name()), source_region.access(), source_region.is_cacheable() ? Region::Cacheable::Yes : Region::Cacheable::No, source_region.is_shared()));
+    auto new_region = Region::try_create_user_accessible(
+        m_process, range, source_region.vmobject(), offset_in_vmobject, KString::try_create(source_region.name()), source_region.access(), source_region.is_cacheable() ? Region::Cacheable::Yes : Region::Cacheable::No, source_region.is_shared());
+    // FIXME: Handle !new_region (by propagating ENOMEM)
+    VERIFY(new_region);
+    auto& region = add_region(new_region.release_nonnull());
     region.set_syscall_region(source_region.is_syscall_region());
     region.set_mmap(source_region.is_mmap());
     region.set_stack(source_region.is_stack());
@@ -164,10 +167,12 @@ KResultOr<Region*> Space::allocate_region(Range const& range, StringView name, i
     auto vmobject = AnonymousVMObject::try_create_with_size(range.size(), strategy);
     if (!vmobject)
         return ENOMEM;
-    auto region = Region::create_user_accessible(m_process, range, vmobject.release_nonnull(), 0, KString::try_create(name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, false);
+    auto region = Region::try_create_user_accessible(m_process, range, vmobject.release_nonnull(), 0, KString::try_create(name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, false);
+    if (!region)
+        return ENOMEM;
     if (!region->map(page_directory()))
         return ENOMEM;
-    return &add_region(move(region));
+    return &add_region(region.release_nonnull());
 }
 
 KResultOr<Region*> Space::allocate_region_with_vmobject(Range const& range, NonnullRefPtr<VMObject> vmobject, size_t offset_in_vmobject, StringView name, int prot, bool shared)
@@ -187,12 +192,17 @@ KResultOr<Region*> Space::allocate_region_with_vmobject(Range const& range, Nonn
         return EINVAL;
     }
     offset_in_vmobject &= PAGE_MASK;
-    auto& region = add_region(Region::create_user_accessible(m_process, range, move(vmobject), offset_in_vmobject, KString::try_create(name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, shared));
-    if (!region.map(page_directory())) {
+    auto region = Region::try_create_user_accessible(m_process, range, move(vmobject), offset_in_vmobject, KString::try_create(name), prot_to_region_access_flags(prot), Region::Cacheable::Yes, shared);
+    if (!region) {
+        dbgln("allocate_region_with_vmobject: Unable to allocate Region");
+        return nullptr;
+    }
+    auto& region_ref = add_region(region.release_nonnull());
+    if (!region_ref.map(page_directory())) {
         // FIXME: What is an appropriate error code here, really?
         return ENOMEM;
     }
-    return &region;
+    return &region_ref;
 }
 
 bool Space::deallocate_region(Region& region)