From 65d5f81afc6b5820dded3ff09ca8cdfe6b57acef Mon Sep 17 00:00:00 2001 From: Brian Gianforcaro Date: Fri, 28 May 2021 03:18:07 -0700 Subject: [PATCH] Kernel: Make PrivateInodeVMObject factory APIs OOM safe --- Kernel/Syscalls/mmap.cpp | 8 +++++--- Kernel/Syscalls/ptrace.cpp | 5 ++++- Kernel/VM/PrivateInodeVMObject.cpp | 6 +++--- Kernel/VM/PrivateInodeVMObject.h | 2 +- 4 files changed, 13 insertions(+), 8 deletions(-) diff --git a/Kernel/Syscalls/mmap.cpp b/Kernel/Syscalls/mmap.cpp index 1dddb431e5..72f0ef3f6c 100644 --- a/Kernel/Syscalls/mmap.cpp +++ b/Kernel/Syscalls/mmap.cpp @@ -632,14 +632,16 @@ KResultOr Process::sys$mremap(Userspaceoffset_in_vmobject(); NonnullRefPtr inode = static_cast(old_region->vmobject()).inode(); + auto new_vmobject = PrivateInodeVMObject::create_with_inode(inode); + if (!new_vmobject) + return ENOMEM; + // Unmap without deallocating the VM range since we're going to reuse it. old_region->unmap(Region::ShouldDeallocateVirtualMemoryRange::No); bool success = space().deallocate_region(*old_region); VERIFY(success); - auto new_vmobject = PrivateInodeVMObject::create_with_inode(inode); - - auto new_region_or_error = space().allocate_region_with_vmobject(range, new_vmobject, old_offset, old_name, old_prot, false); + auto new_region_or_error = space().allocate_region_with_vmobject(range, new_vmobject.release_nonnull(), old_offset, old_name, old_prot, false); if (new_region_or_error.is_error()) return new_region_or_error.error().error(); auto& new_region = *new_region_or_error.value(); diff --git a/Kernel/Syscalls/ptrace.cpp b/Kernel/Syscalls/ptrace.cpp index 1b9695f410..c8a210b701 100644 --- a/Kernel/Syscalls/ptrace.cpp +++ b/Kernel/Syscalls/ptrace.cpp @@ -201,7 +201,10 @@ KResult Process::poke_user_data(Userspace address, u32 data) // If the region is shared, we change its vmobject to a PrivateInodeVMObject // to prevent the write operation from changing any shared inode data VERIFY(region->vmobject().is_shared_inode()); - region->set_vmobject(PrivateInodeVMObject::create_with_inode(static_cast(region->vmobject()).inode())); + auto vmobject = PrivateInodeVMObject::create_with_inode(static_cast(region->vmobject()).inode()); + if (!vmobject) + return ENOMEM; + region->set_vmobject(vmobject.release_nonnull()); region->set_shared(false); } const bool was_writable = region->is_writable(); diff --git a/Kernel/VM/PrivateInodeVMObject.cpp b/Kernel/VM/PrivateInodeVMObject.cpp index 18aa0fb21f..1ddd894089 100644 --- a/Kernel/VM/PrivateInodeVMObject.cpp +++ b/Kernel/VM/PrivateInodeVMObject.cpp @@ -9,14 +9,14 @@ namespace Kernel { -NonnullRefPtr PrivateInodeVMObject::create_with_inode(Inode& inode) +RefPtr PrivateInodeVMObject::create_with_inode(Inode& inode) { - return adopt_ref(*new PrivateInodeVMObject(inode, inode.size())); + return adopt_ref_if_nonnull(new PrivateInodeVMObject(inode, inode.size())); } RefPtr PrivateInodeVMObject::clone() { - return adopt_ref(*new PrivateInodeVMObject(*this)); + return adopt_ref_if_nonnull(new PrivateInodeVMObject(*this)); } PrivateInodeVMObject::PrivateInodeVMObject(Inode& inode, size_t size) diff --git a/Kernel/VM/PrivateInodeVMObject.h b/Kernel/VM/PrivateInodeVMObject.h index 1b483c5898..616f7ab0b0 100644 --- a/Kernel/VM/PrivateInodeVMObject.h +++ b/Kernel/VM/PrivateInodeVMObject.h @@ -18,7 +18,7 @@ class PrivateInodeVMObject final : public InodeVMObject { public: virtual ~PrivateInodeVMObject() override; - static NonnullRefPtr create_with_inode(Inode&); + static RefPtr create_with_inode(Inode&); virtual RefPtr clone() override; private: