diff --git a/Kernel/Devices/BXVGADevice.cpp b/Kernel/Devices/BXVGADevice.cpp index 67d96bd985..44afeb24b1 100644 --- a/Kernel/Devices/BXVGADevice.cpp +++ b/Kernel/Devices/BXVGADevice.cpp @@ -185,7 +185,7 @@ KResultOr BXVGADevice::mmap(Process& process, FileDescription&, Virtual auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes()); if (!vmobject) return KResult(-ENOMEM); - auto* region = process.allocate_region_with_vmobject( + return process.allocate_region_with_vmobject( preferred_vaddr, framebuffer_size_in_bytes(), vmobject.release_nonnull(), @@ -193,10 +193,6 @@ KResultOr BXVGADevice::mmap(Process& process, FileDescription&, Virtual "BXVGA Framebuffer", prot, shared); - if (!region) - return KResult(-ENOMEM); - dbgln("BXVGADevice: mmap with size {} at {}", region->size(), region->vaddr()); - return region; } int BXVGADevice::ioctl(FileDescription&, unsigned request, FlatPtr arg) diff --git a/Kernel/Devices/MBVGADevice.cpp b/Kernel/Devices/MBVGADevice.cpp index 4ddb3c8610..3ae439a689 100644 --- a/Kernel/Devices/MBVGADevice.cpp +++ b/Kernel/Devices/MBVGADevice.cpp @@ -61,7 +61,7 @@ KResultOr MBVGADevice::mmap(Process& process, FileDescription&, Virtual auto vmobject = AnonymousVMObject::create_for_physical_range(m_framebuffer_address, framebuffer_size_in_bytes()); if (!vmobject) return KResult(-ENOMEM); - auto* region = process.allocate_region_with_vmobject( + return process.allocate_region_with_vmobject( preferred_vaddr, framebuffer_size_in_bytes(), vmobject.release_nonnull(), @@ -69,10 +69,6 @@ KResultOr MBVGADevice::mmap(Process& process, FileDescription&, Virtual "MBVGA Framebuffer", prot, shared); - if (!region) - return KResult(-ENOMEM); - dbgln("MBVGADevice: mmap with size {} at {}", region->size(), region->vaddr()); - return region; } int MBVGADevice::ioctl(FileDescription&, unsigned request, FlatPtr arg) diff --git a/Kernel/FileSystem/AnonymousFile.cpp b/Kernel/FileSystem/AnonymousFile.cpp index 676bee25e5..3b2059e633 100644 --- a/Kernel/FileSystem/AnonymousFile.cpp +++ b/Kernel/FileSystem/AnonymousFile.cpp @@ -47,11 +47,7 @@ KResultOr AnonymousFile::mmap(Process& process, FileDescription&, Virtu if (size != m_vmobject->size()) return KResult(-EINVAL); - auto* region = process.allocate_region_with_vmobject(preferred_vaddr, size, m_vmobject, offset, {}, prot, shared); - if (!region) - return KResult(-ENOMEM); - - return region; + return process.allocate_region_with_vmobject(preferred_vaddr, size, m_vmobject, offset, {}, prot, shared); } } diff --git a/Kernel/FileSystem/InodeFile.cpp b/Kernel/FileSystem/InodeFile.cpp index c0c1c5f830..58a3f2459e 100644 --- a/Kernel/FileSystem/InodeFile.cpp +++ b/Kernel/FileSystem/InodeFile.cpp @@ -79,10 +79,7 @@ KResultOr InodeFile::mmap(Process& process, FileDescription& descriptio vmobject = PrivateInodeVMObject::create_with_inode(inode()); if (!vmobject) return KResult(-ENOMEM); - auto* region = process.allocate_region_with_vmobject(preferred_vaddr, size, *vmobject, offset, description.absolute_path(), prot, shared); - if (!region) - return KResult(-ENOMEM); - return region; + return process.allocate_region_with_vmobject(preferred_vaddr, size, *vmobject, offset, description.absolute_path(), prot, shared); } String InodeFile::absolute_path(const FileDescription& description) const diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index 1e30628a74..3f93912f1a 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -142,54 +142,56 @@ Region& Process::allocate_split_region(const Region& source_region, const Range& return region; } -Region* Process::allocate_region(const Range& range, const String& name, int prot, AllocationStrategy strategy) +KResultOr Process::allocate_region(const Range& range, const String& name, int prot, AllocationStrategy strategy) { ASSERT(range.is_valid()); auto vmobject = AnonymousVMObject::create_with_size(range.size(), strategy); if (!vmobject) - return nullptr; + return KResult(-ENOMEM); auto region = Region::create_user_accessible(this, range, vmobject.release_nonnull(), 0, name, prot_to_region_access_flags(prot)); if (!region->map(page_directory())) - return nullptr; + return KResult(-ENOMEM); return &add_region(move(region)); } -Region* Process::allocate_region(VirtualAddress vaddr, size_t size, const String& name, int prot, AllocationStrategy strategy) +KResultOr Process::allocate_region(VirtualAddress vaddr, size_t size, const String& name, int prot, AllocationStrategy strategy) { auto range = allocate_range(vaddr, size); if (!range.is_valid()) - return nullptr; + return KResult(-ENOMEM); return allocate_region(range, name, prot, strategy); } -Region* Process::allocate_region_with_vmobject(const Range& range, NonnullRefPtr vmobject, size_t offset_in_vmobject, const String& name, int prot, bool shared) +KResultOr Process::allocate_region_with_vmobject(const Range& range, NonnullRefPtr vmobject, size_t offset_in_vmobject, const String& name, int prot, bool shared) { ASSERT(range.is_valid()); size_t end_in_vmobject = offset_in_vmobject + range.size(); if (end_in_vmobject <= offset_in_vmobject) { dbgln("allocate_region_with_vmobject: Overflow (offset + size)"); - return nullptr; + return KResult(-EINVAL); } if (offset_in_vmobject >= vmobject->size()) { dbgln("allocate_region_with_vmobject: Attempt to allocate a region with an offset past the end of its VMObject."); - return nullptr; + return KResult(-EINVAL); } if (end_in_vmobject > vmobject->size()) { dbgln("allocate_region_with_vmobject: Attempt to allocate a region with an end past the end of its VMObject."); - return nullptr; + return KResult(-EINVAL); } offset_in_vmobject &= PAGE_MASK; auto& region = add_region(Region::create_user_accessible(this, range, move(vmobject), offset_in_vmobject, name, prot_to_region_access_flags(prot), true, shared)); - if (!region.map(page_directory())) - return nullptr; + if (!region.map(page_directory())) { + // FIXME: What is an appropriate error code here, really? + return KResult(-ENOMEM); + } return ®ion; } -Region* Process::allocate_region_with_vmobject(VirtualAddress vaddr, size_t size, NonnullRefPtr vmobject, size_t offset_in_vmobject, const String& name, int prot, bool shared) +KResultOr Process::allocate_region_with_vmobject(VirtualAddress vaddr, size_t size, NonnullRefPtr vmobject, size_t offset_in_vmobject, const String& name, int prot, bool shared) { auto range = allocate_range(vaddr, size); if (!range.is_valid()) - return nullptr; + return KResult(-ENOMEM); return allocate_region_with_vmobject(range, move(vmobject), offset_in_vmobject, name, prot, shared); } diff --git a/Kernel/Process.h b/Kernel/Process.h index 756317ea20..d504224ddd 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -442,10 +442,10 @@ public: return m_euid == 0; } - Region* allocate_region_with_vmobject(VirtualAddress, size_t, NonnullRefPtr, size_t offset_in_vmobject, const String& name, int prot, bool shared); - Region* allocate_region(VirtualAddress, size_t, const String& name, int prot = PROT_READ | PROT_WRITE, AllocationStrategy strategy = AllocationStrategy::Reserve); - Region* allocate_region_with_vmobject(const Range&, NonnullRefPtr, size_t offset_in_vmobject, const String& name, int prot, bool shared); - Region* allocate_region(const Range&, const String& name, int prot = PROT_READ | PROT_WRITE, AllocationStrategy strategy = AllocationStrategy::Reserve); + KResultOr allocate_region_with_vmobject(VirtualAddress, size_t, NonnullRefPtr, size_t offset_in_vmobject, const String& name, int prot, bool shared); + KResultOr allocate_region(VirtualAddress, size_t, const String& name, int prot = PROT_READ | PROT_WRITE, AllocationStrategy strategy = AllocationStrategy::Reserve); + KResultOr allocate_region_with_vmobject(const Range&, NonnullRefPtr, size_t offset_in_vmobject, const String& name, int prot, bool shared); + KResultOr allocate_region(const Range&, const String& name, int prot = PROT_READ | PROT_WRITE, AllocationStrategy strategy = AllocationStrategy::Reserve); bool deallocate_region(Region& region); Region& allocate_split_region(const Region& source_region, const Range&, size_t offset_in_vmobject); diff --git a/Kernel/SharedBuffer.cpp b/Kernel/SharedBuffer.cpp index 4713e8da9f..bcf72a429a 100644 --- a/Kernel/SharedBuffer.cpp +++ b/Kernel/SharedBuffer.cpp @@ -94,10 +94,10 @@ void* SharedBuffer::ref_for_process_and_get_address(Process& process) for (auto& ref : m_refs) { if (ref.pid == process.pid()) { if (!ref.region) { - auto* region = process.allocate_region_with_vmobject(VirtualAddress(), size(), m_vmobject, 0, "SharedBuffer", PROT_READ | (m_writable ? PROT_WRITE : 0), true); - if (!region) - return (void*)-ENOMEM; - ref.region = region; + auto region_or_error = process.allocate_region_with_vmobject(VirtualAddress(), size(), m_vmobject, 0, "SharedBuffer", PROT_READ | (m_writable ? PROT_WRITE : 0), true); + if (region_or_error.is_error()) + return (void*)region_or_error.error().error(); + ref.region = region_or_error.value(); } ref.count++; m_total_refs++; diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index 7ffce9787e..4d302fb19c 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -142,13 +142,13 @@ KResultOr Process::load_elf_object(FileDescription& object_ size_t executable_size = inode.size(); - auto region = MM.allocate_kernel_region_with_vmobject(*vmobject, PAGE_ROUND_UP(executable_size), "ELF loading", Region::Access::Read); - if (!region) { + auto executable_region = MM.allocate_kernel_region_with_vmobject(*vmobject, PAGE_ROUND_UP(executable_size), "ELF loading", Region::Access::Read); + if (!executable_region) { dbgln("Could not allocate memory for ELF loading"); return KResult(-ENOMEM); } - auto elf_image = ELF::Image(region->vaddr().as_ptr(), executable_size); + auto elf_image = ELF::Image(executable_region->vaddr().as_ptr(), executable_size); if (!elf_image.is_valid()) return KResult(-ENOEXEC); @@ -173,11 +173,12 @@ KResultOr Process::load_elf_object(FileDescription& object_ return IterationDecision::Break; } - master_tls_region = allocate_region({}, program_header.size_in_memory(), String::formatted("{} (master-tls)", elf_name), PROT_READ | PROT_WRITE, AllocationStrategy::Reserve); - if (!master_tls_region) { - ph_load_result = KResult(-ENOMEM); + auto region_or_error = allocate_region({}, program_header.size_in_memory(), String::formatted("{} (master-tls)", elf_name), PROT_READ | PROT_WRITE, AllocationStrategy::Reserve); + if (region_or_error.is_error()) { + ph_load_result = region_or_error.error(); return IterationDecision::Break; } + master_tls_region = region_or_error.value(); master_tls_size = program_header.size_in_memory(); master_tls_alignment = program_header.alignment(); @@ -207,9 +208,9 @@ KResultOr Process::load_elf_object(FileDescription& object_ if (program_header.is_writable()) prot |= PROT_WRITE; auto region_name = String::formatted("{} (data-{}{})", elf_name, program_header.is_readable() ? "r" : "", program_header.is_writable() ? "w" : ""); - auto* region = allocate_region(program_header.vaddr().offset(load_offset), program_header.size_in_memory(), move(region_name), prot, AllocationStrategy::Reserve); - if (!region) { - ph_load_result = KResult(-ENOMEM); + auto region_or_error = allocate_region(program_header.vaddr().offset(load_offset), program_header.size_in_memory(), move(region_name), prot, AllocationStrategy::Reserve); + if (region_or_error.is_error()) { + ph_load_result = region_or_error.error(); return IterationDecision::Break; } @@ -222,7 +223,7 @@ KResultOr Process::load_elf_object(FileDescription& object_ // Accessing it would definitely be a bug. auto page_offset = program_header.vaddr(); page_offset.mask(~PAGE_MASK); - if (!copy_to_user((u8*)region->vaddr().as_ptr() + page_offset.get(), program_header.raw_data(), program_header.size_in_image())) { + if (!copy_to_user((u8*)region_or_error.value()->vaddr().as_ptr() + page_offset.get(), program_header.raw_data(), program_header.size_in_image())) { ph_load_result = KResult(-EFAULT); return IterationDecision::Break; } @@ -239,13 +240,13 @@ KResultOr Process::load_elf_object(FileDescription& object_ prot |= PROT_WRITE; if (program_header.is_executable()) prot |= PROT_EXEC; - auto* region = allocate_region_with_vmobject(program_header.vaddr().offset(load_offset), program_header.size_in_memory(), *vmobject, program_header.offset(), elf_name, prot, true); - if (!region) { - ph_load_result = KResult(-ENOMEM); + auto region_or_error = allocate_region_with_vmobject(program_header.vaddr().offset(load_offset), program_header.size_in_memory(), *vmobject, program_header.offset(), elf_name, prot, true); + if (region_or_error.is_error()) { + ph_load_result = region_or_error.error(); return IterationDecision::Break; } if (program_header.offset() == 0) - load_base_address = (FlatPtr)region->vaddr().as_ptr(); + load_base_address = (FlatPtr)region_or_error.value()->vaddr().as_ptr(); return IterationDecision::Continue; }); @@ -259,10 +260,11 @@ KResultOr Process::load_elf_object(FileDescription& object_ return KResult(-ENOEXEC); } - auto* stack_region = allocate_region(VirtualAddress(), Thread::default_userspace_stack_size, "Stack (Main thread)", PROT_READ | PROT_WRITE, AllocationStrategy::Reserve); - if (!stack_region) - return KResult(-ENOMEM); - stack_region->set_stack(true); + auto stack_region_or_error = allocate_region(VirtualAddress(), Thread::default_userspace_stack_size, "Stack (Main thread)", PROT_READ | PROT_WRITE, AllocationStrategy::Reserve); + if (stack_region_or_error.is_error()) + return stack_region_or_error.error(); + auto& stack_region = *stack_region_or_error.value(); + stack_region.set_stack(true); return LoadResult { load_base_address, @@ -273,7 +275,7 @@ KResultOr Process::load_elf_object(FileDescription& object_ AK::try_make_weak_ptr(master_tls_region), master_tls_size, master_tls_alignment, - stack_region->make_weak_ptr() + stack_region.make_weak_ptr() }; } diff --git a/Kernel/Syscalls/mmap.cpp b/Kernel/Syscalls/mmap.cpp index b1ccb31b8e..0681e7c438 100644 --- a/Kernel/Syscalls/mmap.cpp +++ b/Kernel/Syscalls/mmap.cpp @@ -142,9 +142,12 @@ void* Process::sys$mmap(Userspace user_params) if (map_anonymous) { auto strategy = map_noreserve ? AllocationStrategy::None : AllocationStrategy::Reserve; - region = allocate_region(range.value(), !name.is_null() ? name : "mmap", prot, strategy); - if (!region && (!map_fixed && addr != 0)) - region = allocate_region(allocate_range({}, size), !name.is_null() ? name : "mmap", prot, strategy); + auto region_or_error = allocate_region(range.value(), !name.is_null() ? name : "mmap", prot, strategy); + if (region_or_error.is_error() && (!map_fixed && addr != 0)) + region_or_error = allocate_region(allocate_range({}, size), !name.is_null() ? name : "mmap", prot, strategy); + if (region_or_error.is_error()) + return (void*)region_or_error.error().error(); + region = region_or_error.value(); } else { if (offset < 0) return (void*)-EINVAL; @@ -170,11 +173,11 @@ void* Process::sys$mmap(Userspace user_params) if (region_or_error.is_error()) { // Fail if MAP_FIXED or address is 0, retry otherwise if (map_fixed || addr == 0) - return (void*)(int)region_or_error.error(); + return (void*)region_or_error.error().error(); region_or_error = description->mmap(*this, {}, static_cast(offset), size, prot, map_shared); } if (region_or_error.is_error()) - return (void*)(int)region_or_error.error(); + return (void*)region_or_error.error().error(); region = region_or_error.value(); } @@ -438,13 +441,12 @@ void* Process::sys$mremap(Userspace user_param deallocate_region(*old_region); auto new_vmobject = PrivateInodeVMObject::create_with_inode(inode); - auto* new_region = allocate_region_with_vmobject(range.base(), range.size(), new_vmobject, 0, old_name, old_prot, false); - new_region->set_mmap(true); - - if (!new_region) - return (void*)-ENOMEM; - - return new_region->vaddr().as_ptr(); + auto new_region_or_error = allocate_region_with_vmobject(range.base(), range.size(), new_vmobject, 0, old_name, old_prot, false); + if (new_region_or_error.is_error()) + return (void*)new_region_or_error.error().error(); + auto& new_region = *new_region_or_error.value(); + new_region.set_mmap(true); + return new_region.vaddr().as_ptr(); } dbgln("sys$mremap: Unimplemented remap request (flags={})", params.flags); @@ -471,11 +473,11 @@ void* Process::sys$allocate_tls(size_t size) }); ASSERT(main_thread); - auto tls_region = allocate_region({}, size, String(), PROT_READ | PROT_WRITE); - if (!tls_region) - return (void*)-EFAULT; + auto region_or_error = allocate_region({}, size, String(), PROT_READ | PROT_WRITE); + if (region_or_error.is_error()) + return (void*)region_or_error.error().error(); - m_master_tls_region = tls_region->make_weak_ptr(); + m_master_tls_region = region_or_error.value()->make_weak_ptr(); m_master_tls_size = size; m_master_tls_alignment = PAGE_SIZE; diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index 871187b45b..f76feef359 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -1071,12 +1071,12 @@ KResult Thread::make_thread_specific_region(Badge) if (!process().m_master_tls_region) return KSuccess; - auto* region = process().allocate_region({}, thread_specific_region_size(), "Thread-specific", PROT_READ | PROT_WRITE); - if (!region) - return KResult(-ENOMEM); + auto region_or_error = process().allocate_region({}, thread_specific_region_size(), "Thread-specific", PROT_READ | PROT_WRITE); + if (region_or_error.is_error()) + return region_or_error.error(); SmapDisabler disabler; - auto* thread_specific_data = (ThreadSpecificData*)region->vaddr().offset(align_up_to(process().m_master_tls_size, thread_specific_region_alignment())).as_ptr(); + auto* thread_specific_data = (ThreadSpecificData*)region_or_error.value()->vaddr().offset(align_up_to(process().m_master_tls_size, thread_specific_region_alignment())).as_ptr(); auto* thread_local_storage = (u8*)((u8*)thread_specific_data) - align_up_to(process().m_master_tls_size, process().m_master_tls_alignment); m_thread_specific_data = VirtualAddress(thread_specific_data); thread_specific_data->self = thread_specific_data;