From 084a80f98014f18de47056567cb89806a5ad4b30 Mon Sep 17 00:00:00 2001
From: Michal Klocek <michal.klocek@qt.io>
Date: Fri, 17 Aug 2018 17:48:55 +0200
Subject: [Backport] Security Bug 831117 2/2
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Oilpan: Clear all Persistents on thread termination

Clear all persistents on thread termination, because when we
have a bug and it leaves a Persistent behind, it will not be
a stale pointer and just causes null dereference.

Since we disabled PersistentHeapCollections on non-main threads
we can assume all PersistentNode::Self() can be cast to
Persistent<DummyGCBase>

Bug: 831117
Reviewed-on: https://chromium-review.googlesource.com/1025547

-----------------------------------------------------------------

Oilpan: Clear all Persistents on thread termination.

Persistents should be cleared when NumberOfPersistents() != 0.

Bug: 831117
Reviewed-on: https://chromium-review.googlesource.com/1089460

-----------------------------------------------------------------

Change-Id: Ibe6f61a894195d240b288e995e633c4749870663
Reviewed-by: Michael Brüning <michael.bruning@qt.io>
---
 .../WebKit/Source/platform/heap/PersistentNode.cpp | 27 +++++++++++++++++++---
 .../WebKit/Source/platform/heap/PersistentNode.h   |  1 +
 .../WebKit/Source/platform/heap/ThreadState.cpp    | 21 +++++++++++++++--
 3 files changed, 44 insertions(+), 5 deletions(-)

diff --git a/chromium/third_party/WebKit/Source/platform/heap/PersistentNode.cpp b/chromium/third_party/WebKit/Source/platform/heap/PersistentNode.cpp
index a9499556062..10a035f8472 100644
--- a/chromium/third_party/WebKit/Source/platform/heap/PersistentNode.cpp
+++ b/chromium/third_party/WebKit/Source/platform/heap/PersistentNode.cpp
@@ -125,6 +125,30 @@ void PersistentRegion::TracePersistentNodes(Visitor* visitor,
 #endif
 }
 
+void PersistentRegion::PrepareForThreadStateTermination() {
+  DCHECK(!IsMainThread());
+  PersistentNodeSlots* slots = slots_;
+  while (slots) {
+    for (int i = 0; i < PersistentNodeSlots::kSlotCount; ++i) {
+      PersistentNode* node = &slots->slot_[i];
+      if (node->IsUnused())
+        continue;
+      // It is safe to cast to Persistent<DummyGCBase> because persistent heap
+      // collections are banned in non-main threads.
+
+      Persistent<DummyGCBase>* persistent =
+          reinterpret_cast<Persistent<DummyGCBase>*>(node->Self());
+      DCHECK(persistent);
+      persistent->Clear();
+      DCHECK(node->IsUnused());
+    }
+    slots = slots->next_;
+  }
+#if DCHECK_IS_ON()
+  DCHECK_EQ(persistent_count_, 0);
+#endif
+}
+
 bool CrossThreadPersistentRegion::ShouldTracePersistentNode(
     Visitor* visitor,
     PersistentNode* node) {
@@ -145,9 +169,6 @@ void CrossThreadPersistentRegion::PrepareForThreadStateTermination(
   // out the underlying heap reference.
   MutexLocker lock(mutex_);
 
-  // TODO(sof): consider ways of reducing overhead. (e.g., tracking number of
-  // active CrossThreadPersistent<>s pointing into the heaps of each ThreadState
-  // and use that count to bail out early.)
   PersistentNodeSlots* slots = persistent_region_->slots_;
   while (slots) {
     for (int i = 0; i < PersistentNodeSlots::kSlotCount; ++i) {
diff --git a/chromium/third_party/WebKit/Source/platform/heap/PersistentNode.h b/chromium/third_party/WebKit/Source/platform/heap/PersistentNode.h
index 60d179c6023..cecbb681973 100644
--- a/chromium/third_party/WebKit/Source/platform/heap/PersistentNode.h
+++ b/chromium/third_party/WebKit/Source/platform/heap/PersistentNode.h
@@ -151,6 +151,7 @@ class PLATFORM_EXPORT PersistentRegion final {
       Visitor*,
       ShouldTraceCallback = PersistentRegion::ShouldTracePersistentNode);
   int NumberOfPersistents();
+  void PrepareForThreadStateTermination();
 
  private:
   friend CrossThreadPersistentRegion;
diff --git a/chromium/third_party/WebKit/Source/platform/heap/ThreadState.cpp b/chromium/third_party/WebKit/Source/platform/heap/ThreadState.cpp
index c80d930071c..8b9629b17f1 100644
--- a/chromium/third_party/WebKit/Source/platform/heap/ThreadState.cpp
+++ b/chromium/third_party/WebKit/Source/platform/heap/ThreadState.cpp
@@ -83,6 +83,8 @@ uint8_t ThreadState::main_thread_state_storage_[sizeof(ThreadState)];
 
 const size_t kDefaultAllocatedObjectSizeThreshold = 100 * 1024;
 
+constexpr size_t kMaxTerminationGCLoops = 20;
+
 namespace {
 
 const char* GcReasonString(BlinkGC::GCReason reason) {
@@ -214,9 +216,24 @@ void ThreadState::RunTerminationGC() {
     old_count = current_count;
     current_count = GetPersistentRegion()->NumberOfPersistents();
   }
+
   // We should not have any persistents left when getting to this point,
-  // if we have it is probably a bug so adding a debug ASSERT to catch this.
-  DCHECK(!current_count);
+  // if we have it is a bug, and we have a reference cycle or a missing
+  // RegisterAsStaticReference. Clearing out all the Persistents will avoid
+  // stale pointers and gets them reported as nullptr dereferences.
+
+  if (current_count) {
+    for (size_t i = 0; i < kMaxTerminationGCLoops &&
+                       GetPersistentRegion()->NumberOfPersistents();
+         i++) {
+      GetPersistentRegion()->PrepareForThreadStateTermination();
+      CollectGarbage(BlinkGC::kNoHeapPointersOnStack, BlinkGC::kGCWithSweep,
+                     BlinkGC::kThreadTerminationGC);
+    }
+  }
+
+  CHECK(!GetPersistentRegion()->NumberOfPersistents());
+
   // All of pre-finalizers should be consumed.
   DCHECK(ordered_pre_finalizers_.IsEmpty());
   CHECK_EQ(GcState(), kNoGCScheduled);
-- 
cgit v1.2.3