From 18f260b78ba0c88115b14b6e8ccefc2186f5c0e3 Mon Sep 17 00:00:00 2001
From: sin-ack <sin-ack@users.noreply.github.com>
Date: Sat, 14 Aug 2021 13:11:40 +0000
Subject: [PATCH] Kernel: Handle removal of Process from list before unref

This makes the following scenario impossible with an SMP setup:

1) CPU A enters unref() and decrements the link count to 0.
2) CPU B sees the process in the process list and ref()s it.
3) CPU A removes the process from the list and continues destructing.
4) CPU B is now holding a destructed Process object.

By holding the process list lock before doing anything with it, we
ensure that other CPUs can't find this process in the middle of it being
destructed.
---
 Kernel/Process.cpp | 20 ++++++++++++++++----
 Kernel/Process.h   |  4 +++-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index 4d9aaa18f1a..6c225ab3459 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -295,11 +295,23 @@ Process::~Process()
     VERIFY(!m_alarm_timer);
 
     PerformanceManager::add_process_exit_event(*this);
+}
 
-    if (m_list_node.is_in_list())
-        processes().with_exclusive([&](auto& list) {
-            list.remove(*this);
-        });
+bool Process::unref() const
+{
+    // NOTE: We need to obtain the process list lock before doing anything,
+    //       because otherwise someone might get in between us lowering the
+    //       refcount and acquiring the lock.
+    return processes().with_exclusive([&](auto& list) {
+        auto new_ref_count = deref_base();
+        if (new_ref_count > 0)
+            return false;
+
+        if (m_list_node.is_in_list())
+            list.remove(*const_cast<Process*>(this));
+        delete this;
+        return true;
+    });
 }
 
 // Make sure the compiler doesn't "optimize away" this function:
diff --git a/Kernel/Process.h b/Kernel/Process.h
index 4373c530f76..e7eeeb8c1d1 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -85,7 +85,7 @@ typedef HashMap<FlatPtr, RefPtr<FutexQueue>> FutexQueues;
 struct LoadResult;
 
 class Process
-    : public RefCounted<Process>
+    : public AK::RefCountedBase
     , public Weakable<Process> {
 
     class ProtectedValues {
@@ -175,6 +175,8 @@ public:
     static RefPtr<Process> create_kernel_process(RefPtr<Thread>& first_thread, String&& name, void (*entry)(void*), void* entry_data = nullptr, u32 affinity = THREAD_AFFINITY_DEFAULT, RegisterProcess do_register = RegisterProcess::Yes);
     static RefPtr<Process> create_user_process(RefPtr<Thread>& first_thread, const String& path, uid_t, gid_t, ProcessID ppid, int& error, Vector<String>&& arguments = Vector<String>(), Vector<String>&& environment = Vector<String>(), TTY* = nullptr);
     static void register_new(Process&);
+
+    bool unref() const;
     ~Process();
 
     static NonnullRefPtrVector<Process> all_processes();