From f2fed21c1139c8d5c030bc5caee5c612dfe7979f Mon Sep 17 00:00:00 2001 From: Liam Date: Sun, 7 Jan 2024 13:59:48 -0500 Subject: kernel: fix page leak on process termination --- src/core/hle/kernel/k_page_table_base.cpp | 75 ++++++++++++++++++++++++++++++- src/core/hle/kernel/k_page_table_base.h | 1 + src/core/hle/kernel/k_process.cpp | 6 +++ 3 files changed, 81 insertions(+), 1 deletion(-) (limited to 'src/core/hle/kernel') diff --git a/src/core/hle/kernel/k_page_table_base.cpp b/src/core/hle/kernel/k_page_table_base.cpp index 73fbda331..f01eaa164 100644 --- a/src/core/hle/kernel/k_page_table_base.cpp +++ b/src/core/hle/kernel/k_page_table_base.cpp @@ -431,9 +431,82 @@ Result KPageTableBase::InitializeForProcess(Svc::CreateProcessFlag as_type, bool m_memory_block_slab_manager)); } +Result KPageTableBase::FinalizeProcess() { + // Only process tables should be finalized. + ASSERT(!this->IsKernel()); + + // HLE processes don't have memory mapped. + R_SUCCEED_IF(m_impl == nullptr); + + // NOTE: Here Nintendo calls an unknown OnFinalize function. + // this->OnFinalize(); + + // NOTE: Here Nintendo calls a second unknown OnFinalize function. + // this->OnFinalize2(); + + // Get implementation objects. + auto& impl = this->GetImpl(); + auto& mm = m_kernel.MemoryManager(); + + // Traverse, freeing all pages. + { + // Get the address space size. + const size_t as_size = this->GetAddressSpaceSize(); + + // Begin the traversal. + TraversalContext context; + TraversalEntry cur_entry = { + .phys_addr = 0, + .block_size = 0, + }; + + bool cur_valid = false; + TraversalEntry next_entry; + bool next_valid; + size_t tot_size = 0; + + next_valid = impl.BeginTraversal(std::addressof(next_entry), std::addressof(context), + this->GetAddressSpaceStart()); + + // Iterate over entries. + while (true) { + if ((!next_valid && !cur_valid) || + (next_valid && cur_valid && + next_entry.phys_addr == cur_entry.phys_addr + cur_entry.block_size)) { + cur_entry.block_size += next_entry.block_size; + } else { + if (cur_valid && IsHeapPhysicalAddressForFinalize(cur_entry.phys_addr)) { + mm.Close(cur_entry.phys_addr, cur_entry.block_size / PageSize); + } + + // Update tracking variables. + tot_size += cur_entry.block_size; + cur_entry = next_entry; + cur_valid = next_valid; + } + + if (cur_entry.block_size + tot_size >= as_size) { + break; + } + + next_valid = + impl.ContinueTraversal(std::addressof(next_entry), std::addressof(context)); + } + + // Handle the last block. + if (cur_valid && IsHeapPhysicalAddressForFinalize(cur_entry.phys_addr)) { + mm.Close(cur_entry.phys_addr, cur_entry.block_size / PageSize); + } + } + + R_SUCCEED(); +} + void KPageTableBase::Finalize() { + this->FinalizeProcess(); + auto HostUnmapCallback = [&](KProcessAddress addr, u64 size) { - if (Settings::IsFastmemEnabled()) { + if (m_impl->fastmem_arena) { m_system.DeviceMemory().buffer.Unmap(GetInteger(addr), size, false); } }; diff --git a/src/core/hle/kernel/k_page_table_base.h b/src/core/hle/kernel/k_page_table_base.h index 077cafc96..748419f86 100644 --- a/src/core/hle/kernel/k_page_table_base.h +++ b/src/core/hle/kernel/k_page_table_base.h @@ -241,6 +241,7 @@ public: KResourceLimit* resource_limit, Core::Memory::Memory& memory, KProcessAddress aslr_space_start); + Result FinalizeProcess(); void Finalize(); bool IsKernel() const { diff --git a/src/core/hle/kernel/k_process.cpp b/src/core/hle/kernel/k_process.cpp index 068e71dff..ae332a550 100644 --- a/src/core/hle/kernel/k_process.cpp +++ b/src/core/hle/kernel/k_process.cpp @@ -171,6 +171,12 @@ void KProcess::Finalize() { m_resource_limit->Close(); } + // Clear expensive resources, as the destructor is not called for guest objects. + for (auto& interface : m_arm_interfaces) { + interface.reset(); + } + m_exclusive_monitor.reset(); + // Perform inherited finalization. KSynchronizationObject::Finalize(); } -- cgit v1.2.3 From f90a022d3a20c86399f49a8154847b73bc1b8fd3 Mon Sep 17 00:00:00 2001 From: Liam Date: Tue, 2 Jan 2024 17:12:16 -0500 Subject: kernel: fix debugger and process list lifetime --- src/core/hle/kernel/kernel.cpp | 31 ++++++++++++++++++++++++++++--- src/core/hle/kernel/kernel.h | 6 ++++-- src/core/hle/kernel/svc/svc_process.cpp | 8 +++++--- 3 files changed, 37 insertions(+), 8 deletions(-) (limited to 'src/core/hle/kernel') diff --git a/src/core/hle/kernel/kernel.cpp b/src/core/hle/kernel/kernel.cpp index 1030f0c12..f3683cdcc 100644 --- a/src/core/hle/kernel/kernel.cpp +++ b/src/core/hle/kernel/kernel.cpp @@ -112,7 +112,14 @@ struct KernelCore::Impl { old_process->Close(); } - process_list.clear(); + { + std::scoped_lock lk{process_list_lock}; + for (auto* const process : process_list) { + process->Terminate(); + process->Close(); + } + process_list.clear(); + } next_object_id = 0; next_kernel_process_id = KProcess::InitialProcessIdMin; @@ -770,6 +777,7 @@ struct KernelCore::Impl { std::atomic next_thread_id{1}; // Lists all processes that exist in the current session. + std::mutex process_list_lock; std::vector process_list; std::atomic application_process{}; std::unique_ptr global_scheduler_context; @@ -869,9 +877,19 @@ KResourceLimit* KernelCore::GetSystemResourceLimit() { } void KernelCore::AppendNewProcess(KProcess* process) { + process->Open(); + + std::scoped_lock lk{impl->process_list_lock}; impl->process_list.push_back(process); } +void KernelCore::RemoveProcess(KProcess* process) { + std::scoped_lock lk{impl->process_list_lock}; + if (std::erase(impl->process_list, process)) { + process->Close(); + } +} + void KernelCore::MakeApplicationProcess(KProcess* process) { impl->MakeApplicationProcess(process); } @@ -884,8 +902,15 @@ const KProcess* KernelCore::ApplicationProcess() const { return impl->application_process; } -const std::vector& KernelCore::GetProcessList() const { - return impl->process_list; +std::list> KernelCore::GetProcessList() { + std::list> processes; + std::scoped_lock lk{impl->process_list_lock}; + + for (auto* const process : impl->process_list) { + processes.emplace_back(process); + } + + return processes; } Kernel::GlobalSchedulerContext& KernelCore::GlobalSchedulerContext() { diff --git a/src/core/hle/kernel/kernel.h b/src/core/hle/kernel/kernel.h index 5d4102145..8ea5bed1c 100644 --- a/src/core/hle/kernel/kernel.h +++ b/src/core/hle/kernel/kernel.h @@ -5,6 +5,7 @@ #include #include +#include #include #include #include @@ -116,8 +117,9 @@ public: /// Retrieves a shared pointer to the system resource limit instance. KResourceLimit* GetSystemResourceLimit(); - /// Adds the given shared pointer to an internal list of active processes. + /// Adds/removes the given pointer to an internal list of active processes. void AppendNewProcess(KProcess* process); + void RemoveProcess(KProcess* process); /// Makes the given process the new application process. void MakeApplicationProcess(KProcess* process); @@ -129,7 +131,7 @@ public: const KProcess* ApplicationProcess() const; /// Retrieves the list of processes. - const std::vector& GetProcessList() const; + std::list> GetProcessList(); /// Gets the sole instance of the global scheduler Kernel::GlobalSchedulerContext& GlobalSchedulerContext(); diff --git a/src/core/hle/kernel/svc/svc_process.cpp b/src/core/hle/kernel/svc/svc_process.cpp index caa8bee9a..5c3e8829f 100644 --- a/src/core/hle/kernel/svc/svc_process.cpp +++ b/src/core/hle/kernel/svc/svc_process.cpp @@ -74,13 +74,15 @@ Result GetProcessList(Core::System& system, s32* out_num_processes, u64 out_proc } auto& memory = GetCurrentMemory(kernel); - const auto& process_list = kernel.GetProcessList(); + auto process_list = kernel.GetProcessList(); + auto it = process_list.begin(); + const auto num_processes = process_list.size(); const auto copy_amount = std::min(static_cast(out_process_ids_size), num_processes); - for (std::size_t i = 0; i < copy_amount; ++i) { - memory.Write64(out_process_ids, process_list[i]->GetProcessId()); + for (std::size_t i = 0; i < copy_amount && it != process_list.end(); ++i, ++it) { + memory.Write64(out_process_ids, (*it)->GetProcessId()); out_process_ids += sizeof(u64); } -- cgit v1.2.3 From 2f0b57ca13fb91730d7e210f6f4504357ef6cd0a Mon Sep 17 00:00:00 2001 From: Liam Date: Fri, 12 Jan 2024 19:19:07 -0500 Subject: kernel: optimize page free on shutdown --- src/core/hle/kernel/k_memory_block_manager.cpp | 4 +- src/core/hle/kernel/k_memory_block_manager.h | 4 +- src/core/hle/kernel/k_page_table_base.cpp | 73 +++++--------------------- 3 files changed, 18 insertions(+), 63 deletions(-) (limited to 'src/core/hle/kernel') diff --git a/src/core/hle/kernel/k_memory_block_manager.cpp b/src/core/hle/kernel/k_memory_block_manager.cpp index 58a1e7216..f08a6e448 100644 --- a/src/core/hle/kernel/k_memory_block_manager.cpp +++ b/src/core/hle/kernel/k_memory_block_manager.cpp @@ -28,14 +28,14 @@ Result KMemoryBlockManager::Initialize(KProcessAddress st, KProcessAddress nd, } void KMemoryBlockManager::Finalize(KMemoryBlockSlabManager* slab_manager, - HostUnmapCallback&& host_unmap_callback) { + BlockCallback&& block_callback) { // Erase every block until we have none left. auto it = m_memory_block_tree.begin(); while (it != m_memory_block_tree.end()) { KMemoryBlock* block = std::addressof(*it); it = m_memory_block_tree.erase(it); + block_callback(block->GetAddress(), block->GetSize()); slab_manager->Free(block); - host_unmap_callback(block->GetAddress(), block->GetSize()); } ASSERT(m_memory_block_tree.empty()); diff --git a/src/core/hle/kernel/k_memory_block_manager.h b/src/core/hle/kernel/k_memory_block_manager.h index cb7b6f430..377628504 100644 --- a/src/core/hle/kernel/k_memory_block_manager.h +++ b/src/core/hle/kernel/k_memory_block_manager.h @@ -85,11 +85,11 @@ public: public: KMemoryBlockManager(); - using HostUnmapCallback = std::function; + using BlockCallback = std::function; Result Initialize(KProcessAddress st, KProcessAddress nd, KMemoryBlockSlabManager* slab_manager); - void Finalize(KMemoryBlockSlabManager* slab_manager, HostUnmapCallback&& host_unmap_callback); + void Finalize(KMemoryBlockSlabManager* slab_manager, BlockCallback&& block_callback); iterator end() { return m_memory_block_tree.end(); diff --git a/src/core/hle/kernel/k_page_table_base.cpp b/src/core/hle/kernel/k_page_table_base.cpp index f01eaa164..3f0a39d33 100644 --- a/src/core/hle/kernel/k_page_table_base.cpp +++ b/src/core/hle/kernel/k_page_table_base.cpp @@ -435,69 +435,14 @@ Result KPageTableBase::FinalizeProcess() { // Only process tables should be finalized. ASSERT(!this->IsKernel()); - // HLE processes don't have memory mapped. - R_SUCCEED_IF(m_impl == nullptr); - // NOTE: Here Nintendo calls an unknown OnFinalize function. // this->OnFinalize(); // NOTE: Here Nintendo calls a second unknown OnFinalize function. // this->OnFinalize2(); - // Get implementation objects. - auto& impl = this->GetImpl(); - auto& mm = m_kernel.MemoryManager(); - - // Traverse, freeing all pages. - { - // Get the address space size. - const size_t as_size = this->GetAddressSpaceSize(); - - // Begin the traversal. - TraversalContext context; - TraversalEntry cur_entry = { - .phys_addr = 0, - .block_size = 0, - }; - - bool cur_valid = false; - TraversalEntry next_entry; - bool next_valid; - size_t tot_size = 0; - - next_valid = impl.BeginTraversal(std::addressof(next_entry), std::addressof(context), - this->GetAddressSpaceStart()); - - // Iterate over entries. - while (true) { - if ((!next_valid && !cur_valid) || - (next_valid && cur_valid && - next_entry.phys_addr == cur_entry.phys_addr + cur_entry.block_size)) { - cur_entry.block_size += next_entry.block_size; - } else { - if (cur_valid && IsHeapPhysicalAddressForFinalize(cur_entry.phys_addr)) { - mm.Close(cur_entry.phys_addr, cur_entry.block_size / PageSize); - } - - // Update tracking variables. - tot_size += cur_entry.block_size; - cur_entry = next_entry; - cur_valid = next_valid; - } - - if (cur_entry.block_size + tot_size >= as_size) { - break; - } - - next_valid = - impl.ContinueTraversal(std::addressof(next_entry), std::addressof(context)); - } - - // Handle the last block. - if (cur_valid && IsHeapPhysicalAddressForFinalize(cur_entry.phys_addr)) { - mm.Close(cur_entry.phys_addr, cur_entry.block_size / PageSize); - } - } + // NOTE: Here Nintendo does a page table walk to discover heap pages to free. + // We will use the block manager finalization below to free them. R_SUCCEED(); } @@ -505,14 +450,24 @@ Result KPageTableBase::FinalizeProcess() { void KPageTableBase::Finalize() { this->FinalizeProcess(); - auto HostUnmapCallback = [&](KProcessAddress addr, u64 size) { + auto BlockCallback = [&](KProcessAddress addr, u64 size) { if (m_impl->fastmem_arena) { m_system.DeviceMemory().buffer.Unmap(GetInteger(addr), size, false); } + + // Get physical pages. + KPageGroup pg(m_kernel, m_block_info_manager); + this->MakePageGroup(pg, addr, size / PageSize); + + // Free the pages. + pg.CloseAndReset(); }; // Finalize memory blocks. - m_memory_block_manager.Finalize(m_memory_block_slab_manager, std::move(HostUnmapCallback)); + { + KScopedLightLock lk(m_general_lock); + m_memory_block_manager.Finalize(m_memory_block_slab_manager, std::move(BlockCallback)); + } // Free any unsafe mapped memory. if (m_mapped_unsafe_physical_memory) { -- cgit v1.2.3 From d4acdac168e4b445fc8cb2e0d5e15616c5dc7dbf Mon Sep 17 00:00:00 2001 From: GPUCode Date: Wed, 3 Jan 2024 23:37:41 +0200 Subject: core: Support multiple modules per patcher --- src/core/hle/kernel/k_process.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'src/core/hle/kernel') diff --git a/src/core/hle/kernel/k_process.cpp b/src/core/hle/kernel/k_process.cpp index 068e71dff..8839ddbc2 100644 --- a/src/core/hle/kernel/k_process.cpp +++ b/src/core/hle/kernel/k_process.cpp @@ -1233,10 +1233,10 @@ void KProcess::LoadModule(CodeSet code_set, KProcessAddress base_addr) { ReprotectSegment(code_set.DataSegment(), Svc::MemoryPermission::ReadWrite); #ifdef HAS_NCE - if (this->IsApplication() && Settings::IsNceEnabled()) { + const auto& patch = code_set.PatchSegment(); + if (this->IsApplication() && Settings::IsNceEnabled() && patch.size != 0) { auto& buffer = m_kernel.System().DeviceMemory().buffer; const auto& code = code_set.CodeSegment(); - const auto& patch = code_set.PatchSegment(); buffer.Protect(GetInteger(base_addr + code.addr), code.size, Common::MemoryPermission::Read | Common::MemoryPermission::Execute); buffer.Protect(GetInteger(base_addr + patch.addr), patch.size, -- cgit v1.2.3