[sandbox] Always initialize allocated external pointer handles

Otherwise allocated external pointer handles may be swept if never set
by the caller.

Bug: v8:10391
Change-Id: I3d727b80635ac8e21bd403de6bcad59091ed80a0
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/3832528Reviewed-by: Samuel Groß <saelo@chromium.org>
Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Shu-yu Guo <syg@chromium.org>
Cr-Commit-Position: refs/heads/main@{#82597}

src/execution/isolate.cc

@@ -5850,12 +5850,13 @@ Isolate::InsertWaiterQueueNodeIntoSharedExternalPointerTable(Address node) {
   if (waiter_queue_node_external_pointer_handle_ !=
       kNullExternalPointerHandle) {
     handle = waiter_queue_node_external_pointer_handle_;
+    shared_external_pointer_table().Set(handle, node, kWaiterQueueNodeTag);
   } else {
-    handle = shared_external_pointer_table().AllocateEntry();
+    handle = shared_external_pointer_table().AllocateAndInitializeEntry(
+        node, kWaiterQueueNodeTag);
     waiter_queue_node_external_pointer_handle_ = handle;
   }
   DCHECK_NE(0, handle);
-  shared_external_pointer_table().Set(handle, node, kWaiterQueueNodeTag);
   return handle;
 }
 #endif  // V8_COMPRESS_POINTERS

src/objects/slots-inl.h

@@ -159,8 +159,7 @@ void ExternalPointerSlot::init(Isolate* isolate, Address value,
 #ifdef V8_ENABLE_SANDBOX
   if (IsSandboxedExternalPointerType(tag)) {
     ExternalPointerTable& table = GetExternalPointerTableForTag(isolate, tag);
-    ExternalPointerHandle handle = table.AllocateEntry();
-    table.Set(handle, value, tag);
+    ExternalPointerHandle handle = table.AllocateAndInitializeEntry(value, tag);
     // Use a Release_Store to ensure that the store of the pointer into the
     // table is not reordered after the store of the handle. Otherwise, other
     // threads may access an uninitialized table entry and crash.

src/sandbox/external-pointer-inl.h

@@ -36,8 +36,7 @@ V8_INLINE void InitExternalPointerField(Address field_address, Isolate* isolate,
 #ifdef V8_ENABLE_SANDBOX
   if (IsSandboxedExternalPointerType(tag)) {
     ExternalPointerTable& table = GetExternalPointerTable<tag>(isolate);
-    ExternalPointerHandle handle = table.AllocateEntry();
-    table.Set(handle, value, tag);
+    ExternalPointerHandle handle = table.AllocateAndInitializeEntry(value, tag);
     // Use a Release_Store to ensure that the store of the pointer into the
     // table is not reordered after the store of the handle. Otherwise, other
     // threads may access an uninitialized table entry and crash.

src/sandbox/external-pointer-table-inl.h

@@ -165,9 +165,12 @@ ExternalPointerHandle ExternalPointerTable::AllocateInternal(
   return index_to_handle(index);
 }
 
-ExternalPointerHandle ExternalPointerTable::AllocateEntry() {
+ExternalPointerHandle ExternalPointerTable::AllocateAndInitializeEntry(
+    Address initial_value, ExternalPointerTag tag) {
   constexpr bool is_evacuation_entry = false;
-  return AllocateInternal(is_evacuation_entry);
+  ExternalPointerHandle handle = AllocateInternal(is_evacuation_entry);
+  Set(handle, initial_value, tag);
+  return handle;
 }
 
 ExternalPointerHandle ExternalPointerTable::AllocateEvacuationEntry() {

src/sandbox/external-pointer-table.h

@@ -103,11 +103,11 @@ class V8_EXPORT_PRIVATE ExternalPointerTable {
                           ExternalPointerTag tag);
 
   // Allocates a new entry in the external pointer table. The caller must
-  // initialize the entry afterwards through set(). In particular, the caller is
-  // responsible for setting the mark bit of the new entry.
+  // provide the initial value and tag.
   //
   // This method is atomic and can be called from background threads.
-  inline ExternalPointerHandle AllocateEntry();
+  inline ExternalPointerHandle AllocateAndInitializeEntry(
+      Address initial_value, ExternalPointerTag tag);
 
   // Determines the number of entries currently on the freelist.
   // The freelist entries encode the freelist size and the next entry on the
@@ -240,14 +240,18 @@ class V8_EXPORT_PRIVATE ExternalPointerTable {
     base::Relaxed_Store(&capacity_, new_capacity);
   }
 
-  // Implementation of entry allocation. Called from AllocateEntry and
-  // AllocateEvacuationEntry.
+  // Implementation of entry allocation. Called from AllocateAndInitializeEntry
+  // and AllocateEvacuationEntry.
   //
   // If this method is used to allocate an evacuation entry, it is guaranteed to
   // return an entry before the start of the evacuation area or fail by
   // returning kNullExternalPointerHandle. In particular, it will never grow the
   // table. See the explanation of the compaction algorithm for more details.
   //
+  // The caller must initialize the entry afterwards through Set(). In
+  // particular, the caller is responsible for setting the mark bit of the new
+  // entry.
+  //
   // This method is atomic and can be called from background threads.
   inline ExternalPointerHandle AllocateInternal(bool is_evacuation_entry);