From ba7af815c81c3a2cc634c2516adbd5c5cff109a5 Mon Sep 17 00:00:00 2001
From: Alvin Ji <[email protected]>
Date: Fri, 15 May 2026 13:37:41 -0700
Subject: [PATCH] bluetooth: Fix Use-After-Free in BluetoothSocketMac::Send

IOBluetooth writeAsync callbacks can fire synchronously on macOS,
causing premature request cleanup and UAFs if pending write counters are
incremented after the call.

This CL pre-calculates total chunks and initializes active_async_writes
upfront to ensure safe lifetime management across multi-chunk writes.

Bug: 499007266
Change-Id: I7bb48661e3b2ce72e3f9457c4be07b66a027b845
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/7833477
Reviewed-by: Matt Reynolds <[email protected]>
Commit-Queue: Alvin Ji <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1631527}
---

diff --git a/device/bluetooth/bluetooth_socket_mac.mm b/device/bluetooth/bluetooth_socket_mac.mm
index a303bc0..60b5df97 100644
--- a/device/bluetooth/bluetooth_socket_mac.mm
+++ b/device/bluetooth/bluetooth_socket_mac.mm
@@ -34,6 +34,7 @@
 #include "device/bluetooth/bluetooth_device.h"
 #include "device/bluetooth/bluetooth_l2cap_channel_mac.h"
 #include "device/bluetooth/bluetooth_rfcomm_channel_mac.h"
+#include "device/bluetooth/public/cpp/bluetooth_features.h"
 #include "net/base/io_buffer.h"
 #include "net/base/net_errors.h"
 
@@ -773,33 +774,86 @@
   uint16_t mtu = channel_->GetOutgoingMTU();
   auto send_buffer =
       base::MakeRefCounted<net::DrainableIOBuffer>(buffer.get(), buffer_size);
-  while (send_buffer->BytesRemaining() > 0) {
-    int byte_count = send_buffer->BytesRemaining();
-    if (byte_count > mtu)
-      byte_count = mtu;
-    IOReturn status =
-        channel_->WriteAsync(send_buffer->data(), byte_count, request_ptr);
 
-    if (status != kIOReturnSuccess) {
-      std::stringstream error;
-      error << "Failed to connect bluetooth socket ("
-            << channel_->GetDeviceAddress() << "): (" << status << ")";
-      // Remember the first error only
-      if (request_ptr->status == kIOReturnSuccess)
-        request_ptr->status = status;
-      request_ptr->error_signaled = true;
-      std::move(request_ptr->error_callback).Run(error.str());
-      // We may have failed to issue any write operation. In that case, there
-      // will be no corresponding completion callback for this particular
-      // request, so we must forget about it now.
-      if (request_ptr->active_async_writes == 0) {
-        send_queue_.pop();
+  if (base::FeatureList::IsEnabled(
+          features::kBluetoothSocketMacPreCalculateWriteChunks)) {
+    // Pre-calculate total chunks to prevent premature cleanup if callbacks fire
+    // synchronously.
+    int num_chunks = buffer_size / mtu + (buffer_size % mtu != 0 ? 1 : 0);
+    request_ptr->active_async_writes = num_chunks;
+    int chunks_remaining = num_chunks;
+
+    while (chunks_remaining > 0) {
+      int byte_count = send_buffer->BytesRemaining();
+      if (byte_count > mtu) {
+        byte_count = mtu;
       }
-      return;
-    }
 
-    request_ptr->active_async_writes++;
-    send_buffer->DidConsume(byte_count);
+      IOReturn status =
+          channel_->WriteAsync(send_buffer->data(), byte_count, request_ptr);
+
+      if (status != kIOReturnSuccess) {
+        std::stringstream error;
+        error << "Failed to connect bluetooth socket ("
+              << channel_->GetDeviceAddress() << "): (" << status << ")";
+        // Remember the first error only
+        if (request_ptr->status == kIOReturnSuccess) {
+          request_ptr->status = status;
+        }
+        request_ptr->error_signaled = true;
+        std::move(request_ptr->error_callback).Run(error.str());
+
+        // We failed to issue this write and all subsequent writes.
+        // Subtract the remaining unissued chunks rather than setting to 0
+        // directly, to avoid prematurely freeing the request if previous chunks
+        // succeeded asynchronously and are still in flight.
+        request_ptr->active_async_writes -= chunks_remaining;
+
+        // We may have failed to issue any write operation. In that case, there
+        // will be no corresponding completion callback for this particular
+        // request, so we must forget about it now.
+        if (request_ptr->active_async_writes == 0) {
+          send_queue_.pop();
+        }
+        return;
+      }
+
+      chunks_remaining--;
+      send_buffer->DidConsume(byte_count);
+    }
+  } else {
+    while (send_buffer->BytesRemaining() > 0) {
+      int byte_count = send_buffer->BytesRemaining();
+      if (byte_count > mtu) {
+        byte_count = mtu;
+      }
+
+      IOReturn status =
+          channel_->WriteAsync(send_buffer->data(), byte_count, request_ptr);
+
+      if (status != kIOReturnSuccess) {
+        std::stringstream error;
+        error << "Failed to connect bluetooth socket ("
+              << channel_->GetDeviceAddress() << "): (" << status << ")";
+        // Remember the first error only
+        if (request_ptr->status == kIOReturnSuccess) {
+          request_ptr->status = status;
+        }
+        request_ptr->error_signaled = true;
+        std::move(request_ptr->error_callback).Run(error.str());
+
+        // We may have failed to issue any write operation. In that case, there
+        // will be no corresponding completion callback for this particular
+        // request, so we must forget about it now.
+        if (request_ptr->active_async_writes == 0) {
+          send_queue_.pop();
+        }
+        return;
+      }
+
+      request_ptr->active_async_writes++;
+      send_buffer->DidConsume(byte_count);
+    }
   }
 }
 
diff --git a/device/bluetooth/public/cpp/bluetooth_features.cc b/device/bluetooth/public/cpp/bluetooth_features.cc
index 537b365..f99e54a5 100644
--- a/device/bluetooth/public/cpp/bluetooth_features.cc
+++ b/device/bluetooth/public/cpp/bluetooth_features.cc
@@ -11,4 +11,13 @@
 BASE_FEATURE(kWebBluetoothAllowGetAvailabilityWithBfcache,
              base::FEATURE_ENABLED_BY_DEFAULT);
 
+// When enabled, BluetoothSocketMac::Send pre-calculates the number of chunks
+// to write and initializes request_ptr->active_async_writes to this total
+// upfront. This prevents a Use-After-Free caused by premature cleanup if
+// completion callbacks fire synchronously during a multi-chunk write. Without
+// this, a synchronous callback would decrement the counter and could pop the
+// send_queue_ before all chunks have been issued.
+BASE_FEATURE(kBluetoothSocketMacPreCalculateWriteChunks,
+             base::FEATURE_ENABLED_BY_DEFAULT);
+
 }  // namespace features
diff --git a/device/bluetooth/public/cpp/bluetooth_features.h b/device/bluetooth/public/cpp/bluetooth_features.h
index fdae6b7..ae5e94c4 100644
--- a/device/bluetooth/public/cpp/bluetooth_features.h
+++ b/device/bluetooth/public/cpp/bluetooth_features.h
@@ -16,6 +16,9 @@
 BLUETOOTH_FEATURES_EXPORT BASE_DECLARE_FEATURE(
     kWebBluetoothAllowGetAvailabilityWithBfcache);
 
+BLUETOOTH_FEATURES_EXPORT BASE_DECLARE_FEATURE(
+    kBluetoothSocketMacPreCalculateWriteChunks);
+
 }  // namespace features
 
 #endif  // DEVICE_BLUETOOTH_PUBLIC_CPP_BLUETOOTH_FEATURES_H_