cppgc: shared-cage: Fix UaF when lsan is enabled

Before this CL, the caged heap was lazily initialized upon the first
call of HeapBase ctor. CagedHeap keeps a pointer to PageAllocator which
was provided from cppgc::Platform through the HeapBase ctor. This was
not generally safe: the platform is not enforced to be singleton. If it
happens to die first, then CagedHeap will have a stale pointer. The CL
fixes it simply by moving caged-heap initialization to
cppgc::InitializeProcess(), which already requires a constantly living
PageAllocator.

Bug: chromium:1338030
Change-Id: Ifb70a2db233ef36a99c919db09bed9ff9f3708ac
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3732107
Commit-Queue: Anton Bikineev <bikineev@chromium.org>
Reviewed-by: Michael Lippautz <mlippautz@chromium.org>
Cr-Commit-Position: refs/heads/main@{#81422}

include/cppgc/platform.h

@@ -132,8 +132,8 @@ class V8_EXPORT Platform {
  *
  * Can be called multiple times when paired with `ShutdownProcess()`.
  *
- * \param page_allocator The allocator used for maintaining meta data. Must not
- *   change between multiple calls to InitializeProcess.
+ * \param page_allocator The allocator used for maintaining meta data. Must stay
+ *   always alive and not change between multiple calls to InitializeProcess.
  */
 V8_EXPORT void InitializeProcess(PageAllocator* page_allocator);
 

src/heap/cppgc/caged-heap.h

@@ -50,6 +50,8 @@ class V8_EXPORT_PRIVATE CagedHeap final {
     return OffsetFromAddress(address) < kCagedHeapNormalPageReservationSize;
   }
 
+  static void InitializeIfNeeded(PageAllocator&);
+
   static CagedHeap& Instance();
 
   CagedHeap(const CagedHeap&) = delete;
@@ -86,11 +88,8 @@ class V8_EXPORT_PRIVATE CagedHeap final {
 
  private:
   friend class v8::base::LeakyObject<CagedHeap>;
-  friend class HeapBase;
   friend class testing::TestWithHeap;
 
-  static void InitializeIfNeeded(PageAllocator&);
-
   explicit CagedHeap(PageAllocator& platform_allocator);
 
   void ResetForTesting();

src/heap/cppgc/gc-info-table.cc

@@ -29,18 +29,6 @@ static_assert(v8::base::bits::IsPowerOfTwo(kEntrySize),
               "GCInfoTable entries size must be power of "
               "two");
 
-PageAllocator* GetAllocator(PageAllocator* page_allocator) {
-  if (!page_allocator) {
-    static v8::base::LeakyObject<v8::base::PageAllocator>
-        default_page_allocator;
-    page_allocator = default_page_allocator.get();
-  }
-  // No need to introduce LSAN support for PageAllocator, as `GCInfoTable` is
-  // already a leaky object and the table payload (`GCInfoTable::table_`) should
-  // not refer to dynamically allocated objects.
-  return page_allocator;
-}
-
 }  // namespace
 
 GCInfoTable* GlobalGCInfoTable::global_table_ = nullptr;
@@ -50,7 +38,8 @@ constexpr GCInfoIndex GCInfoTable::kInitialWantedLimit;
 
 // static
 void GlobalGCInfoTable::Initialize(PageAllocator* page_allocator) {
-  static v8::base::LeakyObject<GCInfoTable> table(GetAllocator(page_allocator));
+  DCHECK_NOT_NULL(page_allocator);
+  static v8::base::LeakyObject<GCInfoTable> table(page_allocator);
   if (!global_table_) {
     global_table_ = table.get();
   } else {

src/heap/cppgc/heap-base.cc

@@ -145,7 +145,6 @@ size_t HeapBase::ObjectPayloadSize() const {
 std::unique_ptr<PageBackend> HeapBase::InitializePageBackend(
     PageAllocator& allocator, FatalOutOfMemoryHandler& oom_handler) {
 #if defined(CPPGC_CAGED_HEAP)
-  CagedHeap::InitializeIfNeeded(allocator);
   auto& caged_heap = CagedHeap::Instance();
   return std::make_unique<PageBackend>(caged_heap.normal_page_allocator(),
                                        caged_heap.large_page_allocator(),

src/heap/cppgc/platform.cc

@@ -7,12 +7,18 @@
 #include "src/base/lazy-instance.h"
 #include "src/base/logging.h"
 #include "src/base/macros.h"
+#include "src/base/page-allocator.h"
 #include "src/base/platform/platform.h"
 #include "src/base/sanitizer/asan.h"
+#include "src/base/sanitizer/lsan-page-allocator.h"
 #include "src/heap/cppgc/gc-info-table.h"
 #include "src/heap/cppgc/globals.h"
 #include "src/heap/cppgc/platform.h"
 
+#if defined(CPPGC_CAGED_HEAP)
+#include "src/heap/cppgc/caged-heap.h"
+#endif  // defined(CPPGC_CAGED_HEAP)
+
 namespace cppgc {
 namespace internal {
 
@@ -46,6 +52,23 @@ void FatalOutOfMemoryHandler::SetCustomHandler(Callback* callback) {
 
 namespace {
 PageAllocator* g_page_allocator = nullptr;
+
+PageAllocator& GetAllocator(PageAllocator* page_allocator) {
+  if (!page_allocator) {
+    static v8::base::LeakyObject<v8::base::PageAllocator>
+        default_page_allocator;
+    page_allocator = default_page_allocator.get();
+  }
+#if defined(LEAK_SANITIZER)
+  // If lsan is enabled, override the given allocator with the custom lsan
+  // allocator.
+  static v8::base::LeakyObject<v8::base::LsanPageAllocator> lsan_page_allocator(
+      page_allocator);
+  page_allocator = lsan_page_allocator.get();
+#endif  // LEAK_SANITIZER
+  return *page_allocator;
+}
+
 }  // namespace
 
 TracingController* Platform::GetTracingController() {
@@ -65,9 +88,14 @@ void InitializeProcess(PageAllocator* page_allocator) {
   CHECK_EQ(0u, internal::kAllocationGranularity % poisoning_granularity);
 #endif
 
+  auto& allocator = GetAllocator(page_allocator);
+
   CHECK(!g_page_allocator);
-  internal::GlobalGCInfoTable::Initialize(page_allocator);
-  g_page_allocator = page_allocator;
+  internal::GlobalGCInfoTable::Initialize(&allocator);
+#if defined(CPPGC_CAGED_HEAP)
+  internal::CagedHeap::InitializeIfNeeded(allocator);
+#endif  // defined(CPPGC_CAGED_HEAP)
+  g_page_allocator = &allocator;
 }
 
 void ShutdownProcess() { g_page_allocator = nullptr; }