Kernel: Make Region weakable and use WeakPtr<Region> instead of Region*

This turns use-after-free bugs into null pointer dereferences instead.
cubiclove · Feb 24, 2020 · 30a8991 · 30a8991
1 parent 79576f9
commit 30a8991
Show file tree

Hide file tree

Showing 5 changed files with 14 additions and 8 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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)