From 30a8991dbf298a8f955210ce05a7f7fe019ac388 Mon Sep 17 00:00:00 2001
From: Andreas Kling <kling@serenityos.org>
Date: Mon, 24 Feb 2020 13:24:30 +0100
Subject: [PATCH] Kernel: Make Region weakable and use WeakPtr<Region> instead
 of Region*

This turns use-after-free bugs into null pointer dereferences instead.
---
 Kernel/Process.cpp      | 2 +-
 Kernel/Process.h        | 3 ++-
 Kernel/SharedBuffer.cpp | 9 +++++----
 Kernel/SharedBuffer.h   | 3 ++-
 Kernel/VM/Region.h      | 5 ++++-
 5 files changed, 14 insertions(+), 8 deletions(-)

diff --git a/Kernel/Process.cpp b/Kernel/Process.cpp
index 73bc7266fb62a0..f67312d8f93a5c 100644
--- a/Kernel/Process.cpp
+++ b/Kernel/Process.cpp
@@ -273,7 +273,7 @@ Region* Process::region_from_range(const Range& range)
     for (auto& region : m_regions) {
         if (region.vaddr() == range.base() && region.size() == size) {
             m_region_lookup_cache.range = range;
-            m_region_lookup_cache.region = &region;
+            m_region_lookup_cache.region = region.make_weak_ptr();
             return &region;
         }
     }
diff --git a/Kernel/Process.h b/Kernel/Process.h
index 056bc5e2456104..fd9e3f0a0c092e 100644
--- a/Kernel/Process.h
+++ b/Kernel/Process.h
@@ -31,6 +31,7 @@
 #include <AK/InlineLinkedList.h>
 #include <AK/NonnullOwnPtrVector.h>
 #include <AK/String.h>
+#include <AK/WeakPtr.h>
 #include <Kernel/FileSystem/InodeMetadata.h>
 #include <Kernel/Forward.h>
 #include <Kernel/Lock.h>
@@ -479,7 +480,7 @@ class Process : public InlineLinkedListNode<Process> {
     NonnullOwnPtrVector<Region> m_regions;
     struct RegionLookupCache {
         Range range;
-        Region* region { nullptr };
+        WeakPtr<Region> region;
     };
     RegionLookupCache m_region_lookup_cache;
 
diff --git a/Kernel/SharedBuffer.cpp b/Kernel/SharedBuffer.cpp
index e62aef62003058..34594b2c86ecde 100644
--- a/Kernel/SharedBuffer.cpp
+++ b/Kernel/SharedBuffer.cpp
@@ -86,12 +86,13 @@ void* SharedBuffer::ref_for_process_and_get_address(Process& process)
 
     for (auto& ref : m_refs) {
         if (ref.pid == process.pid()) {
-            ref.count++;
-            m_total_refs++;
-            if (ref.region == nullptr) {
-                ref.region = process.allocate_region_with_vmobject(VirtualAddress(), size(), m_vmobject, 0, "SharedBuffer", PROT_READ | (m_writable ? PROT_WRITE : 0));
+            if (!ref.region) {
+                auto* region = process.allocate_region_with_vmobject(VirtualAddress(), size(), m_vmobject, 0, "SharedBuffer", PROT_READ | (m_writable ? PROT_WRITE : 0));
+                ref.region = region->make_weak_ptr();
                 ref.region->set_shared(true);
             }
+            ref.count++;
+            m_total_refs++;
             sanity_check("ref_for_process_and_get_address");
             return ref.region->vaddr().as_ptr();
         }
diff --git a/Kernel/SharedBuffer.h b/Kernel/SharedBuffer.h
index 949df014283d80..f5d12d6211d82f 100644
--- a/Kernel/SharedBuffer.h
+++ b/Kernel/SharedBuffer.h
@@ -27,6 +27,7 @@
 #pragma once
 
 #include <AK/OwnPtr.h>
+#include <AK/WeakPtr.h>
 #include <Kernel/VM/MemoryManager.h>
 #include <Kernel/VM/PurgeableVMObject.h>
 
@@ -42,7 +43,7 @@ class SharedBuffer {
 
         pid_t pid;
         unsigned count { 0 };
-        Region* region { nullptr };
+        WeakPtr<Region> region;
     };
 
 public:
diff --git a/Kernel/VM/Region.h b/Kernel/VM/Region.h
index f71615b7923c44..3a96f3616e7906 100644
--- a/Kernel/VM/Region.h
+++ b/Kernel/VM/Region.h
@@ -28,6 +28,7 @@
 
 #include <AK/InlineLinkedList.h>
 #include <AK/String.h>
+#include <AK/Weakable.h>
 #include <Kernel/Heap/SlabAllocator.h>
 #include <Kernel/VM/RangeAllocator.h>
 
@@ -41,7 +42,9 @@ enum class PageFaultResponse {
     Continue,
 };
 
-class Region final : public InlineLinkedListNode<Region> {
+class Region final
+    : public InlineLinkedListNode<Region>
+    , public Weakable<Region> {
     friend class MemoryManager;
 
     MAKE_SLAB_ALLOCATED(Region)