Netdev List
 help / color / mirror / Atom feed
From: Michael Bommarito <michael.bommarito@gmail.com>
To: Marcel Holtmann <marcel@holtmann.org>,
	Luiz Augusto von Dentz <luiz.dentz@gmail.com>,
	linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org
Cc: Mat Martineau <martineau@kernel.org>,
	netdev@vger.kernel.org, stable@vger.kernel.org,
	Pauli Virtanen <pav@iki.fi>, Aaron Esau <git@aaronesau.com>,
	Michael Bommarito <michael.bommarito@gmail.com>
Subject: [PATCH 1/4] Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync
Date: Mon, 11 May 2026 10:34:01 -0400	[thread overview]
Message-ID: <490e228dd02983fb1530fb114d4174148f810261.1778506829.git.michael.bommarito@gmail.com> (raw)
In-Reply-To: <cover.1778506829.git.michael.bommarito@gmail.com>

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


  reply	other threads:[~2026-05-11 14:34 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
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 [this message]
2026-05-11 14:53   ` [PATCH 1/4] Bluetooth: hci_sync: pin conn across hci_le_create_conn_sync 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

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=490e228dd02983fb1530fb114d4174148f810261.1778506829.git.michael.bommarito@gmail.com \
    --to=michael.bommarito@gmail.com \
    --cc=git@aaronesau.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.org \
    --cc=martineau@kernel.org \
    --cc=netdev@vger.kernel.org \
    --cc=pav@iki.fi \
    --cc=stable@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox