diff options
author | Andreas Kling <kling@serenityos.org> | 2022-08-23 17:58:05 +0200 |
---|---|---|
committer | Andreas Kling <kling@serenityos.org> | 2022-08-24 14:57:51 +0200 |
commit | cf16b2c8e64709d570c5f54a981017d217e95ed0 (patch) | |
tree | 16c9efdaaa579ae51682a51a58949ce02c3d2092 /Kernel | |
parent | d6ef18f587d4a7e4f58487c84e0b9eb260f3ec5a (diff) | |
download | serenity-cf16b2c8e64709d570c5f54a981017d217e95ed0.zip |
Kernel: Wrap process address spaces in SpinlockProtected
This forces anyone who wants to look into and/or manipulate an address
space to lock it. And this replaces the previous, more flimsy, manual
spinlock use.
Note that pointers *into* the address space are not safe to use after
you unlock the space. We've got many issues like this, and we'll have
to track those down as wlel.
Diffstat (limited to 'Kernel')
38 files changed, 712 insertions, 631 deletions
diff --git a/Kernel/Arch/x86/common/CrashHandler.cpp b/Kernel/Arch/x86/common/CrashHandler.cpp index df0ba7c214..c33276717a 100644 --- a/Kernel/Arch/x86/common/CrashHandler.cpp +++ b/Kernel/Arch/x86/common/CrashHandler.cpp @@ -35,7 +35,7 @@ void handle_crash(Kernel::RegisterState const& regs, char const* description, in dump_registers(regs); if (crashed_in_kernel) { - process.address_space().dump_regions(); + process.address_space().with([&](auto& space) { space->dump_regions(); }); PANIC("Crash in ring 0"); } diff --git a/Kernel/Arch/x86/common/Interrupts.cpp b/Kernel/Arch/x86/common/Interrupts.cpp index e91de65693..604a74da33 100644 --- a/Kernel/Arch/x86/common/Interrupts.cpp +++ b/Kernel/Arch/x86/common/Interrupts.cpp @@ -303,9 +303,15 @@ void page_fault_handler(TrapFrame* trap) }; VirtualAddress userspace_sp = VirtualAddress { regs.userspace_sp() }; - if (!faulted_in_kernel && !MM.validate_user_stack(current_thread->process().address_space(), userspace_sp)) { - dbgln("Invalid stack pointer: {}", userspace_sp); - return handle_crash(regs, "Bad stack on page fault", SIGSEGV); + + if (!faulted_in_kernel) { + bool has_valid_stack_pointer = current_thread->process().address_space().with([&](auto& space) { + return MM.validate_user_stack(*space, userspace_sp); + }); + if (!has_valid_stack_pointer) { + dbgln("Invalid stack pointer: {}", userspace_sp); + return handle_crash(regs, "Bad stack on page fault", SIGSEGV); + } } PageFault fault { regs.exception_code, VirtualAddress { fault_address } }; diff --git a/Kernel/Arch/x86/common/Spinlock.cpp b/Kernel/Arch/x86/common/Spinlock.cpp index 4de2eecfc0..5af0b61495 100644 --- a/Kernel/Arch/x86/common/Spinlock.cpp +++ b/Kernel/Arch/x86/common/Spinlock.cpp @@ -55,6 +55,7 @@ u32 RecursiveSpinlock::lock() void RecursiveSpinlock::unlock(u32 prev_flags) { + VERIFY_INTERRUPTS_DISABLED(); VERIFY(m_recursions > 0); VERIFY(m_lock.load(AK::memory_order_relaxed) == FlatPtr(&Processor::current())); if (--m_recursions == 0) { diff --git a/Kernel/Coredump.cpp b/Kernel/Coredump.cpp index 1c7f3add12..a57ac038c0 100644 --- a/Kernel/Coredump.cpp +++ b/Kernel/Coredump.cpp @@ -46,17 +46,19 @@ Coredump::Coredump(NonnullLockRefPtr<Process> process, NonnullLockRefPtr<OpenFil , m_description(move(description)) { m_num_program_headers = 0; - m_process->address_space().region_tree().with([&](auto& region_tree) { - for (auto& region : region_tree.regions()) { + m_process->address_space().with([&](auto& space) { + space->region_tree().with([&](auto& region_tree) { + for (auto& region : region_tree.regions()) { #if !INCLUDE_USERSPACE_HEAP_MEMORY_IN_COREDUMPS - if (looks_like_userspace_heap_region(region)) - continue; + if (looks_like_userspace_heap_region(region)) + continue; #endif - if (region.access() == Memory::Region::Access::None) - continue; - ++m_num_program_headers; - } + if (region.access() == Memory::Region::Access::None) + continue; + ++m_num_program_headers; + } + }); }); ++m_num_program_headers; // +1 for NOTE segment } @@ -135,38 +137,40 @@ ErrorOr<void> Coredump::write_elf_header() ErrorOr<void> Coredump::write_program_headers(size_t notes_size) { size_t offset = sizeof(ElfW(Ehdr)) + m_num_program_headers * sizeof(ElfW(Phdr)); - m_process->address_space().region_tree().with([&](auto& region_tree) { - for (auto& region : region_tree.regions()) { + m_process->address_space().with([&](auto& space) { + space->region_tree().with([&](auto& region_tree) { + for (auto& region : region_tree.regions()) { #if !INCLUDE_USERSPACE_HEAP_MEMORY_IN_COREDUMPS - if (looks_like_userspace_heap_region(region)) - continue; + if (looks_like_userspace_heap_region(region)) + continue; #endif - if (region.access() == Memory::Region::Access::None) - continue; + if (region.access() == Memory::Region::Access::None) + continue; - ElfW(Phdr) phdr {}; + ElfW(Phdr) phdr {}; - phdr.p_type = PT_LOAD; - phdr.p_offset = offset; - phdr.p_vaddr = region.vaddr().get(); - phdr.p_paddr = 0; + phdr.p_type = PT_LOAD; + phdr.p_offset = offset; + phdr.p_vaddr = region.vaddr().get(); + phdr.p_paddr = 0; - phdr.p_filesz = region.page_count() * PAGE_SIZE; - phdr.p_memsz = region.page_count() * PAGE_SIZE; - phdr.p_align = 0; + phdr.p_filesz = region.page_count() * PAGE_SIZE; + phdr.p_memsz = region.page_count() * PAGE_SIZE; + phdr.p_align = 0; - phdr.p_flags = region.is_readable() ? PF_R : 0; - if (region.is_writable()) - phdr.p_flags |= PF_W; - if (region.is_executable()) - phdr.p_flags |= PF_X; + phdr.p_flags = region.is_readable() ? PF_R : 0; + if (region.is_writable()) + phdr.p_flags |= PF_W; + if (region.is_executable()) + phdr.p_flags |= PF_X; - offset += phdr.p_filesz; + offset += phdr.p_filesz; - [[maybe_unused]] auto rc = m_description->write(UserOrKernelBuffer::for_kernel_buffer(reinterpret_cast<uint8_t*>(&phdr)), sizeof(ElfW(Phdr))); - } + [[maybe_unused]] auto rc = m_description->write(UserOrKernelBuffer::for_kernel_buffer(reinterpret_cast<uint8_t*>(&phdr)), sizeof(ElfW(Phdr))); + } + }); }); ElfW(Phdr) notes_pheader {}; @@ -188,37 +192,39 @@ ErrorOr<void> Coredump::write_regions() { u8 zero_buffer[PAGE_SIZE] = {}; - return m_process->address_space().region_tree().with([&](auto& region_tree) -> ErrorOr<void> { - for (auto& region : region_tree.regions()) { - VERIFY(!region.is_kernel()); + return m_process->address_space().with([&](auto& space) { + return space->region_tree().with([&](auto& region_tree) -> ErrorOr<void> { + for (auto& region : region_tree.regions()) { + VERIFY(!region.is_kernel()); #if !INCLUDE_USERSPACE_HEAP_MEMORY_IN_COREDUMPS - if (looks_like_userspace_heap_region(region)) - continue; + if (looks_like_userspace_heap_region(region)) + continue; #endif - if (region.access() == Memory::Region::Access::None) - continue; - - // If we crashed in the middle of mapping in Regions, they do not have a page directory yet, and will crash on a remap() call - if (!region.is_mapped()) - continue; - - region.set_readable(true); - region.remap(); - - for (size_t i = 0; i < region.page_count(); i++) { - auto page = region.physical_page(i); - auto src_buffer = [&]() -> ErrorOr<UserOrKernelBuffer> { - if (page) - return UserOrKernelBuffer::for_user_buffer(reinterpret_cast<uint8_t*>((region.vaddr().as_ptr() + (i * PAGE_SIZE))), PAGE_SIZE); - // If the current page is not backed by a physical page, we zero it in the coredump file. - return UserOrKernelBuffer::for_kernel_buffer(zero_buffer); - }(); - TRY(m_description->write(src_buffer.value(), PAGE_SIZE)); + if (region.access() == Memory::Region::Access::None) + continue; + + // If we crashed in the middle of mapping in Regions, they do not have a page directory yet, and will crash on a remap() call + if (!region.is_mapped()) + continue; + + region.set_readable(true); + region.remap(); + + for (size_t i = 0; i < region.page_count(); i++) { + auto page = region.physical_page(i); + auto src_buffer = [&]() -> ErrorOr<UserOrKernelBuffer> { + if (page) + return UserOrKernelBuffer::for_user_buffer(reinterpret_cast<uint8_t*>((region.vaddr().as_ptr() + (i * PAGE_SIZE))), PAGE_SIZE); + // If the current page is not backed by a physical page, we zero it in the coredump file. + return UserOrKernelBuffer::for_kernel_buffer(zero_buffer); + }(); + TRY(m_description->write(src_buffer.value(), PAGE_SIZE)); + } } - } - return {}; + return {}; + }); }); } @@ -279,34 +285,36 @@ ErrorOr<void> Coredump::create_notes_threads_data(auto& builder) const ErrorOr<void> Coredump::create_notes_regions_data(auto& builder) const { size_t region_index = 0; - return m_process->address_space().region_tree().with([&](auto& region_tree) -> ErrorOr<void> { - for (auto const& region : region_tree.regions()) { + return m_process->address_space().with([&](auto& space) { + return space->region_tree().with([&](auto& region_tree) -> ErrorOr<void> { + for (auto const& region : region_tree.regions()) { #if !INCLUDE_USERSPACE_HEAP_MEMORY_IN_COREDUMPS - if (looks_like_userspace_heap_region(region)) - continue; + if (looks_like_userspace_heap_region(region)) + continue; #endif - if (region.access() == Memory::Region::Access::None) - continue; + if (region.access() == Memory::Region::Access::None) + continue; - ELF::Core::MemoryRegionInfo info {}; - info.header.type = ELF::Core::NotesEntryHeader::Type::MemoryRegionInfo; + ELF::Core::MemoryRegionInfo info {}; + info.header.type = ELF::Core::NotesEntryHeader::Type::MemoryRegionInfo; - info.region_start = region.vaddr().get(); - info.region_end = region.vaddr().offset(region.size()).get(); - info.program_header_index = region_index++; + info.region_start = region.vaddr().get(); + info.region_end = region.vaddr().offset(region.size()).get(); + info.program_header_index = region_index++; - TRY(builder.append_bytes(ReadonlyBytes { (void*)&info, sizeof(info) })); + TRY(builder.append_bytes(ReadonlyBytes { (void*)&info, sizeof(info) })); - // NOTE: The region name *is* null-terminated, so the following is ok: - auto name = region.name(); - if (name.is_empty()) - TRY(builder.append('\0')); - else - TRY(builder.append(name.characters_without_null_termination(), name.length() + 1)); - } - return {}; + // NOTE: The region name *is* null-terminated, so the following is ok: + auto name = region.name(); + if (name.is_empty()) + TRY(builder.append('\0')); + else + TRY(builder.append(name.characters_without_null_termination(), name.length() + 1)); + } + return {}; + }); }); } @@ -344,7 +352,6 @@ ErrorOr<void> Coredump::create_notes_segment_data(auto& builder) const ErrorOr<void> Coredump::write() { - SpinlockLocker lock(m_process->address_space().get_lock()); ScopedAddressSpaceSwitcher switcher(m_process); auto builder = TRY(KBufferBuilder::try_create()); diff --git a/Kernel/Devices/KCOVDevice.cpp b/Kernel/Devices/KCOVDevice.cpp index 7c326e7e70..4d38ce1505 100644 --- a/Kernel/Devices/KCOVDevice.cpp +++ b/Kernel/Devices/KCOVDevice.cpp @@ -116,7 +116,7 @@ ErrorOr<void> KCOVDevice::ioctl(OpenFileDescription&, unsigned request, Userspac } } -ErrorOr<Memory::Region*> KCOVDevice::mmap(Process& process, OpenFileDescription&, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) +ErrorOr<Memory::Region*> KCOVDevice::mmap(Process& process, Memory::AddressSpace& address_space, OpenFileDescription&, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) { auto pid = process.pid(); auto maybe_kcov_instance = proc_instance->get(pid); @@ -126,8 +126,7 @@ ErrorOr<Memory::Region*> KCOVDevice::mmap(Process& process, OpenFileDescription& if (!kcov_instance->vmobject()) return ENOBUFS; // mmaped, before KCOV_SETBUFSIZE - return process.address_space().allocate_region_with_vmobject( - range, *kcov_instance->vmobject(), offset, {}, prot, shared); + return address_space.allocate_region_with_vmobject(range, *kcov_instance->vmobject(), offset, {}, prot, shared); } } diff --git a/Kernel/Devices/KCOVDevice.h b/Kernel/Devices/KCOVDevice.h index f4860fc2ab..bc5883c61f 100644 --- a/Kernel/Devices/KCOVDevice.h +++ b/Kernel/Devices/KCOVDevice.h @@ -22,7 +22,7 @@ public: static void free_process(); // ^File - ErrorOr<Memory::Region*> mmap(Process&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared) override; + ErrorOr<Memory::Region*> mmap(Process&, Memory::AddressSpace&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared) override; ErrorOr<NonnullLockRefPtr<OpenFileDescription>> open(int options) override; protected: diff --git a/Kernel/Devices/MemoryDevice.cpp b/Kernel/Devices/MemoryDevice.cpp index 6f9de0206e..b823498706 100644 --- a/Kernel/Devices/MemoryDevice.cpp +++ b/Kernel/Devices/MemoryDevice.cpp @@ -42,7 +42,7 @@ ErrorOr<size_t> MemoryDevice::read(OpenFileDescription&, u64 offset, UserOrKerne return length; } -ErrorOr<Memory::Region*> MemoryDevice::mmap(Process& process, OpenFileDescription&, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) +ErrorOr<Memory::Region*> MemoryDevice::mmap(Process&, Memory::AddressSpace& address_space, OpenFileDescription&, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) { auto viewed_address = PhysicalAddress(offset); @@ -63,7 +63,7 @@ ErrorOr<Memory::Region*> MemoryDevice::mmap(Process& process, OpenFileDescriptio auto vmobject = TRY(Memory::AnonymousVMObject::try_create_for_physical_range(viewed_address, range.size())); - return process.address_space().allocate_region_with_vmobject( + return address_space.allocate_region_with_vmobject( range, move(vmobject), 0, diff --git a/Kernel/Devices/MemoryDevice.h b/Kernel/Devices/MemoryDevice.h index 0ca5226887..274438c3de 100644 --- a/Kernel/Devices/MemoryDevice.h +++ b/Kernel/Devices/MemoryDevice.h @@ -19,7 +19,7 @@ public: static NonnullLockRefPtr<MemoryDevice> must_create(); ~MemoryDevice(); - virtual ErrorOr<Memory::Region*> mmap(Process&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared) override; + virtual ErrorOr<Memory::Region*> mmap(Process&, Memory::AddressSpace&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared) override; private: MemoryDevice(); diff --git a/Kernel/FileSystem/AnonymousFile.cpp b/Kernel/FileSystem/AnonymousFile.cpp index 40ad6cc19d..850782495f 100644 --- a/Kernel/FileSystem/AnonymousFile.cpp +++ b/Kernel/FileSystem/AnonymousFile.cpp @@ -17,12 +17,12 @@ AnonymousFile::AnonymousFile(NonnullLockRefPtr<Memory::AnonymousVMObject> vmobje AnonymousFile::~AnonymousFile() = default; -ErrorOr<Memory::Region*> AnonymousFile::mmap(Process& process, OpenFileDescription&, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) +ErrorOr<Memory::Region*> AnonymousFile::mmap(Process&, Memory::AddressSpace& address_space, OpenFileDescription&, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) { if (offset != 0) return EINVAL; - return process.address_space().allocate_region_with_vmobject(range, m_vmobject, offset, {}, prot, shared); + return address_space.allocate_region_with_vmobject(range, m_vmobject, offset, {}, prot, shared); } ErrorOr<NonnullOwnPtr<KString>> AnonymousFile::pseudo_path(OpenFileDescription const&) const diff --git a/Kernel/FileSystem/AnonymousFile.h b/Kernel/FileSystem/AnonymousFile.h index 5391d82f28..45500671f9 100644 --- a/Kernel/FileSystem/AnonymousFile.h +++ b/Kernel/FileSystem/AnonymousFile.h @@ -20,7 +20,7 @@ public: virtual ~AnonymousFile() override; - virtual ErrorOr<Memory::Region*> mmap(Process&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared) override; + virtual ErrorOr<Memory::Region*> mmap(Process&, Memory::AddressSpace&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared) override; private: virtual StringView class_name() const override { return "AnonymousFile"sv; } diff --git a/Kernel/FileSystem/File.cpp b/Kernel/FileSystem/File.cpp index 07bb430ae1..fa15688e03 100644 --- a/Kernel/FileSystem/File.cpp +++ b/Kernel/FileSystem/File.cpp @@ -35,7 +35,7 @@ ErrorOr<void> File::ioctl(OpenFileDescription&, unsigned, Userspace<void*>) return ENOTTY; } -ErrorOr<Memory::Region*> File::mmap(Process&, OpenFileDescription&, Memory::VirtualRange const&, u64, int, bool) +ErrorOr<Memory::Region*> File::mmap(Process&, Memory::AddressSpace&, OpenFileDescription&, Memory::VirtualRange const&, u64, int, bool) { return ENODEV; } diff --git a/Kernel/FileSystem/File.h b/Kernel/FileSystem/File.h index 0679639326..456a6757f6 100644 --- a/Kernel/FileSystem/File.h +++ b/Kernel/FileSystem/File.h @@ -90,7 +90,7 @@ public: virtual ErrorOr<size_t> read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) = 0; virtual ErrorOr<size_t> write(OpenFileDescription&, u64, UserOrKernelBuffer const&, size_t) = 0; virtual ErrorOr<void> ioctl(OpenFileDescription&, unsigned request, Userspace<void*> arg); - virtual ErrorOr<Memory::Region*> mmap(Process&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared); + virtual ErrorOr<Memory::Region*> mmap(Process&, Memory::AddressSpace&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared); virtual ErrorOr<struct stat> stat() const { return EBADF; } // Although this might be better described "name" or "description", these terms already have other meanings. diff --git a/Kernel/FileSystem/InodeFile.cpp b/Kernel/FileSystem/InodeFile.cpp index 9a805774bf..8d69cd787f 100644 --- a/Kernel/FileSystem/InodeFile.cpp +++ b/Kernel/FileSystem/InodeFile.cpp @@ -85,7 +85,7 @@ ErrorOr<void> InodeFile::ioctl(OpenFileDescription& description, unsigned reques } } -ErrorOr<Memory::Region*> InodeFile::mmap(Process& process, OpenFileDescription& description, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) +ErrorOr<Memory::Region*> InodeFile::mmap(Process&, Memory::AddressSpace& address_space, OpenFileDescription& description, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) { // FIXME: If PROT_EXEC, check that the underlying file system isn't mounted noexec. LockRefPtr<Memory::InodeVMObject> vmobject; @@ -94,7 +94,7 @@ ErrorOr<Memory::Region*> InodeFile::mmap(Process& process, OpenFileDescription& else vmobject = TRY(Memory::PrivateInodeVMObject::try_create_with_inode(inode())); auto path = TRY(description.pseudo_path()); - return process.address_space().allocate_region_with_vmobject(range, vmobject.release_nonnull(), offset, path->view(), prot, shared); + return address_space.allocate_region_with_vmobject(range, vmobject.release_nonnull(), offset, path->view(), prot, shared); } ErrorOr<NonnullOwnPtr<KString>> InodeFile::pseudo_path(OpenFileDescription const&) const diff --git a/Kernel/FileSystem/InodeFile.h b/Kernel/FileSystem/InodeFile.h index 0b5b6f9c69..c05177362e 100644 --- a/Kernel/FileSystem/InodeFile.h +++ b/Kernel/FileSystem/InodeFile.h @@ -33,7 +33,7 @@ public: virtual ErrorOr<size_t> read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override; virtual ErrorOr<size_t> write(OpenFileDescription&, u64, UserOrKernelBuffer const&, size_t) override; virtual ErrorOr<void> ioctl(OpenFileDescription&, unsigned request, Userspace<void*> arg) override; - virtual ErrorOr<Memory::Region*> mmap(Process&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared) override; + virtual ErrorOr<Memory::Region*> mmap(Process&, Memory::AddressSpace&, OpenFileDescription&, Memory::VirtualRange const&, u64 offset, int prot, bool shared) override; virtual ErrorOr<struct stat> stat() const override { return inode().metadata().stat(); } virtual ErrorOr<NonnullOwnPtr<KString>> pseudo_path(OpenFileDescription const&) const override; diff --git a/Kernel/FileSystem/OpenFileDescription.cpp b/Kernel/FileSystem/OpenFileDescription.cpp index 4331a5ec8c..153a1baeb3 100644 --- a/Kernel/FileSystem/OpenFileDescription.cpp +++ b/Kernel/FileSystem/OpenFileDescription.cpp @@ -374,9 +374,9 @@ InodeMetadata OpenFileDescription::metadata() const return {}; } -ErrorOr<Memory::Region*> OpenFileDescription::mmap(Process& process, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) +ErrorOr<Memory::Region*> OpenFileDescription::mmap(Process& process, Memory::AddressSpace& address_space, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) { - return m_file->mmap(process, *this, range, offset, prot, shared); + return m_file->mmap(process, address_space, *this, range, offset, prot, shared); } ErrorOr<void> OpenFileDescription::truncate(u64 length) diff --git a/Kernel/FileSystem/OpenFileDescription.h b/Kernel/FileSystem/OpenFileDescription.h index 951ea3f21a..c280ecf71f 100644 --- a/Kernel/FileSystem/OpenFileDescription.h +++ b/Kernel/FileSystem/OpenFileDescription.h @@ -92,7 +92,7 @@ public: RefPtr<Custody> custody(); RefPtr<Custody const> custody() const; - ErrorOr<Memory::Region*> mmap(Process&, Memory::VirtualRange const&, u64 offset, int prot, bool shared); + ErrorOr<Memory::Region*> mmap(Process&, Memory::AddressSpace&, Memory::VirtualRange const&, u64 offset, int prot, bool shared); bool is_blocking() const; void set_blocking(bool b); diff --git a/Kernel/GlobalProcessExposed.cpp b/Kernel/GlobalProcessExposed.cpp index 356f1c7dc2..dcbd29fdf3 100644 --- a/Kernel/GlobalProcessExposed.cpp +++ b/Kernel/GlobalProcessExposed.cpp @@ -540,13 +540,33 @@ private: TRY(process_object.add("nfds"sv, process.fds().with_shared([](auto& fds) { return fds.open_count(); }))); TRY(process_object.add("name"sv, process.name())); TRY(process_object.add("executable"sv, process.executable() ? TRY(process.executable()->try_serialize_absolute_path())->view() : ""sv)); - TRY(process_object.add("amount_virtual"sv, process.address_space().amount_virtual())); - TRY(process_object.add("amount_resident"sv, process.address_space().amount_resident())); - TRY(process_object.add("amount_dirty_private"sv, process.address_space().amount_dirty_private())); - TRY(process_object.add("amount_clean_inode"sv, TRY(process.address_space().amount_clean_inode()))); - TRY(process_object.add("amount_shared"sv, process.address_space().amount_shared())); - TRY(process_object.add("amount_purgeable_volatile"sv, process.address_space().amount_purgeable_volatile())); - TRY(process_object.add("amount_purgeable_nonvolatile"sv, process.address_space().amount_purgeable_nonvolatile())); + + size_t amount_virtual = 0; + size_t amount_resident = 0; + size_t amount_dirty_private = 0; + size_t amount_clean_inode = 0; + size_t amount_shared = 0; + size_t amount_purgeable_volatile = 0; + size_t amount_purgeable_nonvolatile = 0; + + TRY(process.address_space().with([&](auto& space) -> ErrorOr<void> { + amount_virtual = space->amount_virtual(); + amount_resident = space->amount_resident(); + amount_dirty_private = space->amount_dirty_private(); + amount_clean_inode = TRY(space->amount_clean_inode()); + amount_shared = space->amount_shared(); + amount_purgeable_volatile = space->amount_purgeable_volatile(); + amount_purgeable_nonvolatile = space->amount_purgeable_nonvolatile(); + return {}; + })); + + TRY(process_object.add("amount_virtual"sv, amount_virtual)); + TRY(process_object.add("amount_resident"sv, amount_resident)); + TRY(process_object.add("amount_dirty_private"sv, amount_dirty_private)); + TRY(process_object.add("amount_clean_inode"sv, amount_clean_inode)); + TRY(process_object.add("amount_shared"sv, amount_shared)); + TRY(process_object.add("amount_purgeable_volatile"sv, amount_purgeable_volatile)); + TRY(process_object.add("amount_purgeable_nonvolatile"sv, amount_purgeable_nonvolatile)); TRY(process_object.add("dumpable"sv, process.is_dumpable())); TRY(process_object.add("kernel"sv, process.is_kernel_process())); auto thread_array = TRY(process_object.add_array("threads"sv)); diff --git a/Kernel/Graphics/DisplayConnector.cpp b/Kernel/Graphics/DisplayConnector.cpp index ffb47e792e..de02a4ed6f 100644 --- a/Kernel/Graphics/DisplayConnector.cpp +++ b/Kernel/Graphics/DisplayConnector.cpp @@ -32,13 +32,13 @@ DisplayConnector::DisplayConnector(size_t framebuffer_resource_size, bool enable { } -ErrorOr<Memory::Region*> DisplayConnector::mmap(Process& process, OpenFileDescription&, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) +ErrorOr<Memory::Region*> DisplayConnector::mmap(Process&, Memory::AddressSpace& address_space, OpenFileDescription&, Memory::VirtualRange const& range, u64 offset, int prot, bool shared) { VERIFY(m_shared_framebuffer_vmobject); if (offset != 0) return Error::from_errno(ENOTSUP); - return process.address_space().allocate_region_with_vmobject( + return address_space.allocate_region_with_vmobject( range, *m_shared_framebuffer_vmobject, 0, diff --git a/Kernel/Graphics/DisplayConnector.h b/Kernel/Graphics/DisplayConnector.h index 240684bd77..5dc0dc0cd8 100644 --- a/Kernel/Graphics/DisplayConnector.h +++ b/Kernel/Graphics/DisplayConnector.h @@ -137,7 +137,7 @@ private: virtual bool can_write(OpenFileDescription const&, u64) const final override { return true; } virtual ErrorOr<size_t> read(OpenFileDescription&, u64, UserOrKernelBuffer&, size_t) override final; virtual ErrorOr<size_t> write(OpenFileDescription&, u64, UserOrKernelBuffer const&, size_t) override final; - virtual ErrorOr<Memory::Region*> mmap(Process&, OpenFileDescription&, Memory::VirtualRange const&, u64, int, bool) override final; + virtual ErrorOr<Memory::Region*> mmap(Process&, Memory::AddressSpace&, OpenFileDescription&, Memory::VirtualRange const&, u64, int, bool) override final; virtual ErrorOr<void> ioctl(OpenFileDescription&, unsigned request, Userspace<void*> arg) override final; virtual StringView class_name() const override final { return "DisplayConnector"sv; } diff --git a/Kernel/Memory/AddressSpace.h b/Kernel/Memory/AddressSpace.h index 23f7c7a76b..e2df65e0b6 100644 --- a/Kernel/Memory/AddressSpace.h +++ b/Kernel/Memory/AddressSpace.h @@ -53,8 +53,6 @@ public: void remove_all_regions(Badge<Process>); - RecursiveSpinlock& get_lock() const { return m_lock; } - ErrorOr<size_t> amount_clean_inode() const; size_t amount_dirty_private() const; size_t amount_virtual() const; @@ -66,8 +64,6 @@ public: private: AddressSpace(NonnullLockRefPtr<PageDirectory>, VirtualRange total_range); - mutable RecursiveSpinlock m_lock { LockRank::None }; - LockRefPtr<PageDirectory> m_page_directory; // NOTE: The total range is also in the RegionTree, but since it never changes, diff --git a/Kernel/Memory/MemoryManager.cpp b/Kernel/Memory/MemoryManager.cpp index 01d73bc959..3ab26a5731 100644 --- a/Kernel/Memory/MemoryManager.cpp +++ b/Kernel/Memory/MemoryManager.cpp @@ -646,40 +646,32 @@ Region* MemoryManager::kernel_region_from_vaddr(VirtualAddress address) return MM.m_region_tree.with([&](auto& region_tree) { return region_tree.find_region_containing(address); }); } -Region* MemoryManager::find_user_region_from_vaddr_no_lock(AddressSpace& space, VirtualAddress vaddr) -{ - VERIFY(space.get_lock().is_locked_by_current_processor()); - return space.find_region_containing({ vaddr, 1 }); -} - Region* MemoryManager::find_user_region_from_vaddr(AddressSpace& space, VirtualAddress vaddr) { - SpinlockLocker lock(space.get_lock()); - return find_user_region_from_vaddr_no_lock(space, vaddr); + return space.find_region_containing({ vaddr, 1 }); } -void MemoryManager::validate_syscall_preconditions(AddressSpace& space, RegisterState const& regs) +void MemoryManager::validate_syscall_preconditions(Process& process, RegisterState const& regs) { - // We take the space lock once here and then use the no_lock variants - // to avoid excessive spinlock recursion in this extremely common path. - SpinlockLocker lock(space.get_lock()); - - auto unlock_and_handle_crash = [&lock, ®s](char const* description, int signal) { - lock.unlock(); - handle_crash(regs, description, signal); + bool should_crash = false; + char const* crash_description = nullptr; + int crash_signal = 0; + + auto unlock_and_handle_crash = [&](char const* description, int signal) { + should_crash = true; + crash_description = description; + crash_signal = signal; }; - { + process.address_space().with([&](auto& space) -> void { VirtualAddress userspace_sp = VirtualAddress { regs.userspace_sp() }; - if (!MM.validate_user_stack_no_lock(space, userspace_sp)) { + if (!MM.validate_user_stack(*space, userspace_sp)) { dbgln("Invalid stack pointer: {}", userspace_sp); return unlock_and_handle_crash("Bad stack on syscall entry", SIGSEGV); } - } - { VirtualAddress ip = VirtualAddress { regs.ip() }; - auto* calling_region = MM.find_user_region_from_vaddr_no_lock(space, ip); + auto* calling_region = MM.find_user_region_from_vaddr(*space, ip); if (!calling_region) { dbgln("Syscall from {:p} which has no associated region", ip); return unlock_and_handle_crash("Syscall from unknown region", SIGSEGV); @@ -690,10 +682,14 @@ void MemoryManager::validate_syscall_preconditions(AddressSpace& space, Register return unlock_and_handle_crash("Syscall from writable memory", SIGSEGV); } - if (space.enforces_syscall_regions() && !calling_region->is_syscall_region()) { + if (space->enforces_syscall_regions() && !calling_region->is_syscall_region()) { dbgln("Syscall from non-syscall region"); return unlock_and_handle_crash("Syscall from non-syscall region", SIGSEGV); } + }); + + if (should_crash) { + handle_crash(regs, crash_description, crash_signal); } } @@ -830,12 +826,20 @@ ErrorOr<CommittedPhysicalPageSet> MemoryManager::commit_physical_pages(size_t pa dbgln("MM: Unable to commit {} pages, have only {}", page_count, m_system_memory_info.physical_pages_uncommitted); Process::for_each([&](Process const& process) { + size_t amount_resident = 0; + size_t amount_shared = 0; + size_t amount_virtual = 0; + process.address_space().with([&](auto& space) { + amount_resident = space->amount_resident(); + amount_shared = space->amount_shared(); + amount_virtual = space->amount_virtual(); + }); dbgln("{}({}) resident:{}, shared:{}, virtual:{}", process.name(), process.pid(), - process.address_space().amount_resident() / PAGE_SIZE, - process.address_space().amount_shared() / PAGE_SIZE, - process.address_space().amount_virtual() / PAGE_SIZE); + amount_resident / PAGE_SIZE, + amount_shared / PAGE_SIZE, + amount_virtual / PAGE_SIZE); return IterationDecision::Continue; }); @@ -1007,7 +1011,9 @@ ErrorOr<NonnullLockRefPtrVector<PhysicalPage>> MemoryManager::allocate_contiguou void MemoryManager::enter_process_address_space(Process& process) { - enter_address_space(process.address_space()); + process.address_space().with([](auto& space) { + enter_address_space(*space); + }); } void MemoryManager::enter_address_space(AddressSpace& space) @@ -1100,23 +1106,15 @@ void MemoryManager::unquickmap_page() mm_data.m_quickmap_in_use.unlock(mm_data.m_quickmap_prev_flags); } -bool MemoryManager::validate_user_stack_no_lock(AddressSpace& space, VirtualAddress vaddr) const +bool MemoryManager::validate_user_stack(AddressSpace& space, VirtualAddress vaddr) const { - VERIFY(space.get_lock().is_locked_by_current_processor()); - if (!is_user_address(vaddr)) return false; - auto* region = find_user_region_from_vaddr_no_lock(space, vaddr); + auto* region = find_user_region_from_vaddr(space, vaddr); return region && region->is_user() && region->is_stack(); } -bool MemoryManager::validate_user_stack(AddressSpace& space, VirtualAddress vaddr) const -{ - SpinlockLocker lock(space.get_lock()); - return validate_user_stack_no_lock(space, vaddr); -} - void MemoryManager::unregister_kernel_region(Region& region) { VERIFY(region.is_kernel()); diff --git a/Kernel/Memory/MemoryManager.h b/Kernel/Memory/MemoryManager.h index 6bf827773c..4b62ed4e45 100644 --- a/Kernel/Memory/MemoryManager.h +++ b/Kernel/Memory/MemoryManager.h @@ -159,7 +159,6 @@ public: static void enter_process_address_space(Process&); static void enter_address_space(AddressSpace&); - bool validate_user_stack_no_lock(AddressSpace&, VirtualAddress) const; bool validate_user_stack(AddressSpace&, VirtualAddress) const; enum class ShouldZeroFill { @@ -222,8 +221,7 @@ public: } static Region* find_user_region_from_vaddr(AddressSpace&, VirtualAddress); - static Region* find_user_region_from_vaddr_no_lock(AddressSpace&, VirtualAddress); - static void validate_syscall_preconditions(AddressSpace&, RegisterState const&); + static void validate_syscall_preconditions(Process&, RegisterState const&); void dump_kernel_regions(); diff --git a/Kernel/Memory/RegionTree.h b/Kernel/Memory/RegionTree.h index aa5226d448..e028bb3130 100644 --- a/Kernel/Memory/RegionTree.h +++ b/Kernel/Memory/RegionTree.h @@ -45,9 +45,6 @@ public: void delete_all_regions_assuming_they_are_unmapped(); - // FIXME: Access the region tree through a SpinlockProtected or similar. - RecursiveSpinlock& get_lock() const { return m_lock; } - bool remove(Region&); Region* find_region_containing(VirtualAddress); @@ -58,9 +55,6 @@ private: ErrorOr<VirtualRange> allocate_range_specific(VirtualAddress base, size_t size); ErrorOr<VirtualRange> allocate_range_randomized(size_t size, size_t alignment = PAGE_SIZE); - // FIXME: We need a Region rank, but we don't know where to put it. - RecursiveSpinlock mutable m_lock { LockRank::None }; - IntrusiveRedBlackTree<&Region::m_tree_node> m_regions; VirtualRange const m_total_range; }; diff --git a/Kernel/PerformanceEventBuffer.cpp b/Kernel/PerformanceEventBuffer.cpp index 4852c4c8cf..191a50daab 100644 --- a/Kernel/PerformanceEventBuffer.cpp +++ b/Kernel/PerformanceEventBuffer.cpp @@ -334,8 +334,6 @@ OwnPtr<PerformanceEventBuffer> PerformanceEventBuffer::try_create_with_size(size ErrorOr<void> PerformanceEventBuffer::add_process(Process const& process, ProcessEventType event_type) { - SpinlockLocker locker(process.address_space().get_lock()); - OwnPtr<KString> executable; if (process.executable()) executable = TRY(process.executable()->try_serialize_absolute_path()); @@ -354,12 +352,14 @@ ErrorOr<void> PerformanceEventBuffer::add_process(Process const& process, Proces }); TRY(result); - return process.address_space().region_tree().with([&](auto& region_tree) -> ErrorOr<void> { - for (auto const& region : region_tree.regions()) { - TRY(append_with_ip_and_bp(process.pid(), 0, - 0, 0, PERF_EVENT_MMAP, 0, region.range().base().get(), region.range().size(), region.name())); - } - return {}; + return process.address_space().with([&](auto& space) { + return space->region_tree().with([&](auto& region_tree) -> ErrorOr<void> { + for (auto const& region : region_tree.regions()) { + TRY(append_with_ip_and_bp(process.pid(), 0, + 0, 0, PERF_EVENT_MMAP, 0, region.range().base().get(), region.range().size(), region.name())); + } + return {}; + }); }); } diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp index b4ae1bcbe8..19227ea138 100644 --- a/Kernel/Process.cpp +++ b/Kernel/Process.cpp @@ -218,16 +218,25 @@ void Process::unprotect_data() ErrorOr<NonnullLockRefPtr<Process>> Process::try_create(LockRefPtr<Thread>& first_thread, NonnullOwnPtr<KString> name, UserID uid, GroupID gid, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory, RefPtr<Custody> executable, TTY* tty, Process* fork_parent) { - auto space = TRY(Memory::AddressSpace::try_create(fork_parent ? &fork_parent->address_space() : nullptr)); + OwnPtr<Memory::AddressSpace> new_address_space; + if (fork_parent) { + TRY(fork_parent->address_space().with([&](auto& parent_address_space) -> ErrorOr<void> { + new_address_space = TRY(Memory::AddressSpace::try_create(parent_address_space.ptr())); + return {}; + })); + } else { + new_address_space = TRY(Memory::AddressSpace::try_create(nullptr)); + } auto unveil_tree = UnveilNode { TRY(KString::try_create("/"sv)), UnveilMetadata(TRY(KString::try_create("/"sv))) }; auto credentials = TRY(Credentials::create(uid, gid, uid, gid, uid, gid, {})); auto process = TRY(adopt_nonnull_lock_ref_or_enomem(new (nothrow) Process(move(name), move(credentials), ppid, is_kernel_process, move(current_directory), move(executable), tty, move(unveil_tree)))); - TRY(process->attach_resources(move(space), first_thread, fork_parent)); + TRY(process->attach_resources(new_address_space.release_nonnull(), first_thread, fork_parent)); return process; } Process::Process(NonnullOwnPtr<KString> name, NonnullRefPtr<Credentials> credentials, ProcessID ppid, bool is_kernel_process, RefPtr<Custody> current_directory, RefPtr<Custody> executable, TTY* tty, UnveilNode unveil_tree) : m_name(move(name)) + , m_space(LockRank::None) , m_protected_data_lock(LockRank::None) , m_is_kernel_process(is_kernel_process) , m_executable(LockRank::None, move(executable)) @@ -248,7 +257,9 @@ Process::Process(NonnullOwnPtr<KString> name, NonnullRefPtr<Credentials> credent ErrorOr<void> Process::attach_resources(NonnullOwnPtr<Memory::AddressSpace>&& preallocated_space, LockRefPtr<Thread>& first_thread, Process* fork_parent) { - m_space = move(preallocated_space); + m_space.with([&](auto& space) { + space = move(preallocated_space); + }); auto create_first_thread = [&] { if (fork_parent) { @@ -401,7 +412,7 @@ void Process::crash(int signal, FlatPtr ip, bool out_of_memory) protected_data.termination_signal = signal; }); set_should_generate_coredump(!out_of_memory); - address_space().dump_regions(); + address_space().with([](auto& space) { space->dump_regions(); }); VERIFY(is_user_process()); die(); // We can not return from here, as there is nowhere @@ -662,7 +673,7 @@ void Process::finalize() unblock_waiters(Thread::WaitBlocker::UnblockFlags::Terminated); - m_space->remove_all_regions({}); + m_space.with([](auto& space) { space->remove_all_regions({}); }); VERIFY(ref_count() > 0); // WaitBlockerSet::finalize will be in charge of dropping the last diff --git a/Kernel/Process.h b/Kernel/Process.h index 8186aa64f5..7c5024f178 100644 --- a/Kernel/Process.h +++ b/Kernel/Process.h @@ -558,8 +558,8 @@ public: PerformanceEventBuffer* perf_events() { return m_perf_event_buffer; } PerformanceEventBuffer const* perf_events() const { return m_perf_event_buffer; } - Memory::AddressSpace& address_space() { return *m_space; } - Memory::AddressSpace const& address_space() const { return *m_space; } + SpinlockProtected<OwnPtr<Memory::AddressSpace>>& address_space() { return m_space; } + SpinlockProtected<OwnPtr<Memory::AddressSpace>> const& address_space() const { return m_space; } VirtualAddress signal_trampoline() const { @@ -656,7 +656,7 @@ private: NonnullOwnPtr<KString> m_name; - OwnPtr<Memory::AddressSpace> m_space; + SpinlockProtected<OwnPtr<Memory::AddressSpace>> m_space; LockRefPtr<ProcessGroup> m_pg; diff --git a/Kernel/ProcessSpecificExposed.cpp b/Kernel/ProcessSpecificExposed.cpp index 1bda213e5c..2e4cea6c29 100644 --- a/Kernel/ProcessSpecificExposed.cpp +++ b/Kernel/ProcessSpecificExposed.cpp @@ -267,45 +267,47 @@ ErrorOr<void> Process::procfs_get_fds_stats(KBufferBuilder& builder) const ErrorOr<void> Process::procfs_get_virtual_memory_stats(KBufferBuilder& builder) const { auto array = TRY(JsonArraySerializer<>::try_create(builder)); - TRY(address_space().region_tree().with([&](auto& region_tree) -> ErrorOr<void> { - for (auto const& region : region_tree.regions()) { - auto current_process_credentials = Process::current().credentials(); - if (!region.is_user() && !current_process_credentials->is_superuser()) - continue; - auto region_object = TRY(array.add_object()); - TRY(region_object.add("readable"sv, region.is_readable())); - TRY(region_object.add("writable"sv, region.is_writable())); - TRY(region_object.add("executable"sv, region.is_executable())); - TRY(region_object.add("stack"sv, region.is_stack())); - TRY(region_object.add("shared"sv, region.is_shared())); - TRY(region_object.add("syscall"sv, region.is_syscall_region())); - TRY(region_object.add("purgeable"sv, region.vmobject().is_anonymous())); - if (region.vmobject().is_anonymous()) { - TRY(region_object.add("volatile"sv, static_cast<Memory::AnonymousVMObject const&>(region.vmobject()).is_volatile())); - } - TRY(region_object.add("cacheable"sv, region.is_cacheable())); - TRY(region_object.add("address"sv, region.vaddr().get())); - TRY(region_object.add("size"sv, region.size())); - TRY(region_object.add("amount_resident"sv, region.amount_resident())); - TRY(region_object.add("amount_dirty"sv, region.amount_dirty())); - TRY(region_object.add("cow_pages"sv, region.cow_pages())); - TRY(region_object.add("name"sv, region.name())); - TRY(region_object.add("vmobject"sv, region.vmobject().class_name())); - - StringBuilder pagemap_builder; - for (size_t i = 0; i < region.page_count(); ++i) { - auto page = region.physical_page(i); - if (!page) - pagemap_builder.append('N'); - else if (page->is_shared_zero_page() || page->is_lazy_committed_page()) - pagemap_builder.append('Z'); - else - pagemap_builder.append('P'); + TRY(address_space().with([&](auto& space) { + return space->region_tree().with([&](auto& region_tree) -> ErrorOr<void> { + for (auto const& region : region_tree.regions()) { + auto current_process_credentials = Process::current().credentials(); + if (!region.is_user() && !current_process_credentials->is_superuser()) + continue; + auto region_object = TRY(array.add_object()); + TRY(region_object.add("readable"sv, region.is_readable())); + TRY(region_object.add("writable"sv, region.is_writable())); + TRY(region_object.add("executable"sv, region.is_executable())); + TRY(region_object.add("stack"sv, region.is_stack())); + TRY(region_object.add("shared"sv, region.is_shared())); + TRY(region_object.add("syscall"sv, region.is_syscall_region())); + TRY(region_object.add("purgeable"sv, region.vmobject().is_anonymous())); + if (region.vmobject().is_anonymous()) { + TRY(region_object.add("volatile"sv, static_cast<Memory::AnonymousVMObject const&>(region.vmobject()).is_volatile())); + } + TRY(region_object.add("cacheable"sv, region.is_cacheable())); + TRY(region_object.add("address"sv, region.vaddr().get())); + TRY(region_object.add("size"sv, region.size())); + TRY(region_object.add("amount_resident"sv, region.amount_resident())); + TRY(region_object.add("amount_dirty"sv, region.amount_dirty())); + TRY(region_object.add("cow_pages"sv, region.cow_pages())); + TRY(region_object.add("name"sv, region.name())); + TRY(region_object.add("vmobject"sv, region.vmobject().class_name())); + + StringBuilder pagemap_builder; + for (size_t i = 0; i < region.page_count(); ++i) { + auto page = region.physical_page(i); + if (!page) + pagemap_builder.append('N'); + else if (page->is_shared_zero_page() || page->is_lazy_committed_page()) + pagemap_builder.append('Z'); + else + pagemap_builder.append('P'); + } + TRY(region_object.add("pagemap"sv, pagemap_builder.string_view())); + TRY(region_object.finish()); } - TRY(region_object.add("pagemap"sv, pagemap_builder.string_view())); - TRY(region_object.finish()); - } - return {}; + return {}; + }); })); TRY(array.finish()); return {}; diff --git a/Kernel/Syscall.cpp b/Kernel/Syscall.cpp index cd1d46c3c2..9fcd0700b8 100644 --- a/Kernel/Syscall.cpp +++ b/Kernel/Syscall.cpp @@ -204,7 +204,7 @@ NEVER_INLINE void syscall_handler(TrapFrame* trap) PANIC("Syscall from process with IOPL != 0"); } - Memory::MemoryManager::validate_syscall_preconditions(process.address_space(), regs); + Memory::MemoryManager::validate_syscall_preconditions(process, regs); FlatPtr function; FlatPtr arg1; diff --git a/Kernel/Syscalls/clock.cpp b/Kernel/Syscalls/clock.cpp index ed80b8964b..0f0f5d4d7e 100644 --- a/Kernel/Syscalls/clock.cpp +++ b/Kernel/Syscalls/clock.cpp @@ -17,8 +17,10 @@ ErrorOr<FlatPtr> Process::sys$map_time_page() auto& vmobject = TimeManagement::the().time_page_vmobject(); - auto* region = TRY(address_space().allocate_region_with_vmobject(Memory::RandomizeVirtualAddress::Yes, {}, PAGE_SIZE, PAGE_SIZE, vmobject, 0, "Kernel time page"sv, PROT_READ, true)); - return region->vaddr().get(); + return address_space().with([&](auto& space) -> ErrorOr<FlatPtr> { + auto* region = TRY(space->allocate_region_with_vmobject(Memory::RandomizeVirtualAddress::Yes, {}, PAGE_SIZE, PAGE_SIZE, vmobject, 0, "Kernel time page"sv, PROT_READ, true)); + return region->vaddr().get(); + }); } ErrorOr<FlatPtr> Process::sys$clock_gettime(clockid_t clock_id, Userspace<timespec*> user_ts) diff --git a/Kernel/Syscalls/execve.cpp b/Kernel/Syscalls/execve.cpp index fc281fc4a8..99a5b49c91 100644 --- a/Kernel/Syscalls/execve.cpp +++ b/Kernel/Syscalls/execve.cpp @@ -551,7 +551,7 @@ ErrorOr<void> Process::do_exec(NonnullLockRefPtr<OpenFileDescription> main_progr // This ensures that the process always has a valid page directory. Memory::MemoryManager::enter_address_space(*load_result.space); - m_space = load_result.space.release_nonnull(); + m_space.with([&](auto& space) { space = load_result.space.release_nonnull(); }); m_executable.with([&](auto& executable) { executable = main_program_description->custody(); }); m_arguments = move(arguments); @@ -661,7 +661,7 @@ ErrorOr<void> Process::do_exec(NonnullLockRefPtr<OpenFileDescription> main_progr regs.rip = load_result.entry_eip; regs.rsp = new_userspace_sp; #endif - regs.cr3 = address_space().page_directory().cr3(); + regs.cr3 = address_space().with([](auto& space) { return space->page_directory().cr3(); }); { TemporaryChange profiling_disabler(m_profiling, was_profiling); diff --git a/Kernel/Syscalls/fork.cpp b/Kernel/Syscalls/fork.cpp index 68a637d8f5..130a491294 100644 --- a/Kernel/Syscalls/fork.cpp +++ b/Kernel/Syscalls/fork.cpp @@ -65,7 +65,6 @@ ErrorOr<FlatPtr> Process::sys$fork(RegisterState& regs) }); dbgln_if(FORK_DEBUG, "fork: child={}", child); - child->address_space().set_enforces_syscall_regions(address_space().enforces_syscall_regions()); // A child created via fork(2) inherits a copy of its parent's signal mask child_first_thread->update_signal_mask(Thread::current()->signal_mask()); @@ -123,23 +122,26 @@ ErrorOr<FlatPtr> Process::sys$fork(RegisterState& regs) # error Unknown architecture #endif - { - TRY(address_space().region_tree().with([&](auto& parent_region_tree) -> ErrorOr<void> { - return child->address_space().region_tree().with([&](auto& child_region_tree) -> ErrorOr<void> { - for (auto& region : parent_region_tree.regions()) { - dbgln_if(FORK_DEBUG, "fork: cloning Region '{}' @ {}", region.name(), region.vaddr()); - auto region_clone = TRY(region.try_clone()); - TRY(region_clone->map(child->address_space().page_directory(), Memory::ShouldFlushTLB::No)); - TRY(child_region_tree.place_specifically(*region_clone, region.range())); - auto* child_region = region_clone.leak_ptr(); - - if (®ion == m_master_tls_region.unsafe_ptr()) - child->m_master_tls_region = TRY(child_region->try_make_weak_ptr()); - } - return {}; + TRY(address_space().with([&](auto& parent_space) { + return child->address_space().with([&](auto& child_space) { + child_space->set_enforces_syscall_regions(parent_space->enforces_syscall_regions()); + return parent_space->region_tree().with([&](auto& parent_region_tree) -> ErrorOr<void> { + return child_space->region_tree().with([&](auto& child_region_tree) -> ErrorOr<void> { + for (auto& region : parent_region_tree.regions()) { + dbgln_if(FORK_DEBUG, "fork: cloning Region '{}' @ {}", region.name(), region.vaddr()); + auto region_clone = TRY(region.try_clone()); + TRY(region_clone->map(child_space->page_directory(), Memory::ShouldFlushTLB::No)); + TRY(child_region_tree.place_specifically(*region_clone, region.range())); + auto* child_region = region_clone.leak_ptr(); + + if (®ion == m_master_tls_region.unsafe_ptr()) + child->m_master_tls_region = TRY(child_region->try_make_weak_ptr()); + } + return {}; + }); }); - })); - } + }); + })); thread_finalizer_guard.disarm(); diff --git a/Kernel/Syscalls/futex.cpp b/Kernel/Syscalls/futex.cpp index 8289385065..6b12bd584a 100644 --- a/Kernel/Syscalls/futex.cpp +++ b/Kernel/Syscalls/futex.cpp @@ -18,7 +18,7 @@ static Singleton<SpinlockProtected<HashMap<GlobalFutexKey, NonnullLockRefPtr<Fut void Process::clear_futex_queues_on_exec() { s_global_futex_queues->with([this](auto& queues) { - auto const* address_space = &this->address_space(); + auto const* address_space = this->address_space().with([](auto& space) { return space.ptr(); }); queues.remove_all_matching([address_space](auto& futex_key, auto& futex_queue) { if ((futex_key.raw.offset & futex_key_private_flag) == 0) return false; @@ -45,45 +45,47 @@ ErrorOr<GlobalFutexKey> Process::get_futex_key(FlatPtr user_address, bool shared if (!shared) { // If this is thread-shared, we can skip searching the matching region return GlobalFutexKey { .private_ = { - .address_space = &address_space(), + .address_space = this->address_space().with([](auto& space) { return space.ptr(); }), .user_address = user_address | futex_key_private_flag, } }; } - auto* matching_region = address_space().find_region_containing(range); - if (!matching_region) - return EFAULT; + return address_space().with([&](auto& space) -> ErrorOr<GlobalFutexKey> { + auto* matching_region = space->find_region_containing(range); + if (!matching_region) + return EFAULT; + + // The user wants to share this futex, but if the address doesn't point to a shared resource, there's not + // much sharing to be done, so let's mark this as private + if (!matching_region->is_shared()) { + return GlobalFutexKey { + .private_ = { + .address_space = space.ptr(), + .user_address = user_address | futex_key_private_flag, + } + }; + } + + // This address is backed by a shared VMObject, if it's an AnonymousVMObject, it can be shared between processes + // via forking, and shared regions that are cloned during a fork retain their original AnonymousVMObject. + // On the other hand, if it's a SharedInodeVMObject, it can be shared by two processes mapping the same file as + // MAP_SHARED, but since they are deduplicated based on the inode, in all cases the VMObject pointer should be + // a unique global identifier. + // NOTE: This assumes that a program will not unmap the only region keeping the vmobject alive while waiting on it, + // if it does, it will get stuck waiting forever until interrupted by a signal, but since that use case is defined as + // a programmer error, we are fine with it. + + auto const& vmobject = matching_region->vmobject(); + if (vmobject.is_inode()) + VERIFY(vmobject.is_shared_inode()); - // The user wants to share this futex, but if the address doesn't point to a shared resource, there's not - // much sharing to be done, so let's mark this as private - if (!matching_region->is_shared()) { return GlobalFutexKey { - .private_ = { - .address_space = &address_space(), - .user_address = user_address | futex_key_private_flag, - } + .shared = { + .vmobject = &vmobject, + .offset = matching_region->offset_in_vmobject_from_vaddr(range.base()) } }; - } - - // This address is backed by a shared VMObject, if it's an AnonymousVMObject, it can be shared between processes - // via forking, and shared regions that are cloned during a fork retain their original AnonymousVMObject. - // On the other hand, if it's a SharedInodeVMObject, it can be shared by two processes mapping the same file as - // MAP_SHARED, but since they are deduplicated based on the inode, in all cases the VMObject pointer should be - // a unique global identifier. - // NOTE: This assumes that a program will not unmap the only region keeping the vmobject alive while waiting on it, - // if it does, it will get stuck waiting forever until interrupted by a signal, but since that use case is defined as - // a programmer error, we are fine with it. - - auto const& vmobject = matching_region->vmobject(); - if (vmobject.is_inode()) - VERIFY(vmobject.is_shared_inode()); - - return GlobalFutexKey { - .shared = { - .vmobject = &vmobject, - .offset = matching_region->offset_in_vmobject_from_vaddr(range.base()) } - }; + }); } ErrorOr<FlatPtr> Process::sys$futex(Userspace<Syscall::SC_futex_params const*> user_params) diff --git a/Kernel/Syscalls/get_stack_bounds.cpp b/Kernel/Syscalls/get_stack_bounds.cpp index 1c77277b47..7fdbfdd0e8 100644 --- a/Kernel/Syscalls/get_stack_bounds.cpp +++ b/Kernel/Syscalls/get_stack_bounds.cpp @@ -14,16 +14,18 @@ ErrorOr<FlatPtr> Process::sys$get_stack_bounds(Userspace<FlatPtr*> user_stack_ba VERIFY_NO_PROCESS_BIG_LOCK(this); auto& regs = Thread::current()->get_register_dump_from_stack(); FlatPtr stack_pointer = regs.userspace_sp(); - auto* stack_region = address_space().find_region_containing(Memory::VirtualRange { VirtualAddress(stack_pointer), 1 }); + return address_space().with([&](auto& space) -> ErrorOr<FlatPtr> { + auto* stack_region = space->find_region_containing(Memory::VirtualRange { VirtualAddress(stack_pointer), 1 }); - // The syscall handler should have killed us if we had an invalid stack pointer. - VERIFY(stack_region); + // The syscall handler should have killed us if we had an invalid stack pointer. + VERIFY(stack_region); - FlatPtr stack_base = stack_region->range().base().get(); - size_t stack_size = stack_region->size(); - TRY(copy_to_user(user_stack_base, &stack_base)); - TRY(copy_to_user(user_stack_size, &stack_size)); - return 0; + FlatPtr stack_base = stack_region->range().base().get(); + size_t stack_size = stack_region->size(); + TRY(copy_to_user(user_stack_base, &stack_base)); + TRY(copy_to_user(user_stack_size, &stack_size)); + return 0; + }); } } diff --git a/Kernel/Syscalls/mmap.cpp b/Kernel/Syscalls/mmap.cpp index c5afee7b7b..0d5ebba9bd 100644 --- a/Kernel/Syscalls/mmap.cpp +++ b/Kernel/Syscalls/mmap.cpp @@ -192,32 +192,23 @@ ErrorOr<FlatPtr> Process::sys$mmap(Userspace<Syscall::SC_mmap_params const*> use Memory::Region* region = nullptr; - // If MAP_FIXED is specified, existing mappings that intersect the requested range are removed. - if (map_fixed) - TRY(address_space().unmap_mmap_range(VirtualAddress(addr), size)); - - Memory::VirtualRange requested_range { VirtualAddress { addr }, rounded_size }; - if (addr && !(map_fixed || map_fixed_noreplace)) { - // If there's an address but MAP_FIXED wasn't specified, the address is just a hint. - requested_range = { {}, rounded_size }; - } + LockRefPtr<OpenFileDescription> description; + LockRefPtr<Memory::AnonymousVMObject> vmobject; if (map_anonymous) { auto strategy = map_noreserve ? AllocationStrategy::None : AllocationStrategy::Reserve; - LockRefPtr<Memory::AnonymousVMObject> vmobject; + if (flags & MAP_PURGEABLE) { vmobject = TRY(Memory::AnonymousVMObject::try_create_purgeable_with_size(rounded_size, strategy)); } else { vmobject = TRY(Memory::AnonymousVMObject::try_create_with_size(rounded_size, strategy)); } - - region = TRY(address_space().allocate_region_with_vmobject(map_randomized ? Memory::RandomizeVirtualAddress::Yes : Memory::RandomizeVirtualAddress::No, requested_range.base(), requested_range.size(), alignment, vmobject.release_nonnull(), 0, {}, prot, map_shared)); } else { if (offset < 0) return EINVAL; if (static_cast<size_t>(offset) & ~PAGE_MASK) return EINVAL; - auto description = TRY(open_file_description(fd)); + description = TRY(open_file_description(fd)); if (description->is_directory()) return ENODEV; // Require read access even when read protection is not requested. @@ -229,24 +220,40 @@ ErrorOr<FlatPtr> Process::sys$mmap(Userspace<Syscall::SC_mmap_params const*> use } if (description->inode()) TRY(validate_inode_mmap_prot(prot, *description->inode(), map_shared)); - - region = TRY(description->mmap(*this, requested_range, static_cast<u64>(offset), prot, map_shared)); } - if (!region) - return ENOMEM; + return address_space().with([&](auto& space) -> ErrorOr<FlatPtr> { + // If MAP_FIXED is specified, existing mappings that intersect the requested range are removed. + if (map_fixed) + TRY(space->unmap_mmap_range(VirtualAddress(addr), size)); - region->set_mmap(true); - if (map_shared) - region->set_shared(true); - if (map_stack) - region->set_stack(true); - if (name) - region->set_name(move(name)); + Memory::VirtualRange requested_range { VirtualAddress { addr }, rounded_size }; + if (addr && !(map_fixed || map_fixed_noreplace)) { + // If there's an address but MAP_FIXED wasn't specified, the address is just a hint. + requested_range = { {}, rounded_size }; + } + + if (map_anonymous) { + region = TRY(space->allocate_region_with_vmobject(map_randomized ? Memory::RandomizeVirtualAddress::Yes : Memory::RandomizeVirtualAddress::No, requested_range.base(), requested_range.size(), alignment, vmobject.release_nonnull(), 0, {}, prot, map_shared)); + } else { + region = TRY(description->mmap(*this, *space, requested_range, static_cast<u64>(offset), prot, map_shared)); + } + + if (!region) + return ENOMEM; - PerformanceManager::add_mmap_perf_event(*this, *region); + region->set_mmap(true); + if (map_shared) + region->set_shared(true); + if (map_stack) + region->set_stack(true); + if (name) + region->set_name(move(name)); - return region->vaddr().get(); + PerformanceManager::add_mmap_perf_event(*this, *region); + + return region->vaddr().get(); + }); } ErrorOr<FlatPtr> Process::sys$mprotect(Userspace<void*> addr, size_t size, int prot) @@ -265,117 +272,119 @@ ErrorOr<FlatPtr> Process::sys$mprotect(Userspace<void*> addr, size_t size, int p if (!is_user_range(range_to_mprotect)) return EFAULT; - if (auto* whole_region = address_space().find_region_from_range(range_to_mprotect)) { - if (!whole_region->is_mmap()) - return EPERM; - TRY(validate_mmap_prot(prot, whole_region->is_stack(), whole_region->vmobject().is_anonymous(), whole_region)); - 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<Memory::InodeVMObject const&>(whole_region->vmobject()).inode(), 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); - - whole_region->remap(); - return 0; - } - - // Check if we can carve out the desired range from an existing region - if (auto* old_region = address_space().find_region_containing(range_to_mprotect)) { - if (!old_region->is_mmap()) - return EPERM; - TRY(validate_mmap_prot(prot, old_region->is_stack(), old_region->vmobject().is_anonymous(), old_region)); - if (old_region->access() == Memory::prot_to_region_access_flags(prot)) + return address_space().with([&](auto& space) -> ErrorOr<FlatPtr> { + if (auto* whole_region = space->find_region_from_range(range_to_mprotect)) { + if (!whole_region->is_mmap()) + return EPERM; + TRY(validate_mmap_prot(prot, whole_region->is_stack(), whole_region->vmobject().is_anonymous(), whole_region)); + 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<Memory::InodeVMObject const&>(whole_region->vmobject()).inode(), 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); + + whole_region->remap(); return 0; - if (old_region->vmobject().is_inode()) - TRY(validate_inode_mmap_prot(prot, static_cast<Memory::InodeVMObject const&>(old_region->vmobject()).inode(), 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. - auto region = address_space().take_region(*old_region); - region->unmap(); - - // This vector is the region(s) adjacent to our range. - // We need to allocate a new region for the range we wanted to change permission bits on. - auto adjacent_regions = TRY(address_space().try_split_region_around_range(*region, range_to_mprotect)); - - size_t new_range_offset_in_vmobject = region->offset_in_vmobject() + (range_to_mprotect.base().get() - region->range().base().get()); - auto* new_region = TRY(address_space().try_allocate_split_region(*region, range_to_mprotect, new_range_offset_in_vmobject)); - new_region->set_readable(prot & PROT_READ); - new_region->set_writable(prot & PROT_WRITE); - new_region->set_executable(prot & PROT_EXEC); - - // Map the new regions using our page directory (they were just allocated and don't have one). - for (auto* adjacent_region : adjacent_regions) { - TRY(adjacent_region->map(address_space().page_directory())); } - TRY(new_region->map(address_space().page_directory())); - return 0; - } - if (auto const& regions = TRY(address_space().find_regions_intersecting(range_to_mprotect)); regions.size()) { - size_t full_size_found = 0; - // Check that all intersecting regions are compatible. - for (auto const* region : regions) { - if (!region->is_mmap()) + // Check if we can carve out the desired range from an existing region + if (auto* old_region = space->find_region_containing(range_to_mprotect)) { + if (!old_region->is_mmap()) 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<Memory::InodeVMObject const&>(region->vmobject()).inode(), region->is_shared())); - full_size_found += region->range().intersect(range_to_mprotect).size(); - } - - if (full_size_found != range_to_mprotect.size()) - return ENOMEM; - - // Finally, iterate over each region, either updating its access flags if the range covers it wholly, - // or carving out a new subregion with the appropriate access flags set. - for (auto* old_region : regions) { + TRY(validate_mmap_prot(prot, old_region->is_stack(), old_region->vmobject().is_anonymous(), old_region)); if (old_region->access() == Memory::prot_to_region_access_flags(prot)) - continue; - - auto const intersection_to_mprotect = range_to_mprotect.intersect(old_region->range()); - // If the region is completely covered by range, simply update the access flags - if (intersection_to_mprotect == old_region->range()) { - old_region->set_readable(prot & PROT_READ); - old_region->set_writable(prot & PROT_WRITE); - old_region->set_executable(prot & PROT_EXEC); + return 0; + if (old_region->vmobject().is_inode()) + TRY(validate_inode_mmap_prot(prot, static_cast<Memory::InodeVMObject const&>(old_region->vmobject()).inode(), old_region->is_shared())); - old_region->remap(); - continue; - } // Remove the old region from our regions tree, since were going to add another region // with the exact same start address. - auto region = address_space().take_region(*old_region); + auto region = space->take_region(*old_region); region->unmap(); // This vector is the region(s) adjacent to our range. // We need to allocate a new region for the range we wanted to change permission bits on. - auto adjacent_regions = TRY(address_space().try_split_region_around_range(*old_region, intersection_to_mprotect)); - - // Since the range is not contained in a single region, it can only partially cover its starting and ending region, - // therefore carving out a chunk from the region will always produce a single extra region, and not two. - VERIFY(adjacent_regions.size() == 1); - - size_t new_range_offset_in_vmobject = old_region->offset_in_vmobject() + (intersection_to_mprotect.base().get() - old_region->range().base().get()); - auto* new_region = TRY(address_space().try_allocate_split_region(*region, intersection_to_mprotect, new_range_offset_in_vmobject)); + auto adjacent_regions = TRY(space->try_split_region_around_range(*region, range_to_mprotect)); + size_t new_range_offset_in_vmobject = region->offset_in_vmobject() + (range_to_mprotect.base().get() - region->range().base().get()); + auto* new_region = TRY(space->try_allocate_split_region(*region, range_to_mprotect, new_range_offset_in_vmobject)); new_region->set_readable(prot & PROT_READ); new_region->set_writable(prot & PROT_WRITE); new_region->set_executable(prot & PROT_EXEC); - // Map the new region using our page directory (they were just allocated and don't have one) if any. - if (adjacent_regions.size()) - TRY(adjacent_regions[0]->map(address_space().page_directory())); - - TRY(new_region->map(address_space().page_directory())); + // Map the new regions using our page directory (they were just allocated and don't have one). + for (auto* adjacent_region : adjacent_regions) { + TRY(adjacent_region->map(space->page_directory())); + } + TRY(new_region->map(space->page_directory())); + return 0; } - return 0; - } + if (auto const& regions = TRY(space->find_regions_intersecting(range_to_mprotect)); regions.size()) { + size_t full_size_found = 0; + // Check that all intersecting regions are compatible. + for (auto const* region : regions) { + if (!region->is_mmap()) + 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<Memory::InodeVMObject const&>(region->vmobject()).inode(), region->is_shared())); + full_size_found += region->range().intersect(range_to_mprotect).size(); + } - return EINVAL; + if (full_size_found != range_to_mprotect.size()) + return ENOMEM; + + // Finally, iterate over each region, either updating its access flags if the range covers it wholly, + // or carving out a new subregion with the appropriate access flags set. + for (auto* old_region : regions) { + if (old_region->access() == Memory::prot_to_region_access_flags(prot)) + continue; + + auto const intersection_to_mprotect = range_to_mprotect.intersect(old_region->range()); + // If the region is completely covered by range, simply update the access flags + if (intersection_to_mprotect == old_region->range()) { + old_region->set_readable(prot & PROT_READ); + old_region->set_writable(prot & PROT_WRITE); + old_region->set_executable(prot & PROT_EXEC); + + old_region->remap(); + continue; + } + // Remove the old region from our regions tree, since were going to add another region + // with the exact same start address. + auto region = space->take_region(*old_region); + region->unmap(); + + // This vector is the region(s) adjacent to our range. + // We need to allocate a new region for the range we wanted to change permission bits on. + auto adjacent_regions = TRY(space->try_split_region_around_range(*old_region, intersection_to_mprotect)); + + // Since the range is not contained in a single region, it can only partially cover its starting and ending region, + // therefore carving out a chunk from the region will always produce a single extra region, and not two. + VERIFY(adjacent_regions.size() == 1); + + size_t new_range_offset_in_vmobject = old_region->offset_in_vmobject() + (intersection_to_mprotect.base().get() - old_region->range().base().get()); + auto* new_region = TRY(space->try_allocate_split_region(*region, intersection_to_mprotect, new_range_offset_in_vmobject)); + + new_region->set_readable(prot & PROT_READ); + new_region->set_writable(prot & PROT_WRITE); + new_region->set_executable(prot & PROT_EXEC); + + // Map the new region using our page directory (they were just allocated and don't have one) if any. + if (adjacent_regions.size()) + TRY(adjacent_regions[0]->map(space->page_directory())); + + TRY(new_region->map(space->page_directory())); + } + + return 0; + } + + return EINVAL; + }); } ErrorOr<FlatPtr> Process::sys$madvise(Userspace<void*> address, size_t size, int advice) @@ -391,22 +400,24 @@ ErrorOr<FlatPtr> Process::sys$madvise(Userspace<void*> address, size_t size, int if (!is_user_range(range_to_madvise)) return EFAULT; - auto* region = address_space().find_region_from_range(range_to_madvise); - if (!region) - return EINVAL; - if (!region->is_mmap()) - return EPERM; - if (advice == MADV_SET_VOLATILE || advice == MADV_SET_NONVOLATILE) { - if (!region->vmobject().is_anonymous()) - return EINVAL; - auto& vmobject = static_cast<Memory::AnonymousVMObject&>(region->vmobject()); - if (!vmobject.is_purgeable()) + return address_space().with([&](auto& space) -> ErrorOr<FlatPtr> { + auto* region = space->find_region_from_range(range_to_madvise); + if (!region) return EINVAL; - bool was_purged = false; - TRY(vmobject.set_volatile(advice == MADV_SET_VOLATILE, was_purged)); - return was_purged ? 1 : 0; - } - return EINVAL; + if (!region->is_mmap()) + return EPERM; + if (advice == MADV_SET_VOLATILE || advice == MADV_SET_NONVOLATILE) { + if (!region->vmobject().is_anonymous()) + return EINVAL; + auto& vmobject = static_cast<Memory::AnonymousVMObject&>(region->vmobject()); + if (!vmobject.is_purgeable()) + return EINVAL; + bool was_purged = false; + TRY(vmobject.set_volatile(advice == MADV_SET_VOLATILE, was_purged)); + return was_purged ? 1 : 0; + } + return EINVAL; + }); } ErrorOr<FlatPtr> Process::sys$set_mmap_name(Userspace<Syscall::SC_set_mmap_name_params const*> user_params) @@ -421,23 +432,27 @@ ErrorOr<FlatPtr> Process::sys$set_mmap_name(Userspace<Syscall::SC_set_mmap_name_ auto name = TRY(try_copy_kstring_from_user(params.name)); auto range = TRY(Memory::expand_range_to_page_boundaries((FlatPtr)params.addr, params.size)); - auto* region = address_space().find_region_from_range(range); - if (!region) - return EINVAL; - if (!region->is_mmap()) - return EPERM; + return address_space().with([&](auto& space) -> ErrorOr<FlatPtr> { + auto* region = space->find_region_from_range(range); + if (!region) + return EINVAL; + if (!region->is_mmap()) + return EPERM; - region->set_name(move(name)); - PerformanceManager::add_mmap_perf_event(*this, *region); + region->set_name(move(name)); + PerformanceManager::add_mmap_perf_event(*this, *region); - return 0; + return 0; + }); } ErrorOr<FlatPtr> Process::sys$munmap(Userspace<void*> addr, size_t size) { VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this); TRY(require_promise(Pledge::stdio)); - TRY(address_space().unmap_mmap_range(addr.vaddr(), size)); + TRY(address_space().with([&](auto& space) { + return space->unmap_mmap_range(addr.vaddr(), size); + })); return 0; } @@ -449,32 +464,34 @@ ErrorOr<FlatPtr> Process::sys$mremap(Userspace<Syscall::SC_mremap_params const*> auto old_range = TRY(Memory::expand_range_to_page_boundaries((FlatPtr)params.old_address, params.old_size)); - auto* old_region = address_space().find_region_from_range(old_range); - if (!old_region) - return EINVAL; + return address_space().with([&](auto& space) -> ErrorOr<FlatPtr> { + auto* old_region = space->find_region_from_range(old_range); + if (!old_region) + return EINVAL; - if (!old_region->is_mmap()) - return EPERM; + if (!old_region->is_mmap()) + return EPERM; - if (old_region->vmobject().is_shared_inode() && params.flags & MAP_PRIVATE && !(params.flags & (MAP_ANONYMOUS | MAP_NORESERVE))) { - auto range = old_region->range(); - auto old_prot = region_access_flags_to_prot(old_region->access()); - auto old_offset = old_region->offset_in_vmobject(); - NonnullLockRefPtr inode = static_cast<Memory::SharedInodeVMObject&>(old_region->vmobject()).inode(); + if (old_region->vmobject().is_shared_inode() && params.flags & MAP_PRIVATE && !(params.flags & (MAP_ANONYMOUS | MAP_NORESERVE))) { + auto range = old_region->range(); + auto old_prot = region_access_flags_to_prot(old_region->access()); + auto old_offset = old_region->offset_in_vmobject(); + NonnullLockRefPtr inode = static_cast<Memory::SharedInodeVMObject&>(old_region->vmobject()).inode(); - auto new_vmobject = TRY(Memory::PrivateInodeVMObject::try_create_with_inode(inode)); - auto old_name = old_region->take_name(); + auto new_vmobject = TRY(Memory::PrivateInodeVMObject::try_create_with_inode(inode)); + auto old_name = old_region->take_name(); - old_region->unmap(); - address_space().deallocate_region(*old_region); + old_region->unmap(); + space->deallocate_region(*old_region); - auto* new_region = TRY(address_space().allocate_region_with_vmobject(range, move(new_vmobject), old_offset, old_name->view(), old_prot, false)); - new_region->set_mmap(true); - return new_region->vaddr().get(); - } + 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); + return new_region->vaddr().get(); + } - dbgln("sys$mremap: Unimplemented remap request (flags={})", params.flags); - return ENOTIMPL; + dbgln("sys$mremap: Unimplemented remap request (flags={})", params.flags); + return ENOTIMPL; + }); } ErrorOr<FlatPtr> Process::sys$allocate_tls(Userspace<char const*> initial_data, size_t size) @@ -504,56 +521,61 @@ ErrorOr<FlatPtr> Process::sys$allocate_tls(Userspace<char const*> initial_data, if (multiple_threads) return EINVAL; - auto* region = TRY(address_space().allocate_region(Memory::RandomizeVirtualAddress::Yes, {}, size, PAGE_SIZE, "Master TLS"sv, PROT_READ | PROT_WRITE)); + return address_space().with([&](auto& space) -> ErrorOr<FlatPtr> { + auto* region = TRY(space->allocate_region(Memory::RandomizeVirtualAddress::Yes, {}, size, PAGE_SIZE, "Master TLS"sv, PROT_READ | PROT_WRITE)); - m_master_tls_region = TRY(region->try_make_weak_ptr()); - m_master_tls_size = size; - m_master_tls_alignment = PAGE_SIZE; + m_master_tls_region = TRY(region->try_make_weak_ptr()); + m_master_tls_size = size; + m_master_tls_alignment = PAGE_SIZE; - { - Kernel::SmapDisabler disabler; - void* fault_at; - if (!Kernel::safe_memcpy((char*)m_master_tls_region.unsafe_ptr()->vaddr().as_ptr(), (char*)initial_data.ptr(), size, fault_at)) - return EFAULT; - } + { + Kernel::SmapDisabler disabler; + void* fault_at; + if (!Kernel::safe_memcpy((char*)m_master_tls_region.unsafe_ptr()->vaddr().as_ptr(), (char*)initial_data.ptr(), size, fault_at)) + return EFAULT; + } - TRY(main_thread->make_thread_specific_region({})); + TRY(main_thread->make_thread_specific_region({})); #if ARCH(I386) - auto& tls_descriptor = Processor::current().get_gdt_entry(GDT_SELECTOR_TLS); - tls_descriptor.set_base(main_thread->thread_specific_data()); - tls_descriptor.set_limit(main_thread->thread_specific_region_size()); + auto& tls_descriptor = Processor::current().get_gdt_entry(GDT_SELECTOR_TLS); + tls_descriptor.set_base(main_thread->thread_specific_data()); + tls_descriptor.set_limit(main_thread->thread_specific_region_size()); #else - MSR fs_base_msr(MSR_FS_BASE); - fs_base_msr.set(main_thread->thread_specific_data().get()); + MSR fs_base_msr(MSR_FS_BASE); + fs_base_msr.set(main_thread->thread_specific_data().get()); #endif - return m_master_tls_region.unsafe_ptr()->vaddr().get(); + return m_master_tls_region.unsafe_ptr()->vaddr().get(); + }); } ErrorOr<FlatPtr> Process::sys$msyscall(Userspace<void*> address) { VERIFY_PROCESS_BIG_LOCK_ACQUIRED(this); - if (address_space().enforces_syscall_regions()) - return EPERM; - if (!address) { - address_space().set_enforces_syscall_regions(true); - return 0; - } + return address_space().with([&](auto& space) -> ErrorOr<FlatPtr> { + if (space->enforces_syscall_regions()) + return EPERM; - if (!Memory::is_user_address(address.vaddr())) - return EFAULT; + if (!address) { + space->set_enforces_syscall_regions(true); + return 0; + } - auto* region = address_space().find_region_containing(Memory::VirtualRange { address.vaddr(), 1 }); - if (!region) - return EINVAL; + if (!Memory::is_user_address(address.vaddr())) + return EFAULT; - if (!region->is_mmap()) - return EINVAL; + auto* region = space->find_region_containing(Memory::VirtualRange { address.vaddr(), 1 }); + if (!region) + return EINVAL; - region->set_syscall_region(true); - return 0; + if (!region->is_mmap()) + return EINVAL; + + region->set_syscall_region(true); + return 0; + }); } ErrorOr<FlatPtr> Process::sys$msync(Userspace<void*> address, size_t size, int flags) @@ -572,37 +594,39 @@ ErrorOr<FlatPtr> Process::sys$msync(Userspace<void*> address, size_t size, int f // Note: This is not specified auto rounded_size = TRY(Memory::page_round_up(size)); - auto regions = TRY(address_space().find_regions_intersecting(Memory::VirtualRange { address.vaddr(), rounded_size })); - // All regions from address upto address+size shall be mapped - if (regions.is_empty()) - return ENOMEM; + return address_space().with([&](auto& space) -> ErrorOr<FlatPtr> { + auto regions = TRY(space->find_regions_intersecting(Memory::VirtualRange { address.vaddr(), rounded_size })); + // All regions from address upto address+size shall be mapped + if (regions.is_empty()) + return ENOMEM; - size_t total_intersection_size = 0; - Memory::VirtualRange range_to_sync { address.vaddr(), rounded_size }; - for (auto const* region : regions) { - // Region was not mapped - if (!region->is_mmap()) + size_t total_intersection_size = 0; + Memory::VirtualRange range_to_sync { address.vaddr(), rounded_size }; + for (auto const* region : regions) { + // Region was not mapped + if (!region->is_mmap()) + return ENOMEM; + total_intersection_size += region->range().intersect(range_to_sync).size(); + } + // Part of the indicated range was not mapped + if (total_intersection_size != size) return ENOMEM; - total_intersection_size += region->range().intersect(range_to_sync).size(); - } - // Part of the indicated range was not mapped - if (total_intersection_size != size) - return ENOMEM; - - for (auto* region : regions) { - auto& vmobject = region->vmobject(); - if (!vmobject.is_shared_inode()) - continue; - - off_t offset = region->offset_in_vmobject() + address.ptr() - region->range().base().get(); - - auto& inode_vmobject = static_cast<Memory::SharedInodeVMObject&>(vmobject); - // FIXME: If multiple regions belong to the same vmobject we might want to coalesce these writes - // FIXME: Handle MS_ASYNC - TRY(inode_vmobject.sync(offset / PAGE_SIZE, rounded_size / PAGE_SIZE)); - // FIXME: Handle MS_INVALIDATE - } - return 0; + + for (auto* region : regions) { + auto& vmobject = region->vmobject(); + if (!vmobject.is_shared_inode()) + continue; + + off_t offset = region->offset_in_vmobject() + address.ptr() - region->range().base().get(); + + auto& inode_vmobject = static_cast<Memory::SharedInodeVMObject&>(vmobject); + // FIXME: If multiple regions belong to the same vmobject we might want to coalesce these writes + // FIXME: Handle MS_ASYNC + TRY(inode_vmobject.sync(offset / PAGE_SIZE, rounded_size / PAGE_SIZE)); + // FIXME: Handle MS_INVALIDATE + } + return 0; + }); } } diff --git a/Kernel/Syscalls/ptrace.cpp b/Kernel/Syscalls/ptrace.cpp index 3915b39c51..68842ac9d0 100644 --- a/Kernel/Syscalls/ptrace.cpp +++ b/Kernel/Syscalls/ptrace.cpp @@ -197,31 +197,34 @@ ErrorOr<void> Process::peek_user_data(Span<u8> destination, Userspace<u8 const*> ErrorOr<void> Process::poke_user_data(Userspace<FlatPtr*> address, FlatPtr data) { Memory::VirtualRange range = { address.vaddr(), sizeof(FlatPtr) }; - auto* region = address_space().find_region_containing(range); - if (!region) - return EFAULT; - ScopedAddressSpaceSwitcher switcher(*this); - if (region->is_shared()) { - // 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()); - auto vmobject = TRY(Memory::PrivateInodeVMObject::try_create_with_inode(static_cast<Memory::SharedInodeVMObject&>(region->vmobject()).inode())); - region->set_vmobject(move(vmobject)); - region->set_shared(false); - } - bool const was_writable = region->is_writable(); - if (!was_writable) { - region->set_writable(true); - region->remap(); - } - ScopeGuard rollback([&]() { + + return address_space().with([&](auto& space) -> ErrorOr<void> { + auto* region = space->find_region_containing(range); + if (!region) + return EFAULT; + ScopedAddressSpaceSwitcher switcher(*this); + if (region->is_shared()) { + // 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()); + auto vmobject = TRY(Memory::PrivateInodeVMObject::try_create_with_inode(static_cast<Memory::SharedInodeVMObject&>(region->vmobject()).inode())); + region->set_vmobject(move(vmobject)); + region->set_shared(false); + } + bool const was_writable = region->is_writable(); if (!was_writable) { - region->set_writable(false); + region->set_writable(true); region->remap(); } + ScopeGuard rollback([&]() { + if (!was_writable) { + region->set_writable(false); + region->remap(); + } + }); + + return copy_to_user(address, &data); }); - - return copy_to_user(address, &data); } ErrorOr<FlatPtr> Thread::peek_debug_register(u32 register_index) diff --git a/Kernel/Syscalls/sigaction.cpp b/Kernel/Syscalls/sigaction.cpp index 7c4b9488bf..6417901c1a 100644 --- a/Kernel/Syscalls/sigaction.cpp +++ b/Kernel/Syscalls/sigaction.cpp @@ -134,100 +134,40 @@ ErrorOr<void> Process::remap_range_as_stack(FlatPtr address, size_t size) if (!is_user_range(range_to_remap)) return EFAULT; - if (auto* whole_region = address_space().find_region_from_range(range_to_remap)) { - if (!whole_region->is_mmap()) - return EPERM; - if (!whole_region->vmobject().is_anonymous() || whole_region->is_shared()) - return EINVAL; - whole_region->unsafe_clear_access(); - whole_region->set_readable(true); - whole_region->set_writable(true); - whole_region->set_stack(true); - whole_region->set_syscall_region(false); - whole_region->clear_to_zero(); - whole_region->remap(); - - return {}; - } - - if (auto* old_region = address_space().find_region_containing(range_to_remap)) { - if (!old_region->is_mmap()) - return EPERM; - if (!old_region->vmobject().is_anonymous() || old_region->is_shared()) - return EINVAL; - - // Remove the old region from our regions tree, since were going to add another region - // with the exact same start address. - auto region = address_space().take_region(*old_region); - region->unmap(); - - // This vector is the region(s) adjacent to our range. - // We need to allocate a new region for the range we wanted to change permission bits on. - auto adjacent_regions = TRY(address_space().try_split_region_around_range(*region, range_to_remap)); - - size_t new_range_offset_in_vmobject = region->offset_in_vmobject() + (range_to_remap.base().get() - region->range().base().get()); - auto* new_region = TRY(address_space().try_allocate_split_region(*region, range_to_remap, new_range_offset_in_vmobject)); - new_region->unsafe_clear_access(); - new_region->set_readable(true); - new_region->set_writable(true); - new_region->set_stack(true); - new_region->set_syscall_region(false); - new_region->clear_to_zero(); - - // Map the new regions using our page directory (they were just allocated and don't have one). - for (auto* adjacent_region : adjacent_regions) { - TRY(adjacent_region->map(address_space().page_directory())); + return address_space().with([&](auto& space) -> ErrorOr<void> { + if (auto* whole_region = space->find_region_from_range(range_to_remap)) { + if (!whole_region->is_mmap()) + return EPERM; + if (!whole_region->vmobject().is_anonymous() || whole_region->is_shared()) + return EINVAL; + whole_region->unsafe_clear_access(); + whole_region->set_readable(true); + whole_region->set_writable(true); + whole_region->set_stack(true); + whole_region->set_syscall_region(false); + whole_region->clear_to_zero(); + whole_region->remap(); + + return {}; } - TRY(new_region->map(address_space().page_directory())); - - return {}; - } - if (auto const& regions = TRY(address_space().find_regions_intersecting(range_to_remap)); regions.size()) { - size_t full_size_found = 0; - // Check that all intersecting regions are compatible. - for (auto const* region : regions) { - if (!region->is_mmap()) + if (auto* old_region = space->find_region_containing(range_to_remap)) { + if (!old_region->is_mmap()) return EPERM; - if (!region->vmobject().is_anonymous() || region->is_shared()) + if (!old_region->vmobject().is_anonymous() || old_region->is_shared()) return EINVAL; - full_size_found += region->range().intersect(range_to_remap).size(); - } - if (full_size_found != range_to_remap.size()) - return ENOMEM; - - // Finally, iterate over each region, either updating its access flags if the range covers it wholly, - // or carving out a new subregion with the appropriate access flags set. - for (auto* old_region : regions) { - auto const intersection_to_remap = range_to_remap.intersect(old_region->range()); - // If the region is completely covered by range, simply update the access flags - if (intersection_to_remap == old_region->range()) { - old_region->unsafe_clear_access(); - old_region->set_readable(true); - old_region->set_writable(true); - old_region->set_stack(true); - old_region->set_syscall_region(false); - old_region->clear_to_zero(); - old_region->remap(); - continue; - } // Remove the old region from our regions tree, since were going to add another region // with the exact same start address. - auto region = address_space().take_region(*old_region); + auto region = space->take_region(*old_region); region->unmap(); // This vector is the region(s) adjacent to our range. // We need to allocate a new region for the range we wanted to change permission bits on. - auto adjacent_regions = TRY(address_space().try_split_region_around_range(*old_region, intersection_to_remap)); - - // Since the range is not contained in a single region, it can only partially cover its starting and ending region, - // therefore carving out a chunk from the region will always produce a single extra region, and not two. - VERIFY(adjacent_regions.size() == 1); - - size_t new_range_offset_in_vmobject = old_region->offset_in_vmobject() + (intersection_to_remap.base().get() - old_region->range().base().get()); - auto* new_region = TRY(address_space().try_allocate_split_region(*region, intersection_to_remap, new_range_offset_in_vmobject)); + auto adjacent_regions = TRY(space->try_split_region_around_range(*region, range_to_remap)); + size_t new_range_offset_in_vmobject = region->offset_in_vmobject() + (range_to_remap.base().get() - region->range().base().get()); + auto* new_region = TRY(space->try_allocate_split_region(*region, range_to_remap, new_range_offset_in_vmobject)); new_region->unsafe_clear_access(); new_region->set_readable(true); new_region->set_writable(true); @@ -235,16 +175,78 @@ ErrorOr<void> Process::remap_range_as_stack(FlatPtr address, size_t size) new_region->set_syscall_region(false); new_region->clear_to_zero(); - // Map the new region using our page directory (they were just allocated and don't have one) if any. - TRY(adjacent_regions[0]->map(address_space().page_directory())); + // Map the new regions using our page directory (they were just allocated and don't have one). + for (auto* adjacent_region : adjacent_regions) { + TRY(adjacent_region->map(space->page_directory())); + } + TRY(new_region->map(space->page_directory())); - TRY(new_region->map(address_space().page_directory())); + return {}; } - return {}; - } + if (auto const& regions = TRY(space->find_regions_intersecting(range_to_remap)); regions.size()) { + size_t full_size_found = 0; + // Check that all intersecting regions are compatible. + for (auto const* region : regions) { + if (!region->is_mmap()) + return EPERM; + if (!region->vmobject().is_anonymous() || region->is_shared()) + return EINVAL; + full_size_found += region->range().intersect(range_to_remap).size(); + } + + if (full_size_found != range_to_remap.size()) + return ENOMEM; - return EINVAL; + // Finally, iterate over each region, either updating its access flags if the range covers it wholly, + // or carving out a new subregion with the appropriate access flags set. + for (auto* old_region : regions) { + auto const intersection_to_remap = range_to_remap.intersect(old_region->range()); + // If the region is completely covered by range, simply update the access flags + if (intersection_to_remap == old_region->range()) { + old_region->unsafe_clear_access(); + old_region->set_readable(true); + old_region->set_writable(true); + old_region->set_stack(true); + old_region->set_syscall_region(false); + old_region->clear_to_zero(); + old_region->remap(); + continue; + } + // Remove the old region from our regions tree, since were going to add another region + // with the exact same start address. + auto region = space->take_region(*old_region); + region->unmap(); + + // This vector is the region(s) adjacent to our range. + // We need to allocate a new region for the range we wanted to change permission bits on. + auto adjacent_regions = TRY(space->try_split_region_around_range(*old_region, intersection_to_remap)); + + // Since the range is not contained in a single region, it can only partially cover its starting and ending region, + // therefore carving out a chunk from the region will always produce a single extra region, and not two. + VERIFY(adjacent_regions.size() == 1); + + size_t new_range_offset_in_vmobject = old_region->offset_in_vmobject() + (intersection_to_remap.base().get() - old_region->range().base().get()); + auto* new_region = TRY(space->try_allocate_split_region(*region, intersection_to_remap, new_range_offset_in_vmobject)); + + new_region->unsafe_clear_access(); + new_region->set_readable(true); + new_region->set_writable(true); + new_region->set_stack(true); + new_region->set_syscall_region(false); + new_region->clear_to_zero(); + + // Map the new region using our page directory (they were just allocated and don't have one) if any. + TRY(adjacent_regions[0]->map(space->page_directory())); + + TRY(new_region->map(space->page_directory())); + } + + return {}; + } + + return EINVAL; + }); } ErrorOr<FlatPtr> Process::sys$sigaltstack(Userspace<stack_t const*> user_ss, Userspace<stack_t*> user_old_ss) diff --git a/Kernel/Syscalls/thread.cpp b/Kernel/Syscalls/thread.cpp index 8f6c6b0d81..27f06924da 100644 --- a/Kernel/Syscalls/thread.cpp +++ b/Kernel/Syscalls/thread.cpp @@ -27,8 +27,11 @@ ErrorOr<FlatPtr> Process::sys$create_thread(void* (*entry)(void*), Userspace<Sys if (user_sp.has_overflow()) return EOVERFLOW; - if (!MM.validate_user_stack(this->address_space(), VirtualAddress(user_sp.value() - 4))) - return EFAULT; + TRY(address_space().with([&](auto& space) -> ErrorOr<void> { + if (!MM.validate_user_stack(*space, VirtualAddress(user_sp.value() - 4))) + return EFAULT; + return {}; + })); // FIXME: return EAGAIN if Thread::all_threads().size() is greater than PTHREAD_THREADS_MAX @@ -60,7 +63,7 @@ ErrorOr<FlatPtr> Process::sys$create_thread(void* (*entry)(void*), Userspace<Sys regs.rdx = params.rdx; regs.rcx = params.rcx; #endif - regs.cr3 = address_space().page_directory().cr3(); + regs.cr3 = address_space().with([](auto& space) { return space->page_directory().cr3(); }); TRY(thread->make_thread_specific_region({})); @@ -92,7 +95,9 @@ void Process::sys$exit_thread(Userspace<void*> exit_value, Userspace<void*> stac PerformanceManager::add_thread_exit_event(*current_thread); if (stack_location) { - auto unmap_result = address_space().unmap_mmap_range(stack_location.vaddr(), stack_size); + auto unmap_result = address_space().with([&](auto& space) { + return space->unmap_mmap_range(stack_location.vaddr(), stack_size); + }); if (unmap_result.is_error()) dbgln("Failed to unmap thread stack, terminating thread anyway. Error code: {}", unmap_result.error()); } diff --git a/Kernel/Thread.cpp b/Kernel/Thread.cpp index fc4ae0b549..ba6281755d 100644 --- a/Kernel/Thread.cpp +++ b/Kernel/Thread.cpp @@ -105,7 +105,7 @@ Thread::Thread(NonnullLockRefPtr<Process> process, NonnullOwnPtr<Memory::Region> # error Unknown architecture #endif - m_regs.cr3 = m_process->address_space().page_directory().cr3(); + m_regs.cr3 = m_process->address_space().with([](auto& space) { return space->page_directory().cr3(); }); m_kernel_stack_base = m_kernel_stack_region->vaddr().get(); m_kernel_stack_top = m_kernel_stack_region->vaddr().offset(default_kernel_stack_size).get() & ~(FlatPtr)0x7u; @@ -471,8 +471,10 @@ void Thread::exit(void* exit_value) u32 unlock_count; [[maybe_unused]] auto rc = unlock_process_if_locked(unlock_count); if (m_thread_specific_range.has_value()) { - auto* region = process().address_space().find_region_from_range(m_thread_specific_range.value()); - process().address_space().deallocate_region(*region); + process().address_space().with([&](auto& space) { + auto* region = space->find_region_from_range(m_thread_specific_range.value()); + space->deallocate_region(*region); + }); } #ifdef ENABLE_KERNEL_COVERAGE_COLLECTION KCOVDevice::free_thread(); @@ -1352,15 +1354,18 @@ static ErrorOr<bool> symbolicate(RecognizedSymbol const& symbol, Process& proces if (!Memory::is_user_address(VirtualAddress(symbol.address))) { TRY(builder.try_append("0xdeadc0de\n"sv)); } else { - if (auto* region = process.address_space().find_region_containing({ VirtualAddress(symbol.address), sizeof(FlatPtr) })) { - size_t offset = symbol.address - region->vaddr().get(); - if (auto region_name = region->name(); !region_name.is_null() && !region_name.is_empty()) - TRY(builder.try_appendff("{:p} {} + {:#x}\n", (void*)symbol.address, region_name, offset)); - else - TRY(builder.try_appendff("{:p} {:p} + {:#x}\n", (void*)symbol.address, region->vaddr().as_ptr(), offset)); - } else { - TRY(builder.try_appendff("{:p}\n", symbol.address)); - } + TRY(process.address_space().with([&](auto& space) -> ErrorOr<void> { + if (auto* region = space->find_region_containing({ VirtualAddress(symbol.address), sizeof(FlatPtr) })) { + size_t offset = symbol.address - region->vaddr().get(); + if (auto region_name = region->name(); !region_name.is_null() && !region_name.is_empty()) + TRY(builder.try_appendff("{:p} {} + {:#x}\n", (void*)symbol.address, region_name, offset)); + else + TRY(builder.try_appendff("{:p} {:p} + {:#x}\n", (void*)symbol.address, region->vaddr().as_ptr(), offset)); + } else { + TRY(builder.try_appendff("{:p}\n", symbol.address)); + } + return {}; + })); } return true; } @@ -1412,20 +1417,22 @@ ErrorOr<void> Thread::make_thread_specific_region(Badge<Process>) if (!process().m_master_tls_region) return {}; - auto* region = TRY(process().address_space().allocate_region(Memory::RandomizeVirtualAddress::Yes, {}, thread_specific_region_size(), PAGE_SIZE, "Thread-specific"sv, PROT_READ | PROT_WRITE)); + return process().address_space().with([&](auto& space) -> ErrorOr<void> { + auto* region = TRY(space->allocate_region(Memory::RandomizeVirtualAddress::Yes, {}, thread_specific_region_size(), PAGE_SIZE, "Thread-specific"sv, PROT_READ | PROT_WRITE)); - m_thread_specific_range = region->range(); + m_thread_specific_range = region->range(); - 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_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; + 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_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; - if (process().m_master_tls_size != 0) - memcpy(thread_local_storage, process().m_master_tls_region.unsafe_ptr()->vaddr().as_ptr(), process().m_master_tls_size); + if (process().m_master_tls_size != 0) + memcpy(thread_local_storage, process().m_master_tls_region.unsafe_ptr()->vaddr().as_ptr(), process().m_master_tls_size); - return {}; + return {}; + }); } LockRefPtr<Thread> Thread::from_tid(ThreadID tid) |