From b86443f0e108ea71a9e3e5eae45e937ced6efaa5 Mon Sep 17 00:00:00 2001 From: Andreas Kling Date: Sat, 8 Jan 2022 15:43:56 +0100 Subject: [PATCH] Kernel: Lock weak pointer revocation during listed-ref-counted unref When doing the last unref() on a listed-ref-counted object, we keep the list locked while mutating the ref count. The destructor itself is invoked after unlocking the list. This was racy with weakable classes, since their weak pointer factory still pointed to the object after we'd decided to destroy it. That opened a small time window where someone could try to strong-ref a weak pointer to an object after it was removed from the list, but just before the destructor got invoked. This patch closes the race window by explicitly revoking all weak pointers while the list is locked. --- Kernel/Library/ListedRefCounted.h | 6 +++++- Kernel/Net/TCPSocket.cpp | 1 + Kernel/TTY/SlavePTY.cpp | 1 + 3 files changed, 7 insertions(+), 1 deletion(-) diff --git a/Kernel/Library/ListedRefCounted.h b/Kernel/Library/ListedRefCounted.h index 2dd8f470393..67e31a26a89 100644 --- a/Kernel/Library/ListedRefCounted.h +++ b/Kernel/Library/ListedRefCounted.h @@ -29,8 +29,12 @@ public: auto callback = [&](auto& list) { auto new_ref_count = deref_base(); - if (new_ref_count == 0) + if (new_ref_count == 0) { list.remove(const_cast(*that)); + if constexpr (requires { that->revoke_weak_ptrs(); }) { + that->revoke_weak_ptrs(); + } + } return new_ref_count; }; diff --git a/Kernel/Net/TCPSocket.cpp b/Kernel/Net/TCPSocket.cpp index 52a915f4ae1..60bb2d949ea 100644 --- a/Kernel/Net/TCPSocket.cpp +++ b/Kernel/Net/TCPSocket.cpp @@ -35,6 +35,7 @@ bool TCPSocket::unref() const if (deref_base()) return false; table.remove(tuple()); + const_cast(*this).revoke_weak_ptrs(); return true; }); if (did_hit_zero) { diff --git a/Kernel/TTY/SlavePTY.cpp b/Kernel/TTY/SlavePTY.cpp index 512f54e3837..3de56626040 100644 --- a/Kernel/TTY/SlavePTY.cpp +++ b/Kernel/TTY/SlavePTY.cpp @@ -26,6 +26,7 @@ bool SlavePTY::unref() const if (deref_base()) return false; m_list_node.remove(); + const_cast(*this).revoke_weak_ptrs(); return true; }); if (did_hit_zero) {