From 1d96c30488a80602c1e773fe97c48f8711dd0361 Mon Sep 17 00:00:00 2001 From: Idan Horowitz Date: Sun, 10 Jul 2022 16:40:48 +0300 Subject: [PATCH] Kernel: Stop leaking leftover committed cow pages from forked processes Since both the parent process and child process hold a reference to the COW committed set, once the child process exits, the committed COW pages are effectively leaked, only being slowly re-claimed each time the parent process writes to one of them, realizing it's no longer shared, and uncommitting it. In order to mitigate this we now hold a weak reference the parent VMObject from which the pages are cloned, and we use it on destruction when available to drop the reference to the committed set from it as well. --- Kernel/Memory/AnonymousVMObject.cpp | 20 ++++++++++++++++---- Kernel/Memory/AnonymousVMObject.h | 3 ++- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/Kernel/Memory/AnonymousVMObject.cpp b/Kernel/Memory/AnonymousVMObject.cpp index 2c94e67283..59a7ead9d3 100644 --- a/Kernel/Memory/AnonymousVMObject.cpp +++ b/Kernel/Memory/AnonymousVMObject.cpp @@ -124,7 +124,8 @@ ErrorOr> AnonymousVMObject::try_create_for_phys ErrorOr> AnonymousVMObject::try_create_with_shared_cow(AnonymousVMObject const& other, NonnullRefPtr shared_committed_cow_pages, FixedArray>&& new_physical_pages) { - auto vmobject = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AnonymousVMObject(other, move(shared_committed_cow_pages), move(new_physical_pages)))); + auto weak_parent = TRY(other.try_make_weak_ptr()); + auto vmobject = TRY(adopt_nonnull_ref_or_enomem(new (nothrow) AnonymousVMObject(move(weak_parent), move(shared_committed_cow_pages), move(new_physical_pages)))); TRY(vmobject->ensure_cow_map()); @@ -159,14 +160,25 @@ AnonymousVMObject::AnonymousVMObject(FixedArray>&& new_phys { } -AnonymousVMObject::AnonymousVMObject(AnonymousVMObject const& other, NonnullRefPtr shared_committed_cow_pages, FixedArray>&& new_physical_pages) +AnonymousVMObject::AnonymousVMObject(WeakPtr other, NonnullRefPtr shared_committed_cow_pages, FixedArray>&& new_physical_pages) : VMObject(move(new_physical_pages)) + , m_cow_parent(move(other)) , m_shared_committed_cow_pages(move(shared_committed_cow_pages)) - , m_purgeable(other.m_purgeable) + , m_purgeable(m_cow_parent.strong_ref()->m_purgeable) { } -AnonymousVMObject::~AnonymousVMObject() = default; +AnonymousVMObject::~AnonymousVMObject() +{ + if (!m_shared_committed_cow_pages || m_shared_committed_cow_pages->is_empty()) + return; + auto cow_parent = m_cow_parent.strong_ref(); + if (!cow_parent) + return; + SpinlockLocker lock(cow_parent->m_lock); + if (cow_parent->m_shared_committed_cow_pages == m_shared_committed_cow_pages) + cow_parent->m_shared_committed_cow_pages.clear(); +} size_t AnonymousVMObject::purge() { diff --git a/Kernel/Memory/AnonymousVMObject.h b/Kernel/Memory/AnonymousVMObject.h index c940333c27..562b8caa76 100644 --- a/Kernel/Memory/AnonymousVMObject.h +++ b/Kernel/Memory/AnonymousVMObject.h @@ -46,7 +46,7 @@ private: explicit AnonymousVMObject(FixedArray>&&, AllocationStrategy, Optional); explicit AnonymousVMObject(PhysicalAddress, FixedArray>&&); explicit AnonymousVMObject(FixedArray>&&); - explicit AnonymousVMObject(AnonymousVMObject const&, NonnullRefPtr, FixedArray>&&); + explicit AnonymousVMObject(WeakPtr, NonnullRefPtr, FixedArray>&&); virtual StringView class_name() const override { return "AnonymousVMObject"sv; } @@ -82,6 +82,7 @@ private: CommittedPhysicalPageSet m_committed_pages; }; + WeakPtr m_cow_parent; RefPtr m_shared_committed_cow_pages; bool m_purgeable { false };