From 68159aa869246c82bfc7883e6ca7dd2b164e7abc Mon Sep 17 00:00:00 2001
From: Michael Lippautz <[email protected]>
Date: Tue, 19 May 2026 05:33:39 -0700
Subject: [PATCH] heap: Make range-based write barrier more robust

Fix bailouts on the barrier for possible nullptr cases.

CONV=47326a54-47d2-449d-b308-f51eb6b5b3b8
TAG=AGY

Fixed: 513737952
Change-Id: Iaa8598bc06a1782f77af7a3ea02359d918673688
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7852268
Reviewed-by: Anton Bikineev <[email protected]>
Commit-Queue: Michael Lippautz <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1632789}
---

diff --git a/third_party/blink/renderer/platform/heap/member.h b/third_party/blink/renderer/platform/heap/member.h
index e39f2e1..30f07ac 100644
--- a/third_party/blink/renderer/platform/heap/member.h
+++ b/third_party/blink/renderer/platform/heap/member.h
@@ -230,14 +230,27 @@
   }
 
   static void NotifyNewElements(base::span<T> members) {
-    // Checking the first element is sufficient for determining whether a
-    // marking or generational barrier is required.
-    if (members.empty() ||
-        !WriteBarrier::IsWriteBarrierNeeded(&members.front())) [[likely]] {
+    // TODO(mlippautz): We can expose whether the write barrier is enabled at
+    // all and get away with a single global variable check here.
+
+    // We need a non-null pointer to check if the write barrier is needed.
+    // nullptr will just always bail out.
+    auto current = members.begin();
+    for (; current != members.end(); ++current) {
+      if (static_cast<bool>(*current)) {
+        break;
+      }
+    }
+    if (current == members.end()) {
       return;
     }
-    for (auto& member : members) {
-      WriteBarrier::DispatchForObject(&member);
+    // Checking one pointer is sufficient for determining whether a
+    // marking or generational barrier is required.
+    if (!WriteBarrier::IsWriteBarrierNeeded(&*current)) {
+      return;
+    }
+    for (; current != members.end(); ++current) {
+      WriteBarrier::DispatchForObject(&*current);
     }
   }
 };
diff --git a/third_party/blink/renderer/platform/heap/test/incremental_marking_test.cc b/third_party/blink/renderer/platform/heap/test/incremental_marking_test.cc
index c83417b..b74ffd44 100644
--- a/third_party/blink/renderer/platform/heap/test/incremental_marking_test.cc
+++ b/third_party/blink/renderer/platform/heap/test/incremental_marking_test.cc
@@ -1554,5 +1554,59 @@
   driver.FinishGC();
 }
 
+TEST_F(IncrementalMarkingTest, HeapVectorCopyMemberNullFirstSlotUAF) {
+  // Regression test: https://crbug.com/513737952
+  using Vec = GCedHeapVector<Member<LinkedObject>>;
+  Persistent<Vec> dst = MakeGarbageCollected<Vec>();
+  dst->reserve(4);
+  Vec* src = MakeGarbageCollected<Vec>();
+  src->push_back(nullptr);
+  src->push_back(MakeGarbageCollected<LinkedObject>());
+  WeakPersistent<LinkedObject> victim_observer = src->at(1).Get();
+  IncrementalMarkingTestDriver driver(ThreadState::Current());
+  driver.StartGC();
+  driver.TriggerMarkingSteps();
+  // No reallocation. This should just trigger a write barrier.
+  *dst = *src;
+  driver.FinishGC();
+  EXPECT_TRUE(victim_observer);
+}
+
+TEST_F(IncrementalMarkingTest, HeapVectorCopyMemberNonNullFirstSlotControl) {
+  // Regression test: https://crbug.com/513737952
+  using Vec = GCedHeapVector<Member<LinkedObject>>;
+  Persistent<Vec> dst = MakeGarbageCollected<Vec>();
+  dst->reserve(4);
+  Vec* src = MakeGarbageCollected<Vec>();
+  src->push_back(MakeGarbageCollected<LinkedObject>());
+  src->push_back(nullptr);
+  WeakPersistent<LinkedObject> victim_observer = src->at(0).Get();
+  IncrementalMarkingTestDriver driver(ThreadState::Current());
+  driver.StartGC();
+  driver.TriggerMarkingSteps();
+  *dst = *src;
+  driver.FinishGC();
+  EXPECT_TRUE(victim_observer);
+}
+
+TEST_F(IncrementalMarkingTest, HeapVectorAppendRangeNullFirstSlotUAF) {
+  // Regression test: https://crbug.com/513737952
+  using Vec = GCedHeapVector<Member<LinkedObject>>;
+  Persistent<Vec> dst = MakeGarbageCollected<Vec>();
+  dst->reserve(4);
+  Vec* src = MakeGarbageCollected<Vec>();
+  src->push_back(nullptr);
+  src->push_back(MakeGarbageCollected<LinkedObject>());
+  WeakPersistent<LinkedObject> victim_observer = src->at(1).Get();
+  IncrementalMarkingTestDriver driver(ThreadState::Current());
+  driver.StartGC();
+  driver.TriggerMarkingSteps();
+  // The following call performs no reallocation. Appending the range must
+  // trigger write barriers.
+  dst->append_range(*src);
+  driver.FinishGC();
+  EXPECT_TRUE(victim_observer);
+}
+
 }  // namespace incremental_marking_test
 }  // namespace blink

diff --git a/third_party/blink/renderer/platform/heap/test/incremental_marking_test.cc b/third_party/blink/renderer/platform/heap/test/incremental_marking_test.cc
index c83417b..b74ffd44 100644
--- a/third_party/blink/renderer/platform/heap/test/incremental_marking_test.cc
+++ b/third_party/blink/renderer/platform/heap/test/incremental_marking_test.cc
@@ -1554,5 +1554,59 @@
   driver.FinishGC();
 }
 
+TEST_F(IncrementalMarkingTest, HeapVectorCopyMemberNullFirstSlotUAF) {
+  // Regression test: https://crbug.com/513737952
+  using Vec = GCedHeapVector<Member<LinkedObject>>;
+  Persistent<Vec> dst = MakeGarbageCollected<Vec>();
+  dst->reserve(4);
+  Vec* src = MakeGarbageCollected<Vec>();
+  src->push_back(nullptr);
+  src->push_back(MakeGarbageCollected<LinkedObject>());
+  WeakPersistent<LinkedObject> victim_observer = src->at(1).Get();
+  IncrementalMarkingTestDriver driver(ThreadState::Current());
+  driver.StartGC();
+  driver.TriggerMarkingSteps();
+  // No reallocation. This should just trigger a write barrier.
+  *dst = *src;
+  driver.FinishGC();
+  EXPECT_TRUE(victim_observer);
+}
+
+TEST_F(IncrementalMarkingTest, HeapVectorCopyMemberNonNullFirstSlotControl) {
+  // Regression test: https://crbug.com/513737952
+  using Vec = GCedHeapVector<Member<LinkedObject>>;
+  Persistent<Vec> dst = MakeGarbageCollected<Vec>();
+  dst->reserve(4);
+  Vec* src = MakeGarbageCollected<Vec>();
+  src->push_back(MakeGarbageCollected<LinkedObject>());
+  src->push_back(nullptr);
+  WeakPersistent<LinkedObject> victim_observer = src->at(0).Get();
+  IncrementalMarkingTestDriver driver(ThreadState::Current());
+  driver.StartGC();
+  driver.TriggerMarkingSteps();
+  *dst = *src;
+  driver.FinishGC();
+  EXPECT_TRUE(victim_observer);
+}
+
+TEST_F(IncrementalMarkingTest, HeapVectorAppendRangeNullFirstSlotUAF) {
+  // Regression test: https://crbug.com/513737952
+  using Vec = GCedHeapVector<Member<LinkedObject>>;
+  Persistent<Vec> dst = MakeGarbageCollected<Vec>();
+  dst->reserve(4);
+  Vec* src = MakeGarbageCollected<Vec>();
+  src->push_back(nullptr);
+  src->push_back(MakeGarbageCollected<LinkedObject>());
+  WeakPersistent<LinkedObject> victim_observer = src->at(1).Get();
+  IncrementalMarkingTestDriver driver(ThreadState::Current());
+  driver.StartGC();
+  driver.TriggerMarkingSteps();
+  // The following call performs no reallocation. Appending the range must
+  // trigger write barriers.
+  dst->append_range(*src);
+  driver.FinishGC();
+  EXPECT_TRUE(victim_observer);
+}
+
 }  // namespace incremental_marking_test
 }  // namespace blink