Netdev List
 help / color / mirror / Atom feed
* [PATCH 0/4] Bluetooth: hci_sync: fix TOCTOU UAF in cmd_sync callbacks
@ 2026-05-11 14:34 Michael Bommarito
  2026-05-11 14:34 ` [PATCH 1/4] Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync Michael Bommarito
                   ` (3 more replies)
  0 siblings, 4 replies; 8+ messages in thread
From: Michael Bommarito @ 2026-05-11 14:34 UTC (permalink / raw)
  Cc: Mat Martineau, netdev, stable, Pauli Virtanen, Aaron Esau,
	Michael Bommarito

Four hci_sync.c cmd_sync callbacks share a TOCTOU bug: each receives a
struct hci_conn pointer via void *data, calls hci_conn_valid() at
function entry, then dereferences the conn without holding a lifetime
reference. hci_disconn_complete_evt() running on hdev->workqueue
rx_work can call hci_conn_del() -> hci_conn_cleanup() -> put_device()
between the validity check and the body's first deref, freeing the
hci_conn slot. The cmd_sync work item then reads or writes through
the freed pointer.

The four callsites:

  net/bluetooth/hci_sync.c hci_le_create_conn_sync
  net/bluetooth/hci_sync.c hci_acl_create_conn_sync
  net/bluetooth/hci_sync.c hci_le_pa_create_sync
  net/bluetooth/hci_sync.c hci_le_big_create_sync

The fix shape used by commit 035c25007c9e ("Bluetooth: hci_sync: Fix
UAF in le_read_features_complete") and commit 0beddb0c380b
("Bluetooth: hci_conn: fix potential UAF in create_big_sync") is the
same one this series adopts: hci_conn_get() at the
cmd_sync_queue_once() call site so the conn slot stays valid for the
duration of the workqueue dispatch, with hci_conn_put() in the
completion handler.

Patch 1 introduces a small static helper hci_cmd_sync_queue_conn_once()
that centralises the get/put pair so it cannot be miscoded at each
queue site. The kerneldoc on the helper explains the -EEXIST contract.
Patches 2-4 convert the remaining three sites.

Each callback was reproduced under UML+KASAN against linux-next tip
commit bee6ea30c487 ("Add linux-next specific files for 20260421"):
all four produce a slab-use-after-free splat in cache kmalloc-8k
matching the syzbot trace cited in commit 035c25007c9e
("Bluetooth: hci_sync: Fix UAF in le_read_features_complete"). On
the patched kernel the reproducer returns -ECANCELED cleanly with
no splat.

An unprivileged process holding an AF_BLUETOOTH socket drives the
cmd_sync queue side (no CAP_* required); the racing freeing context
is hci_disconn_complete_evt() arriving via the controller, which an
adjacent attacker controlling a BLE / BR-EDR peer can drive.

Pauli Virtanen posted a series-wide variant of this fix as
https://lore.kernel.org/linux-bluetooth/e18591f264c50e15917cb8b9e5f9798d9880979d.1762100290.git.pav@iki.fi/
(PATCH v2 8/8, 2025-11-02); this series re-derives the fix on top of
current linux-next with per-site Fixes: tags and factors the get/put
pair into one helper. hci_le_past_sync is not included; the past path
uses a struct past_data wrapper, not a direct hci_conn *, so it does
not fit the same helper.

Aaron Esau's sibling fix for hci_enhanced_setup_sync() in hci_conn.c
covers the same pattern at a fifth callback,
https://lore.kernel.org/linux-bluetooth/20260330140347.906689-3-git@aaronesau.com/
(2026-03-30).

Build clean on UML+KASAN+SLUB (linux-next tip bee6ea30c487) with no
new warnings. checkpatch.pl --strict reports 0/0/0 on each patch.

Michael Bommarito (4):
  Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync
  Bluetooth: hci_sync: pin conn across hci_le_pa_create_sync
  Bluetooth: hci_sync: pin conn across hci_le_big_create_sync
  Bluetooth: hci_sync: pin conn across hci_acl_create_conn_sync

 net/bluetooth/hci_sync.c | 77 ++++++++++++++++++++++++++++++++--------
 1 file changed, 61 insertions(+), 16 deletions(-)

--
2.53.0


^ permalink raw reply	[flat|nested] 8+ messages in thread

* [PATCH 1/4] Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync
  2026-05-11 14:34 [PATCH 0/4] Bluetooth: hci_sync: fix TOCTOU UAF in cmd_sync callbacks Michael Bommarito
@ 2026-05-11 14:34 ` Michael Bommarito
  2026-05-11 14:53   ` Luiz Augusto von Dentz
  2026-05-11 14:34 ` [PATCH 2/4] Bluetooth: hci_sync: pin conn across hci_le_pa_create_sync Michael Bommarito
                   ` (2 subsequent siblings)
  3 siblings, 1 reply; 8+ messages in thread
From: Michael Bommarito @ 2026-05-11 14:34 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz, linux-bluetooth,
	linux-kernel
  Cc: Mat Martineau, netdev, stable, Pauli Virtanen, Aaron Esau,
	Michael Bommarito

hci_le_create_conn_sync() runs from the cmd_sync workqueue with a
struct hci_conn pointer it interprets out of the work item's void
*data argument. The hci_conn_valid() check at function entry is a
TOCTOU: nothing prevents hci_disconn_complete_evt() (executing on
hdev->workqueue rx_work) from running between the
hci_conn_hash_lookup walk in hci_conn_valid() and the body's first
deref. hci_disconn_complete_evt() -> hci_conn_del() -> hci_conn_cleanup()
unregisters the device and drops the final kref, which kfrees the
hci_conn slot. The cmd_sync callback then writes through the freed
pointer (clear_bit on conn->flags, conn->state, the four
le_conn_*_interval fields).

A KASAN slab-use-after-free splat in cache kmalloc-8k confirms the
bug on linux-next tip commit bee6ea30c487 ("Add linux-next specific
files for 20260421") under UML+KASAN, matching the slab geometry of
the syzbot trace fixed in commit 035c25007c9e ("Bluetooth: hci_sync:
Fix UAF in le_read_features_complete").

Follow the reference-pinning pattern from commit 035c25007c9e
("Bluetooth: hci_sync: Fix UAF in le_read_features_complete") and
commit 0beddb0c380b ("Bluetooth: hci_conn: fix potential UAF in
create_big_sync"): the queue site takes a reference via
hci_conn_get() so the slot is not freed between
hci_disconn_complete_evt() retiring the conn and the cmd_sync
callback / completion handler returning. The completion handler
drops the reference on every exit path, including the -ECANCELED
short-circuit.

Introduce a static helper hci_cmd_sync_queue_conn_once() so the
get/put pair is not open-coded at every queue site. See the
helper's kerneldoc for the -EEXIST contract.

The hci_conn_valid() check in the callback body is retained: a
logically-deleted-but-still-referenced conn has stale
hdev->conn_hash.list state, and continuing to drive a connection
attempt on it would be a logic bug even though the memory is safe.

Pauli Virtanen posted a series-wide variant of this fix as
https://lore.kernel.org/linux-bluetooth/e18591f264c50e15917cb8b9e5f9798d9880979d.1762100290.git.pav@iki.fi/
(PATCH v2 8/8, 2025-11-02). KASAN reproducer captured under
UML+KASAN (linux-next tip bee6ea30c487).

Fixes: 881559af5f5c ("Bluetooth: hci_sync: Attempt to dequeue connection attempt")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 net/bluetooth/hci_sync.c | 41 ++++++++++++++++++++++++++++++++--------
 1 file changed, 34 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index fd3aacdea512..b20e07474257 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -786,6 +786,31 @@ int hci_cmd_sync_queue_once(struct hci_dev *hdev, hci_cmd_sync_work_func_t func,
 }
 EXPORT_SYMBOL(hci_cmd_sync_queue_once);
 
+/* Queue an HCI command entry once, pinning a hci_conn for the duration.
+ *
+ * On success, the cmd_sync queue owns one hci_conn_get() reference;
+ * the supplied destroy callback must hci_conn_put() to balance.
+ *
+ * On any failure return (including -EEXIST, where
+ * hci_cmd_sync_queue_once() neither invokes destroy nor consumes the
+ * data pointer because an existing entry already owns the slot), the
+ * helper releases the reference before returning, so callers do not
+ * need to discriminate failure codes to keep the refcount balanced.
+ */
+static int hci_cmd_sync_queue_conn_once(struct hci_dev *hdev,
+					hci_cmd_sync_work_func_t func,
+					struct hci_conn *conn,
+					hci_cmd_sync_work_destroy_t destroy)
+{
+	int err;
+
+	err = hci_cmd_sync_queue_once(hdev, func, hci_conn_get(conn), destroy);
+	if (err)
+		hci_conn_put(conn);
+
+	return err;
+}
+
 /* Run HCI command:
  *
  * - hdev must be running
@@ -6982,36 +7007,38 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
 	bt_dev_dbg(hdev, "err %d", err);
 
 	if (err == -ECANCELED)
-		return;
+		goto done;
 
 	hci_dev_lock(hdev);
 
 	if (!hci_conn_valid(hdev, conn))
-		goto done;
+		goto unlock;
 
 	if (!err) {
 		hci_connect_le_scan_cleanup(conn, 0x00);
-		goto done;
+		goto unlock;
 	}
 
 	/* Check if connection is still pending */
 	if (conn != hci_lookup_le_connect(hdev))
-		goto done;
+		goto unlock;
 
 	/* Flush to make sure we send create conn cancel command if needed */
 	flush_delayed_work(&conn->le_conn_timeout);
 	hci_conn_failed(conn, bt_status(err));
 
-done:
+unlock:
 	hci_dev_unlock(hdev);
+done:
+	hci_conn_put(conn);
 }
 
 int hci_connect_le_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
 	int err;
 
-	err = hci_cmd_sync_queue_once(hdev, hci_le_create_conn_sync, conn,
-				      create_le_conn_complete);
+	err = hci_cmd_sync_queue_conn_once(hdev, hci_le_create_conn_sync, conn,
+					   create_le_conn_complete);
 	return (err == -EEXIST) ? 0 : err;
 }
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 2/4] Bluetooth: hci_sync: pin conn across hci_le_pa_create_sync
  2026-05-11 14:34 [PATCH 0/4] Bluetooth: hci_sync: fix TOCTOU UAF in cmd_sync callbacks Michael Bommarito
  2026-05-11 14:34 ` [PATCH 1/4] Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync Michael Bommarito
@ 2026-05-11 14:34 ` Michael Bommarito
  2026-05-11 14:34 ` [PATCH 3/4] Bluetooth: hci_sync: pin conn across hci_le_big_create_sync Michael Bommarito
  2026-05-11 14:34 ` [PATCH 4/4] Bluetooth: hci_sync: pin conn across hci_acl_create_conn_sync Michael Bommarito
  3 siblings, 0 replies; 8+ messages in thread
From: Michael Bommarito @ 2026-05-11 14:34 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz, linux-bluetooth,
	linux-kernel
  Cc: Mat Martineau, netdev, stable, Pauli Virtanen, Aaron Esau,
	Michael Bommarito

hci_le_pa_create_sync() exhibits the same TOCTOU pattern as
hci_le_create_conn_sync(): the cmd_sync callback receives a struct
hci_conn pointer via void *data, calls hci_conn_valid() at entry,
and then dereferences conn->sync_handle, sets a bit on conn->flags,
reads conn->dst / conn->dst_type / conn->iso_qos / conn->sid /
conn->conn_timeout, and blocks waiting for HCI_EV_LE_PA_SYNC_ESTABLISHED.
The wait can run for conn->conn_timeout milliseconds (typically
multiple seconds for periodic-advertising-sync), giving
hci_disconn_complete_evt() a wide window to retire the conn out
from under the callback.

A KASAN slab-use-after-free splat ("Read of size 2 at addr ... The
buggy address is located 52 bytes inside of freed 8192-byte
region", cache kmalloc-8k) confirms the bug on linux-next tip
commit bee6ea30c487 ("Add linux-next specific files for 20260421").
Offset 52 corresponds to conn->sync_handle.

Convert hci_connect_pa_sync() to the hci_cmd_sync_queue_conn_once()
helper introduced in the previous patch, and balance the conn pin
in create_pa_complete()'s -ECANCELED short-circuit.

Prior art: Pauli Virtanen's PATCH v2 8/8 at
https://lore.kernel.org/linux-bluetooth/e18591f264c50e15917cb8b9e5f9798d9880979d.1762100290.git.pav@iki.fi/.

Fixes: 6d0417e4e1cf ("Bluetooth: hci_conn: Fix not setting conn_timeout for Broadcast Receiver")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 net/bluetooth/hci_sync.c | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index b20e07474257..43779375209b 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -7089,7 +7089,7 @@ static void create_pa_complete(struct hci_dev *hdev, void *data, int err)
 	bt_dev_dbg(hdev, "err %d", err);
 
 	if (err == -ECANCELED)
-		return;
+		goto done;
 
 	hci_dev_lock(hdev);
 
@@ -7113,6 +7113,8 @@ static void create_pa_complete(struct hci_dev *hdev, void *data, int err)
 
 unlock:
 	hci_dev_unlock(hdev);
+done:
+	hci_conn_put(conn);
 }
 
 static int hci_le_past_params_sync(struct hci_dev *hdev, struct hci_conn *conn,
@@ -7251,8 +7253,8 @@ int hci_connect_pa_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
 	int err;
 
-	err = hci_cmd_sync_queue_once(hdev, hci_le_pa_create_sync, conn,
-				      create_pa_complete);
+	err = hci_cmd_sync_queue_conn_once(hdev, hci_le_pa_create_sync, conn,
+					   create_pa_complete);
 	return (err == -EEXIST) ? 0 : err;
 }
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 3/4] Bluetooth: hci_sync: pin conn across hci_le_big_create_sync
  2026-05-11 14:34 [PATCH 0/4] Bluetooth: hci_sync: fix TOCTOU UAF in cmd_sync callbacks Michael Bommarito
  2026-05-11 14:34 ` [PATCH 1/4] Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync Michael Bommarito
  2026-05-11 14:34 ` [PATCH 2/4] Bluetooth: hci_sync: pin conn across hci_le_pa_create_sync Michael Bommarito
@ 2026-05-11 14:34 ` Michael Bommarito
  2026-05-11 14:34 ` [PATCH 4/4] Bluetooth: hci_sync: pin conn across hci_acl_create_conn_sync Michael Bommarito
  3 siblings, 0 replies; 8+ messages in thread
From: Michael Bommarito @ 2026-05-11 14:34 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz, linux-bluetooth,
	linux-kernel
  Cc: Mat Martineau, netdev, stable, Pauli Virtanen, Aaron Esau,
	Michael Bommarito

hci_le_big_create_sync() interprets its void *data argument as a
struct hci_conn pointer and dereferences conn->iso_qos,
conn->sync_handle, conn->num_bis, conn->bis, and conn->conn_timeout
after the entry hci_conn_valid() check. As with the sibling
cmd_sync callbacks, hci_disconn_complete_evt() can retire the conn
between the validity check and the body's first deref, and the
blocking wait for HCI_EVT_LE_BIG_SYNC_ESTABLISHED extends the
race window to seconds.

A KASAN slab-use-after-free splat in cache kmalloc-8k at conn->flags
(set_bit(HCI_CONN_CREATE_BIG_SYNC, &conn->flags)) confirms the bug
on linux-next tip commit bee6ea30c487 ("Add linux-next specific
files for 20260421").

Convert hci_connect_big_sync() to the hci_cmd_sync_queue_conn_once()
helper and balance the conn pin in create_big_complete()'s
-ECANCELED short-circuit. Promote create_big_complete()'s
hci_conn_valid() + clear_bit() pair to run under hci_dev_lock so
that hci_disconn_complete_evt() cannot remove conn from
hdev->conn_hash.list between the check and the write.

Prior art: Pauli Virtanen's PATCH v2 8/8 at
https://lore.kernel.org/linux-bluetooth/e18591f264c50e15917cb8b9e5f9798d9880979d.1762100290.git.pav@iki.fi/.

Fixes: 024421cf3992 ("Bluetooth: hci_conn: Fix not setting timeout for BIG Create Sync")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 net/bluetooth/hci_sync.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 43779375209b..47ce9ba63fe2 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -7265,10 +7265,16 @@ static void create_big_complete(struct hci_dev *hdev, void *data, int err)
 	bt_dev_dbg(hdev, "err %d", err);
 
 	if (err == -ECANCELED)
-		return;
+		goto done;
+
+	hci_dev_lock(hdev);
 
 	if (hci_conn_valid(hdev, conn))
 		clear_bit(HCI_CONN_CREATE_BIG_SYNC, &conn->flags);
+
+	hci_dev_unlock(hdev);
+done:
+	hci_conn_put(conn);
 }
 
 static int hci_le_big_create_sync(struct hci_dev *hdev, void *data)
@@ -7320,8 +7326,8 @@ int hci_connect_big_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
 	int err;
 
-	err = hci_cmd_sync_queue_once(hdev, hci_le_big_create_sync, conn,
-				      create_big_complete);
+	err = hci_cmd_sync_queue_conn_once(hdev, hci_le_big_create_sync, conn,
+					   create_big_complete);
 	return (err == -EEXIST) ? 0 : err;
 }
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* [PATCH 4/4] Bluetooth: hci_sync: pin conn across hci_acl_create_conn_sync
  2026-05-11 14:34 [PATCH 0/4] Bluetooth: hci_sync: fix TOCTOU UAF in cmd_sync callbacks Michael Bommarito
                   ` (2 preceding siblings ...)
  2026-05-11 14:34 ` [PATCH 3/4] Bluetooth: hci_sync: pin conn across hci_le_big_create_sync Michael Bommarito
@ 2026-05-11 14:34 ` Michael Bommarito
  3 siblings, 0 replies; 8+ messages in thread
From: Michael Bommarito @ 2026-05-11 14:34 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz, Jonas Dreßler,
	linux-bluetooth, linux-kernel
  Cc: Mat Martineau, netdev, stable, Pauli Virtanen, Aaron Esau,
	Michael Bommarito

hci_acl_create_conn_sync() shares the TOCTOU pattern with the
three sibling cmd_sync callbacks just fixed: the work item's
void *data is interpreted as a struct hci_conn pointer, validated
with hci_conn_valid() at entry, then immediately written
(conn->state, conn->out, conn->role, conn->attempt++) followed by
a memcpy(conn->dev_class, ie->data.dev_class, 3). If the TOCTOU
race fires the memcpy lands on a freed slot; the three dev_class
bytes are sourced from a remote BR/EDR inquiry response, so a
successful exploit can land attacker-chosen bytes into the heap
object that reused conn's slot.

A KASAN slab-use-after-free splat in cache kmalloc-8k at
conn->state confirms the bug on linux-next tip commit bee6ea30c487
("Add linux-next specific files for 20260421") with the synthetic
harness driving the conn->state write.

The existing queue site at hci_connect_acl_sync() passed a NULL
destroy callback, so the conn was never pinned for the cmd_sync
workqueue dispatch. Introduce create_acl_conn_complete() to balance
the conn pin and convert the queue site to the
hci_cmd_sync_queue_conn_once() helper. The dequeue-on-cancel path
in hci_cancel_connect_sync() now looks up the entry with the same
destroy callback, keeping the hci_cmd_sync_lookup_entry() triple
match consistent.

Prior art: Pauli Virtanen's PATCH v2 8/8 at
https://lore.kernel.org/linux-bluetooth/e18591f264c50e15917cb8b9e5f9798d9880979d.1762100290.git.pav@iki.fi/.

Fixes: 45340097ce6e ("Bluetooth: hci_conn: Only do ACL connections sequentially")
Cc: stable@vger.kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
---
 net/bluetooth/hci_sync.c | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index 47ce9ba63fe2..9a133de16f63 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -6994,12 +6994,21 @@ static int hci_acl_create_conn_sync(struct hci_dev *hdev, void *data)
 					conn->conn_timeout, NULL);
 }
 
+static void create_acl_conn_complete(struct hci_dev *hdev, void *data, int err)
+{
+	struct hci_conn *conn = data;
+
+	bt_dev_dbg(hdev, "err %d", err);
+
+	hci_conn_put(conn);
+}
+
 int hci_connect_acl_sync(struct hci_dev *hdev, struct hci_conn *conn)
 {
 	int err;
 
-	err = hci_cmd_sync_queue_once(hdev, hci_acl_create_conn_sync, conn,
-				      NULL);
+	err = hci_cmd_sync_queue_conn_once(hdev, hci_acl_create_conn_sync, conn,
+					   create_acl_conn_complete);
 	return (err == -EEXIST) ? 0 : err;
 }
 
@@ -7054,7 +7063,8 @@ int hci_cancel_connect_sync(struct hci_dev *hdev, struct hci_conn *conn)
 	case ACL_LINK:
 		return !hci_cmd_sync_dequeue_once(hdev,
 						  hci_acl_create_conn_sync,
-						  conn, NULL);
+						  conn,
+						  create_acl_conn_complete);
 	case LE_LINK:
 		return !hci_cmd_sync_dequeue_once(hdev, hci_le_create_conn_sync,
 						  conn, create_le_conn_complete);
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/4] Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync
  2026-05-11 14:34 ` [PATCH 1/4] Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync Michael Bommarito
@ 2026-05-11 14:53   ` Luiz Augusto von Dentz
  2026-05-11 17:01     ` Michael Bommarito
  0 siblings, 1 reply; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2026-05-11 14:53 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: Marcel Holtmann, linux-bluetooth, linux-kernel, Mat Martineau,
	netdev, stable, Pauli Virtanen, Aaron Esau

Hi Michael,

On Mon, May 11, 2026 at 10:34 AM Michael Bommarito
<michael.bommarito@gmail.com> wrote:
>
> hci_le_create_conn_sync() runs from the cmd_sync workqueue with a
> struct hci_conn pointer it interprets out of the work item's void
> *data argument. The hci_conn_valid() check at function entry is a
> TOCTOU: nothing prevents hci_disconn_complete_evt() (executing on
> hdev->workqueue rx_work) from running between the
> hci_conn_hash_lookup walk in hci_conn_valid() and the body's first
> deref. hci_disconn_complete_evt() -> hci_conn_del() -> hci_conn_cleanup()
> unregisters the device and drops the final kref, which kfrees the
> hci_conn slot. The cmd_sync callback then writes through the freed
> pointer (clear_bit on conn->flags, conn->state, the four
> le_conn_*_interval fields).
>
> A KASAN slab-use-after-free splat in cache kmalloc-8k confirms the
> bug on linux-next tip commit bee6ea30c487 ("Add linux-next specific
> files for 20260421") under UML+KASAN, matching the slab geometry of
> the syzbot trace fixed in commit 035c25007c9e ("Bluetooth: hci_sync:
> Fix UAF in le_read_features_complete").
>
> Follow the reference-pinning pattern from commit 035c25007c9e
> ("Bluetooth: hci_sync: Fix UAF in le_read_features_complete") and
> commit 0beddb0c380b ("Bluetooth: hci_conn: fix potential UAF in
> create_big_sync"): the queue site takes a reference via
> hci_conn_get() so the slot is not freed between
> hci_disconn_complete_evt() retiring the conn and the cmd_sync
> callback / completion handler returning. The completion handler
> drops the reference on every exit path, including the -ECANCELED
> short-circuit.
>
> Introduce a static helper hci_cmd_sync_queue_conn_once() so the
> get/put pair is not open-coded at every queue site. See the
> helper's kerneldoc for the -EEXIST contract.
>
> The hci_conn_valid() check in the callback body is retained: a
> logically-deleted-but-still-referenced conn has stale
> hdev->conn_hash.list state, and continuing to drive a connection
> attempt on it would be a logic bug even though the memory is safe.
>
> Pauli Virtanen posted a series-wide variant of this fix as
> https://lore.kernel.org/linux-bluetooth/e18591f264c50e15917cb8b9e5f9798d9880979d.1762100290.git.pav@iki.fi/
> (PATCH v2 8/8, 2025-11-02). KASAN reproducer captured under
> UML+KASAN (linux-next tip bee6ea30c487).
>
> Fixes: 881559af5f5c ("Bluetooth: hci_sync: Attempt to dequeue connection attempt")
> Cc: stable@vger.kernel.org
> Assisted-by: Claude:claude-opus-4-7
> Signed-off-by: Michael Bommarito <michael.bommarito@gmail.com>
> ---
>  net/bluetooth/hci_sync.c | 41 ++++++++++++++++++++++++++++++++--------
>  1 file changed, 34 insertions(+), 7 deletions(-)
>
> diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
> index fd3aacdea512..b20e07474257 100644
> --- a/net/bluetooth/hci_sync.c
> +++ b/net/bluetooth/hci_sync.c
> @@ -786,6 +786,31 @@ int hci_cmd_sync_queue_once(struct hci_dev *hdev, hci_cmd_sync_work_func_t func,
>  }
>  EXPORT_SYMBOL(hci_cmd_sync_queue_once);
>
> +/* Queue an HCI command entry once, pinning a hci_conn for the duration.
> + *
> + * On success, the cmd_sync queue owns one hci_conn_get() reference;
> + * the supplied destroy callback must hci_conn_put() to balance.
> + *
> + * On any failure return (including -EEXIST, where
> + * hci_cmd_sync_queue_once() neither invokes destroy nor consumes the
> + * data pointer because an existing entry already owns the slot), the
> + * helper releases the reference before returning, so callers do not
> + * need to discriminate failure codes to keep the refcount balanced.
> + */
> +static int hci_cmd_sync_queue_conn_once(struct hci_dev *hdev,

Id suggest we dropped the once at the end so just hci_cmd_sync_queue_conn.

> +                                       hci_cmd_sync_work_func_t func,
> +                                       struct hci_conn *conn,
> +                                       hci_cmd_sync_work_destroy_t destroy)
> +{
> +       int err;
> +
> +       err = hci_cmd_sync_queue_once(hdev, func, hci_conn_get(conn), destroy);
> +       if (err)
> +               hci_conn_put(conn);
> +
> +       return err;

Then we incorporate return (err == -EEXIST) ? 0 : err; logic above, so
I don't think any caller should require queuing multiple procedures
for the same conn.

> +}
> +
>  /* Run HCI command:
>   *
>   * - hdev must be running
> @@ -6982,36 +7007,38 @@ static void create_le_conn_complete(struct hci_dev *hdev, void *data, int err)
>         bt_dev_dbg(hdev, "err %d", err);
>
>         if (err == -ECANCELED)
> -               return;
> +               goto done;
>
>         hci_dev_lock(hdev);
>
>         if (!hci_conn_valid(hdev, conn))
> -               goto done;
> +               goto unlock;
>
>         if (!err) {
>                 hci_connect_le_scan_cleanup(conn, 0x00);
> -               goto done;
> +               goto unlock;
>         }
>
>         /* Check if connection is still pending */
>         if (conn != hci_lookup_le_connect(hdev))
> -               goto done;
> +               goto unlock;
>
>         /* Flush to make sure we send create conn cancel command if needed */
>         flush_delayed_work(&conn->le_conn_timeout);
>         hci_conn_failed(conn, bt_status(err));
>
> -done:
> +unlock:
>         hci_dev_unlock(hdev);
> +done:
> +       hci_conn_put(conn);
>  }
>
>  int hci_connect_le_sync(struct hci_dev *hdev, struct hci_conn *conn)
>  {
>         int err;
>
> -       err = hci_cmd_sync_queue_once(hdev, hci_le_create_conn_sync, conn,
> -                                     create_le_conn_complete);
> +       err = hci_cmd_sync_queue_conn_once(hdev, hci_le_create_conn_sync, conn,
> +                                          create_le_conn_complete);
>         return (err == -EEXIST) ? 0 : err;
>  }
>
> --
> 2.53.0
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/4] Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync
  2026-05-11 14:53   ` Luiz Augusto von Dentz
@ 2026-05-11 17:01     ` Michael Bommarito
  2026-05-11 17:35       ` Luiz Augusto von Dentz
  0 siblings, 1 reply; 8+ messages in thread
From: Michael Bommarito @ 2026-05-11 17:01 UTC (permalink / raw)
  To: Luiz Augusto von Dentz
  Cc: Marcel Holtmann, linux-bluetooth, linux-kernel, Mat Martineau,
	netdev, stable, Pauli Virtanen, Aaron Esau

On Mon, May 11, 2026 at 10:53 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> Id suggest we dropped the once at the end so just hci_cmd_sync_queue_conn.
>
> > +                                       hci_cmd_sync_work_func_t func,
> > +                                       struct hci_conn *conn,
> > +                                       hci_cmd_sync_work_destroy_t destroy)
> > +{
> > +       int err;
> > +
> > +       err = hci_cmd_sync_queue_once(hdev, func, hci_conn_get(conn), destroy);
> > +       if (err)
> > +               hci_conn_put(conn);
> > +
> > +       return err;
>
> Then we incorporate return (err == -EEXIST) ? 0 : err; logic above, so
> I don't think any caller should require queuing multiple procedures
> for the same conn.

OK, good call.  I checked the other 10 callers for
hci_cmd_sync_queue_once and there isn't any variation today, so that
seems safe.

Do you want me to wait another 24-48 for others to weigh in or ship v2 now?

^ permalink raw reply	[flat|nested] 8+ messages in thread

* Re: [PATCH 1/4] Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync
  2026-05-11 17:01     ` Michael Bommarito
@ 2026-05-11 17:35       ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 8+ messages in thread
From: Luiz Augusto von Dentz @ 2026-05-11 17:35 UTC (permalink / raw)
  To: Michael Bommarito
  Cc: Marcel Holtmann, linux-bluetooth, linux-kernel, Mat Martineau,
	netdev, stable, Pauli Virtanen, Aaron Esau

Hi Michael,

On Mon, May 11, 2026 at 1:01 PM Michael Bommarito
<michael.bommarito@gmail.com> wrote:
>
> On Mon, May 11, 2026 at 10:53 AM Luiz Augusto von Dentz
> <luiz.dentz@gmail.com> wrote:
> > Id suggest we dropped the once at the end so just hci_cmd_sync_queue_conn.
> >
> > > +                                       hci_cmd_sync_work_func_t func,
> > > +                                       struct hci_conn *conn,
> > > +                                       hci_cmd_sync_work_destroy_t destroy)
> > > +{
> > > +       int err;
> > > +
> > > +       err = hci_cmd_sync_queue_once(hdev, func, hci_conn_get(conn), destroy);
> > > +       if (err)
> > > +               hci_conn_put(conn);
> > > +
> > > +       return err;
> >
> > Then we incorporate return (err == -EEXIST) ? 0 : err; logic above, so
> > I don't think any caller should require queuing multiple procedures
> > for the same conn.
>
> OK, good call.  I checked the other 10 callers for
> hci_cmd_sync_queue_once and there isn't any variation today, so that
> seems safe.
>
> Do you want me to wait another 24-48 for others to weigh in or ship v2 now?

Just create a v2, anyone with comments can add them there as well.

-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 8+ messages in thread

end of thread, other threads:[~2026-05-11 17:35 UTC | newest]

Thread overview: 8+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-05-11 14:34 [PATCH 0/4] Bluetooth: hci_sync: fix TOCTOU UAF in cmd_sync callbacks Michael Bommarito
2026-05-11 14:34 ` [PATCH 1/4] Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync Michael Bommarito
2026-05-11 14:53   ` Luiz Augusto von Dentz
2026-05-11 17:01     ` Michael Bommarito
2026-05-11 17:35       ` Luiz Augusto von Dentz
2026-05-11 14:34 ` [PATCH 2/4] Bluetooth: hci_sync: pin conn across hci_le_pa_create_sync Michael Bommarito
2026-05-11 14:34 ` [PATCH 3/4] Bluetooth: hci_sync: pin conn across hci_le_big_create_sync Michael Bommarito
2026-05-11 14:34 ` [PATCH 4/4] Bluetooth: hci_sync: pin conn across hci_acl_create_conn_sync Michael Bommarito

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox