From 5a79228e0ac524c389cfd2c216d47d38ae3adae2 Mon Sep 17 00:00:00 2001
From: Omer Katz <[email protected]>
Date: Wed, 09 Sep 2026 15:12:11 +0200
Subject: [PATCH] [sandbox] Avoid double read of JSArrayBuffer::extension_

Bug: 554619028
Change-Id: Id2e89d46581acddba586286ab3cb1980bb617019
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/8365887
Reviewed-by: Maksim Ivanov <[email protected]>
Reviewed-by: Michael Lippautz <[email protected]>
Commit-Queue: Omer Katz <[email protected]>
Cr-Commit-Position: refs/heads/main@{#109748}
---
diff --git a/src/base/atomic-utils.h b/src/base/atomic-utils.h
index 3605230..4959538 100644
--- a/src/base/atomic-utils.h
+++ b/src/base/atomic-utils.h
@@ -112,8 +112,15 @@
   template <typename T>
   static T SeqCst_Swap(T* addr, std::remove_reference_t<T> new_value) {
     static_assert(sizeof(T) <= sizeof(AtomicStorageType));
-    return base::SeqCst_AtomicExchange(
-        to_storage_addr(addr), cast_helper<T>::to_storage_type(new_value));
+    return cast_helper<T>::to_return_type(base::SeqCst_AtomicExchange(
+        to_storage_addr(addr), cast_helper<T>::to_storage_type(new_value)));
+  }
+
+  template <typename T>
+  static T Relaxed_Swap(T* addr, std::remove_reference_t<T> new_value) {
+    static_assert(sizeof(T) <= sizeof(AtomicStorageType));
+    return cast_helper<T>::to_return_type(base::Relaxed_AtomicExchange(
+        to_storage_addr(addr), cast_helper<T>::to_storage_type(new_value)));
   }
 
   template <typename T>
diff --git a/src/objects/js-array-buffer-inl.h b/src/objects/js-array-buffer-inl.h
index 5e90240..79c39f0 100644
--- a/src/objects/js-array-buffer-inl.h
+++ b/src/objects/js-array-buffer-inl.h
@@ -169,6 +169,23 @@
   WriteBarrier::ForArrayBufferExtension(this, extension);
 }
 
+ArrayBufferExtension* JSArrayBuffer::extract_extension(
+    Isolate* isolate,
+    const DisallowGarbageCollection& disallow_gc V8_LIFETIME_BOUND) {
+#if V8_COMPRESS_POINTERS
+  ExternalPointerHandle handle =
+      base::AsAtomic32::Relaxed_Load(extension_handle_location());
+  if (handle == kNullExternalPointerHandle) {
+    return nullptr;
+  }
+  return reinterpret_cast<ArrayBufferExtension*>(
+      isolate->external_pointer_table().Exchange(handle, kNullAddress,
+                                                 kArrayBufferExtensionTag));
+#else
+  return base::AsAtomicPointer::Relaxed_Swap(extension_location(), nullptr);
+#endif  // V8_COMPRESS_POINTERS
+}
+
 #if V8_COMPRESS_POINTERS
 ExternalPointerHandle* JSArrayBuffer::extension_handle_location() const {
   return reinterpret_cast<ExternalPointerHandle*>(extension_.storage_address());
diff --git a/src/objects/js-array-buffer.cc b/src/objects/js-array-buffer.cc
index 9c10ef1..5c44f10 100644
--- a/src/objects/js-array-buffer.cc
+++ b/src/objects/js-array-buffer.cc
@@ -217,14 +217,19 @@
 void JSArrayBuffer::DetachInternal(DirectHandle<JSArrayBuffer> array_buffer,
                                    bool force_for_wasm_memory,
                                    Isolate* isolate) {
-  ArrayBufferExtension* extension = array_buffer->extension();
-
-  if (extension) {
+  {
     DisallowGarbageCollection disallow_gc;
-    isolate->heap()->DetachArrayBufferExtension(extension);
-    std::shared_ptr<BackingStore> backing_store =
-        array_buffer->RemoveExtension();
-    CHECK_IMPLIES(force_for_wasm_memory, backing_store->is_wasm_memory());
+    ArrayBufferExtension* extension =
+        array_buffer->extract_extension(isolate, disallow_gc);
+    if (extension) {
+      isolate->heap()->DetachArrayBufferExtension(extension);
+      // Prevent concurrent detachment vs background copying if an attacker
+      // swapped the extension pointer to a shared buffer's extension.
+      SBXCHECK(!extension->is_shared());
+      std::shared_ptr<BackingStore> backing_store =
+          extension->RemoveBackingStore();
+      CHECK_IMPLIES(force_for_wasm_memory, backing_store->is_wasm_memory());
+    }
   }
 
   array_buffer->set_was_detached(true, kReleaseStore);
@@ -331,19 +336,6 @@
   return extension;
 }
 
-std::shared_ptr<BackingStore> JSArrayBuffer::RemoveExtension() {
-  ArrayBufferExtension* extension = this->extension();
-  DCHECK_NOT_NULL(extension);
-  // Prevent concurrent detachment vs background copying if an attacker
-  // swapped the extension pointer to a shared buffer's extension.
-  SBXCHECK(!extension->is_shared());
-  auto result = extension->RemoveBackingStore();
-  // Remove pointer to extension such that the next GC will free it
-  // automatically.
-  set_extension(nullptr);
-  return result;
-}
-
 Handle<JSArrayBuffer> JSTypedArray::GetBuffer(Isolate* isolate) {
   DirectHandle<JSTypedArray> self(this, isolate);
   DCHECK(IsTypedArrayOrRabGsabTypedArrayElementsKind(self->GetElementsKind()));
diff --git a/src/objects/js-array-buffer.h b/src/objects/js-array-buffer.h
index ec73e6f..0376f9f 100644
--- a/src/objects/js-array-buffer.h
+++ b/src/objects/js-array-buffer.h
@@ -56,6 +56,12 @@
   inline ArrayBufferExtension* extension() const;
   inline void set_extension(ArrayBufferExtension* value);
   inline void init_extension();
+  // Returns the current extension and resets it to nullptr. Expects a witness
+  // DisallowGarbageCollection to make sure the returned extension isn't freed
+  // while a pointer to it is held on stack.
+  inline ArrayBufferExtension* extract_extension(
+      Isolate* isolate,
+      const DisallowGarbageCollection& disallow_gc V8_LIFETIME_BOUND);
 
   // [bit_field]: boolean flags
   inline uint32_t bit_field() const;
@@ -183,9 +189,6 @@
   V8_EXPORT_PRIVATE ArrayBufferExtension* CreateExtension(
       Isolate* isolate, std::shared_ptr<BackingStore> backing_store);
 
-  // Frees the associated ArrayBufferExtension and returns its backing store.
-  std::shared_ptr<BackingStore> RemoveExtension();
-
   //
   // Serializer/deserializer support.
   //
diff --git a/src/sandbox/external-pointer-table-inl.h b/src/sandbox/external-pointer-table-inl.h
index b718e5d..a6ba845 100644
--- a/src/sandbox/external-pointer-table-inl.h
+++ b/src/sandbox/external-pointer-table-inl.h
@@ -210,8 +210,9 @@
 Address ExternalPointerTable::Exchange(ExternalPointerHandle handle,
                                        Address value, ExternalPointerTag tag) {
   DCHECK_NE(kNullExternalPointerHandle, handle);
-  DCHECK(!IsManagedExternalPointerType(tag));
   uint32_t index = HandleToIndex(handle);
+  FreeManagedResourceIfPresent(index);
+  TakeOwnershipOfManagedResourceIfNecessary(value, handle, tag);
   return at(index).ExchangeExternalPointer(value, tag);
 }