From 5d8fc68ff55c002566acc9852f491ef60639d203 Mon Sep 17 00:00:00 2001
From: Gigi Joseph <[email protected]>
Date: Tue, 21 Jul 2026 07:14:46 -0700
Subject: [PATCH] [floss] Keep BluetoothSocketFloss alive across listen callbacks

DoConnectionStateChanged is dispatched through a WeakPtr-bound
RepeatingCallback, so the call stack does not hold a strong reference to
the socket. After CompleteListen, the only scoped_refptr to the socket
may be the one bound inside pending_listen_ready_callback_ itself.
Running that callback can therefore release the last reference before
the rest of the method writes is_accepting_ and issues the Accept call.

Hold a local scoped_refptr for the duration of the method so the object
is guaranteed to outlive the listen-ready, listen-close, and
accept-error callbacks it invokes. Add a unit test that exercises a
listen-ready callback which drops its socket reference.

Bug: b:516947491
Test: out/cros_asan/device_unittests --gtest_filter='BluetoothSocketFlossTest.ListenReadyCallbackDropsLastRef'
Change-Id: I90ab7ae2bb01687d94a863e4f61bdf067fc20c78
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/8114226
Reviewed-by: Hsin-chen Chuang <[email protected]>
Commit-Queue: Gigi Joseph (xWF) <[email protected]>
Reviewed-by: Michael Sun <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1665454}
---

diff --git a/device/bluetooth/floss/bluetooth_socket_floss.cc b/device/bluetooth/floss/bluetooth_socket_floss.cc
index 376a8157..d29dcc5 100644
--- a/device/bluetooth/floss/bluetooth_socket_floss.cc
+++ b/device/bluetooth/floss/bluetooth_socket_floss.cc
@@ -211,6 +211,10 @@
     FlossSocketManager::ServerSocketState state,
     FlossSocketManager::FlossListeningSocket socket,
     FlossDBusClient::BtifStatus status) {
+  // The callbacks invoked below may hold the last reference to |this|, so keep
+  // a local reference for the duration of the method.
+  scoped_refptr<BluetoothSocketFloss> self(this);
+
   // If we don't already have socket info, store it.
   if (!listening_socket_info_) {
     listening_socket_info_ = socket;
diff --git a/device/bluetooth/floss/bluetooth_socket_floss_unittest.cc b/device/bluetooth/floss/bluetooth_socket_floss_unittest.cc
index c1205a3..987f8cbf 100644
--- a/device/bluetooth/floss/bluetooth_socket_floss_unittest.cc
+++ b/device/bluetooth/floss/bluetooth_socket_floss_unittest.cc
@@ -371,4 +371,48 @@
   DisconnectSocket(server_socket.get());
 }
 
+// Regression test for a use-after-free in DoConnectionStateChanged.
+//
+// After CompleteListen(), the sole strong reference to the BluetoothSocketFloss
+// is held by its own |pending_listen_ready_callback_| member.
+// DoConnectionStateChanged is dispatched via a WeakPtr-bound RepeatingCallback
+// (no strong ref on the stack) and synchronously runs that callback. If the
+// callee drops the scoped_refptr without retaining it (which happens in
+// production when chrome.bluetoothSocket.close() removed the BluetoothApiSocket
+// before listen completed), |this| is freed mid-method and the subsequent
+// member writes/reads are use-after-free.
+TEST_F(BluetoothSocketFlossTest, ListenReadyCallbackDropsLastRef) {
+  FlossSocketManager::SocketId id = GetFakeFlossSocketManager()->GetNextId();
+
+  // Simulates BluetoothSocketListenFunction::OnCreateService when
+  // GetSocket(socket_id()) returns nullptr: the scoped_refptr parameter is
+  // destroyed without being retained anywhere.
+  auto drop_last_ref = [](scoped_refptr<device::BluetoothSocket> socket) {
+    // |socket| destructs here. Without a self-ref in DoConnectionStateChanged
+    // this is the last reference -> refcount hits 0 -> ~BluetoothSocketFloss().
+  };
+
+  adapter_->CreateRfcommService(
+      device::BluetoothUUID(FakeFlossSocketManager::kRfcommUuid),
+      BluetoothAdapter::ServiceOptions(),
+      base::BindLambdaForTesting(drop_last_ref),
+      base::BindLambdaForTesting(
+          [](const std::string& msg) { FAIL() << msg; }));
+
+  // At this point CompleteListen has run synchronously (FakeFlossSocketManager
+  // invokes the response callback inline), so the only owner of the
+  // BluetoothSocketFloss is its own pending_listen_ready_callback_ member.
+  //
+  // SendSocketReady dispatches DoConnectionStateChanged via the WeakPtr-bound
+  // ConnectionStateChanged callback. Inside, pending_listen_ready_callback_ is
+  // run, |drop_last_ref| destroys the last scoped_refptr, the object is freed,
+  // and DoConnectionStateChanged then writes is_accepting_ and dereferences
+  // weak_ptr_factory_ on freed heap. ASan reports heap-use-after-free here.
+  GetFakeFlossSocketManager()->SendSocketReady(
+      id, device::BluetoothUUID(FakeFlossSocketManager::kRfcommUuid),
+      FlossDBusClient::BtifStatus::kSuccess);
+
+  base::RunLoop().RunUntilIdle();
+}
+
 }  // namespace floss

diff --git a/device/bluetooth/floss/bluetooth_socket_floss_unittest.cc b/device/bluetooth/floss/bluetooth_socket_floss_unittest.cc
index c1205a3..987f8cbf 100644
--- a/device/bluetooth/floss/bluetooth_socket_floss_unittest.cc
+++ b/device/bluetooth/floss/bluetooth_socket_floss_unittest.cc
@@ -371,4 +371,48 @@
   DisconnectSocket(server_socket.get());
 }
 
+// Regression test for a use-after-free in DoConnectionStateChanged.
+//
+// After CompleteListen(), the sole strong reference to the BluetoothSocketFloss
+// is held by its own |pending_listen_ready_callback_| member.
+// DoConnectionStateChanged is dispatched via a WeakPtr-bound RepeatingCallback
+// (no strong ref on the stack) and synchronously runs that callback. If the
+// callee drops the scoped_refptr without retaining it (which happens in
+// production when chrome.bluetoothSocket.close() removed the BluetoothApiSocket
+// before listen completed), |this| is freed mid-method and the subsequent
+// member writes/reads are use-after-free.
+TEST_F(BluetoothSocketFlossTest, ListenReadyCallbackDropsLastRef) {
+  FlossSocketManager::SocketId id = GetFakeFlossSocketManager()->GetNextId();
+
+  // Simulates BluetoothSocketListenFunction::OnCreateService when
+  // GetSocket(socket_id()) returns nullptr: the scoped_refptr parameter is
+  // destroyed without being retained anywhere.
+  auto drop_last_ref = [](scoped_refptr<device::BluetoothSocket> socket) {
+    // |socket| destructs here. Without a self-ref in DoConnectionStateChanged
+    // this is the last reference -> refcount hits 0 -> ~BluetoothSocketFloss().
+  };
+
+  adapter_->CreateRfcommService(
+      device::BluetoothUUID(FakeFlossSocketManager::kRfcommUuid),
+      BluetoothAdapter::ServiceOptions(),
+      base::BindLambdaForTesting(drop_last_ref),
+      base::BindLambdaForTesting(
+          [](const std::string& msg) { FAIL() << msg; }));
+
+  // At this point CompleteListen has run synchronously (FakeFlossSocketManager
+  // invokes the response callback inline), so the only owner of the
+  // BluetoothSocketFloss is its own pending_listen_ready_callback_ member.
+  //
+  // SendSocketReady dispatches DoConnectionStateChanged via the WeakPtr-bound
+  // ConnectionStateChanged callback. Inside, pending_listen_ready_callback_ is
+  // run, |drop_last_ref| destroys the last scoped_refptr, the object is freed,
+  // and DoConnectionStateChanged then writes is_accepting_ and dereferences
+  // weak_ptr_factory_ on freed heap. ASan reports heap-use-after-free here.
+  GetFakeFlossSocketManager()->SendSocketReady(
+      id, device::BluetoothUUID(FakeFlossSocketManager::kRfcommUuid),
+      FlossDBusClient::BtifStatus::kSuccess);
+
+  base::RunLoop().RunUntilIdle();
+}
+
 }  // namespace floss