[Backport] CVE-2018-6158

Reland "[oilpan] Fix GCInfoTable for multiple threads"

Previously, grow and access from different threads could lead to a race
on the table backing; see bug.

- Rework the table to work on an existing reservation.
- Commit upon growing, avoiding any copies.

Reland:
- Fix an issue for component builds were the singleton was instantiated
 multiple times.

Bug: chromium:841280

Reviewed-on: https://chromium-review.googlesource.com/1068636
Change-Id: Iaddee6b594d6853dcbb29aec2c29330987c3b6a9
Reviewed-by: Michael Brüning <michael.bruning@qt.io>

5 files changed, 142 insertions, 67 deletions

diff --git a/chromium/third_party/WebKit/Source/platform/heap/GCInfo.cpp b/chromium/third_party/WebKit/Source/platform/heap/GCInfo.cpp
index 6a2c3b94a2d..449f7349274 100644
--- a/chromium/third_party/WebKit/Source/platform/heap/GCInfo.cpp
+++ b/chromium/third_party/WebKit/Source/platform/heap/GCInfo.cpp
@@ -9,20 +9,64 @@
 
 namespace blink {
 
-// GCInfo indices start from 1 for heap objects, with 0 being treated
-// specially as the index for freelist entries and large heap objects.
-size_t GCInfoTable::gc_info_index_ = 0;
+namespace {
+
+inline uintptr_t RoundUpToPageAllocationGranularity(uintptr_t address) {
+  return (address + base::kPageAllocationGranularityOffsetMask) &
+         base::kPageAllocationGranularityBaseMask;
+}
+
+constexpr inline bool IsPowerOfTwo(size_t value) {
+  return value > 0 && (value & (value - 1)) == 0;
+}
+
+constexpr size_t kEntrySize = sizeof(GCInfo*);
+
+// Allocation and resizing are built around the following invariants.
+static_assert(IsPowerOfTwo(kEntrySize),
+              "GCInfoTable entries size must be power of "
+              "two");
+static_assert(
+    0 == base::kPageAllocationGranularity % base::kSystemPageSize,
+    "System page size must be a multiple of page page allocation granularity");
+
+size_t ComputeInitialTableLimit() {
+  // (Light) experimentation suggests that Blink doesn't need more than this
+  // while handling content on popular web properties.
+  constexpr size_t kInitialWantedLimit = 512;
+
+  // Different OSes have different page sizes, so we have to choose the minimum
+  // of memory wanted and OS page size.
+
+  constexpr size_t memory_wanted = kInitialWantedLimit * kEntrySize;
+  return RoundUpToPageAllocationGranularity(memory_wanted) / kEntrySize;
+}
 
-size_t GCInfoTable::gc_info_table_size_ = 0;
-GCInfo const** g_gc_info_table = nullptr;
+
+size_t MaxTableSize() {
+  static const size_t kMaxTableSize =
+      RoundUpToPageAllocationGranularity(GCInfoTable::kMaxIndex * kEntrySize);
+  return kMaxTableSize;
+}
+
+}  // namespace
+
+GCInfoTable* GCInfoTable::global_table_ = nullptr;
+
+void GCInfoTable::CreateGlobalTable() {
+  DEFINE_STATIC_LOCAL(GCInfoTable, table, ());
+  global_table_ = &table;
+}
 
 void GCInfoTable::EnsureGCInfoIndex(const GCInfo* gc_info,
                                     size_t* gc_info_index_slot) {
   DCHECK(gc_info);
   DCHECK(gc_info_index_slot);
-  // Keep a global GCInfoTable lock while allocating a new slot.
-  DEFINE_THREAD_SAFE_STATIC_LOCAL(Mutex, mutex, ());
-  MutexLocker locker(mutex);
+
+  // Ensuring a new index involves current index adjustment as well
+  // as potentially resizing the table, both operations that require
+  // a lock.
+  MutexLocker locker(table_mutex_);
 
   // If more than one thread ends up allocating a slot for
   // the same GCInfo, have later threads reuse the slot
@@ -30,37 +74,52 @@ void GCInfoTable::EnsureGCInfoIndex(const GCInfo* gc_info,
   if (*gc_info_index_slot)
     return;
 
-  int index = ++gc_info_index_;
+  int index = ++current_index_;
   size_t gc_info_index = static_cast<size_t>(index);
   CHECK(gc_info_index < GCInfoTable::kMaxIndex);
-  if (gc_info_index >= gc_info_table_size_)
+  if (current_index_ >= limit_)
     Resize();
 
-  g_gc_info_table[gc_info_index] = gc_info;
+  table_[gc_info_index] = gc_info;
   ReleaseStore(reinterpret_cast<int*>(gc_info_index_slot), index);
 }
 
 void GCInfoTable::Resize() {
-  static const int kGcInfoZapValue = 0x33;
-  // (Light) experimentation suggests that Blink doesn't need
-  // more than this while handling content on popular web properties.
-  const size_t kInitialSize = 512;
-
-  size_t new_size =
-      gc_info_table_size_ ? 2 * gc_info_table_size_ : kInitialSize;
-  DCHECK(new_size < GCInfoTable::kMaxIndex);
-  g_gc_info_table =
-      reinterpret_cast<GCInfo const**>(WTF::Partitions::FastRealloc(
-          g_gc_info_table, new_size * sizeof(GCInfo), "GCInfo"));
-  DCHECK(g_gc_info_table);
-  memset(reinterpret_cast<uint8_t*>(g_gc_info_table) +
-             gc_info_table_size_ * sizeof(GCInfo),
-         kGcInfoZapValue, (new_size - gc_info_table_size_) * sizeof(GCInfo));
-  gc_info_table_size_ = new_size;
+  const size_t new_limit = (limit_) ? 2 * limit_ : ComputeInitialTableLimit();
+  const size_t old_committed_size = limit_ * kEntrySize;
+  const size_t new_committed_size = new_limit * kEntrySize;
+  CHECK(table_);
+  CHECK_EQ(0u, new_committed_size % base::kPageAllocationGranularity);
+  CHECK_GE(MaxTableSize(), limit_ * kEntrySize);
+  // Recommitting and zapping assumes byte-addressable storage.
+  uint8_t* const current_table_end =
+      reinterpret_cast<uint8_t*>(table_) + old_committed_size;
+  const size_t table_size_delta = new_committed_size - old_committed_size;
+  // Commit the new size and allow read/write.
+  // TODO(ajwong): SetSystemPagesAccess should be part of RecommitSystemPages to
+  // avoid having two calls here.
+  bool ok = base::SetSystemPagesAccess(current_table_end, table_size_delta,
+                                       base::PageReadWrite);
+  CHECK(ok);
+  ok = base::RecommitSystemPages(current_table_end, table_size_delta,
+                                 base::PageReadWrite);
+  CHECK(ok);
+
+#if DCHECK_IS_ON()
+  // Check that newly-committed memory is zero-initialized.
+  for (size_t i = 0; i < (table_size_delta / sizeof(uintptr_t)); ++i) {
+    DCHECK(!reinterpret_cast<uintptr_t*>(current_table_end)[i]);
+  }
+#endif  // DCHECK_IS_ON()
+  limit_ = new_limit;
 }
 
-void GCInfoTable::Init() {
-  CHECK(!g_gc_info_table);
+GCInfoTable::GCInfoTable() {
+  CHECK(!table_);
+  table_ = reinterpret_cast<GCInfo const**>(base::AllocPages(
+      nullptr, MaxTableSize(), base::kPageAllocationGranularity,
+      base::PageInaccessible));
+  CHECK(table_);
   Resize();
 }
 
diff --git a/chromium/third_party/WebKit/Source/platform/heap/GCInfo.h b/chromium/third_party/WebKit/Source/platform/heap/GCInfo.h
index a07634d454d..c6f56591c3c 100644
--- a/chromium/third_party/WebKit/Source/platform/heap/GCInfo.h
+++ b/chromium/third_party/WebKit/Source/platform/heap/GCInfo.h
@@ -151,38 +151,62 @@ struct GCInfo {
   bool has_v_table_;
 };
 
-// s_gcInfoTable holds the per-class GCInfo descriptors; each HeapObjectHeader
-// keeps an index into this table.
-extern PLATFORM_EXPORT GCInfo const** g_gc_info_table;
-
 #if DCHECK_IS_ON()
 PLATFORM_EXPORT void AssertObjectHasGCInfo(const void*, size_t gc_info_index);
 #endif
 
-class GCInfoTable {
-  STATIC_ONLY(GCInfoTable);
-
+class PLATFORM_EXPORT GCInfoTable {
  public:
-  PLATFORM_EXPORT static void EnsureGCInfoIndex(const GCInfo*, size_t*);
-
-  static void Init();
-
-  static size_t GcInfoIndex() { return gc_info_index_; }
-
-  // The (max + 1) GCInfo index supported.
+  // At maximum |kMaxIndex - 1| indices are supported.
   //
   // We assume that 14 bits is enough to represent all possible types: during
   // telemetry runs, we see about 1,000 different types; looking at the output
   // of the Oilpan GC Clang plugin, there appear to be at most about 6,000
   // types. Thus 14 bits should be more than twice as many bits as we will ever
   // need.
-  static const size_t kMaxIndex = 1 << 14;
+  static constexpr size_t kMaxIndex = 1 << 14;
+
+  // Sets up a singleton table that can be acquired using Get().
+  static void CreateGlobalTable();
+
+  static GCInfoTable& Get() { return *global_table_; }
+
+  inline const GCInfo* GCInfoFromIndex(size_t index) {
+    DCHECK_GE(index, 1u);
+    DCHECK(index < kMaxIndex);
+    DCHECK(table_);
+    const GCInfo* info = table_[index];
+    DCHECK(info);
+    return info;
+  }
+
+  void EnsureGCInfoIndex(const GCInfo*, size_t*);
+
+  size_t GcInfoIndex() { return current_index_; }
 
  private:
-  static void Resize();
+  // Use GCInfoTable::Get() for retrieving the global table outside of testing
+
+  // code.
+  GCInfoTable();
+
+  void Resize();
+
+  // Singleton for each process. Retrieved through Get().
+  static GCInfoTable* global_table_;
 
-  static size_t gc_info_index_;
-  static size_t gc_info_table_size_;
+  // Holds the per-class GCInfo descriptors; each HeapObjectHeader keeps an
+  // index into this table.
+  const GCInfo** table_ = nullptr;
+
+  // GCInfo indices start from 1 for heap objects, with 0 being treated
+  // specially as the index for freelist entries and large heap objects.
+  size_t current_index_ = 0;
+
+  // The limit (exclusive) of the currently allocated table.
+  size_t limit_ = 0;
+
+  Mutex table_mutex_;
 };
 
 // GCInfoAtBaseType should be used when returning a unique 14 bit integer
@@ -196,11 +220,9 @@ struct GCInfoAtBaseType {
         TraceTrait<T>::Trace, FinalizerTrait<T>::Finalize,
         FinalizerTrait<T>::kNonTrivialFinalizer, std::is_polymorphic<T>::value,
     };
-
     static size_t gc_info_index = 0;
-    DCHECK(g_gc_info_table);
     if (!AcquireLoad(&gc_info_index))
-      GCInfoTable::EnsureGCInfoIndex(&kGcInfo, &gc_info_index);
+      GCInfoTable::Get().EnsureGCInfoIndex(&kGcInfo, &gc_info_index);
     DCHECK_GE(gc_info_index, 1u);
     DCHECK(gc_info_index < GCInfoTable::kMaxIndex);
     return gc_info_index;
diff --git a/chromium/third_party/WebKit/Source/platform/heap/Heap.cpp b/chromium/third_party/WebKit/Source/platform/heap/Heap.cpp
index b35635e42d4..c08f6f27e73 100644
--- a/chromium/third_party/WebKit/Source/platform/heap/Heap.cpp
+++ b/chromium/third_party/WebKit/Source/platform/heap/Heap.cpp
@@ -66,7 +66,7 @@ void ProcessHeap::Init() {
   total_allocated_object_size_ = 0;
   total_marked_object_size_ = 0;
 
-  GCInfoTable::Init();
+  GCInfoTable::CreateGlobalTable();
   CallbackStackMemoryPool::Instance().Initialize();
 }
 
@@ -711,7 +711,7 @@ void ThreadHeap::TakeSnapshot(SnapshotType type) {
 
   // 0 is used as index for freelist entries. Objects are indexed 1 to
   // gcInfoIndex.
-  ThreadState::GCSnapshotInfo info(GCInfoTable::GcInfoIndex() + 1);
+  ThreadState::GCSnapshotInfo info(GCInfoTable::Get().GcInfoIndex() + 1);
   String thread_dump_name =
       String::Format("blink_gc/thread_%lu",
                      static_cast<unsigned long>(thread_state_->ThreadId()));
@@ -761,8 +761,8 @@ void ThreadHeap::TakeSnapshot(SnapshotType type) {
   size_t total_dead_count = 0;
   size_t total_live_size = 0;
   size_t total_dead_size = 0;
-  for (size_t gc_info_index = 1; gc_info_index <= GCInfoTable::GcInfoIndex();
-       ++gc_info_index) {
+  for (size_t gc_info_index = 1;
+      gc_info_index <= GCInfoTable::Get().GcInfoIndex(); ++gc_info_index) {
     total_live_count += info.live_count[gc_info_index];
     total_dead_count += info.dead_count[gc_info_index];
     total_live_size += info.live_size[gc_info_index];
@@ -837,7 +837,7 @@ void ThreadHeap::WriteBarrierInternal(BasePage* page, const void* value) {
   // Mark and push trace callback.
   header->Mark();
   PushTraceCallback(header->Payload(),
-                    ThreadHeap::GcInfo(header->GcInfoIndex())->trace_);
+                    GCInfoTable::Get().GCInfoFromIndex(header->GcInfoIndex())->trace_);
 }
 
 ThreadHeap* ThreadHeap::main_thread_heap_ = nullptr;
diff --git a/chromium/third_party/WebKit/Source/platform/heap/Heap.h b/chromium/third_party/WebKit/Source/platform/heap/Heap.h
index 33753e94a52..6c1ba3a0ae7 100644
--- a/chromium/third_party/WebKit/Source/platform/heap/Heap.h
+++ b/chromium/third_party/WebKit/Source/platform/heap/Heap.h
@@ -413,15 +413,6 @@ class PLATFORM_EXPORT ThreadHeap {
   // heap-page if one exists.
   BasePage* LookupPageForAddress(Address);
 
-  static const GCInfo* GcInfo(size_t gc_info_index) {
-    DCHECK_GE(gc_info_index, 1u);
-    DCHECK(gc_info_index < GCInfoTable::kMaxIndex);
-    DCHECK(g_gc_info_table);
-    const GCInfo* info = g_gc_info_table[gc_info_index];
-    DCHECK(info);
-    return info;
-  }
-
   static void ReportMemoryUsageHistogram();
   static void ReportMemoryUsageForTracing();
 
@@ -761,7 +752,9 @@ Address ThreadHeap::Reallocate(void* previous, size_t size) {
 
   size_t gc_info_index = GCInfoTrait<T>::Index();
   // TODO(haraken): We don't support reallocate() for finalizable objects.
-  DCHECK(!ThreadHeap::GcInfo(previous_header->GcInfoIndex())->HasFinalizer());
+  DCHECK(!GCInfoTable::Get()
+             .GCInfoFromIndex(previous_header->GcInfoIndex())
+             ->HasFinalizer());
   DCHECK_EQ(previous_header->GcInfoIndex(), gc_info_index);
   HeapAllocHooks::FreeHookIfEnabled(static_cast<Address>(previous));
   Address address;
diff --git a/chromium/third_party/WebKit/Source/platform/heap/HeapPage.cpp b/chromium/third_party/WebKit/Source/platform/heap/HeapPage.cpp
index 63e72a88a04..d8f76cdcf92 100644
--- a/chromium/third_party/WebKit/Source/platform/heap/HeapPage.cpp
+++ b/chromium/third_party/WebKit/Source/platform/heap/HeapPage.cpp
@@ -96,7 +96,7 @@ void HeapObjectHeader::ZapMagic() {
 
 void HeapObjectHeader::Finalize(Address object, size_t object_size) {
   HeapAllocHooks::FreeHookIfEnabled(object);
-  const GCInfo* gc_info = ThreadHeap::GcInfo(GcInfoIndex());
+  const GCInfo* gc_info = GCInfoTable::Get().GCInfoFromIndex(GcInfoIndex());
   if (gc_info->HasFinalizer())
     gc_info->finalize_(object);
 
@@ -1698,7 +1698,8 @@ static bool IsUninitializedMemory(void* object_pointer, size_t object_size) {
 #endif
 
 static void MarkPointer(Visitor* visitor, HeapObjectHeader* header) {
-  const GCInfo* gc_info = ThreadHeap::GcInfo(header->GcInfoIndex());
+  const GCInfo* gc_info =
+      GCInfoTable::Get().GCInfoFromIndex(header->GcInfoIndex());
   if (gc_info->HasVTable() && !VTableInitialized(header->Payload())) {
     // We hit this branch when a GC strikes before GarbageCollected<>'s
     // constructor runs.