linux-kernel - [PATCH] Bluetooth: hci: fix refcounts in LE remote features command

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20251216191255.882653-1-cakturk@gmail.com>
Date: Tue, 16 Dec 2025 22:12:55 +0300
From: Cihangir Akturk <cakturk@...il.com>
To: linux-bluetooth@...r.kernel.org
Cc: Marcel Holtmann <marcel@...tmann.org>,
	Johan Hedberg <johan.hedberg@...il.com>,
	Luiz Augusto von Dentz <luiz.dentz@...il.com>,
	linux-kernel@...r.kernel.org,
	Cihangir Akturk <cakturk@...il.com>,
	syzbot+87badbb9094e008e0685@...kaller.appspotmail.com
Subject: [PATCH] Bluetooth: hci: fix refcounts in LE remote features command

KASAN reported a slab-use-after-free in le_read_features_complete()
running from hci_cmd_sync_work.  le_read_features_complete() can run
after hci_rx_work/hci_conn_del() has removed the link, so the destroy
callback may touch a freed hci_conn and trigger a KASAN use-after-free.
Duplicate submissions also need to drop the references to avoid leaking
the hold and blocking teardown.

Fix this by taking hci_conn_get() before queueing, passing the conn
directly as work data, and balancing hci_conn_hold()/drop() and
hci_conn_get()/put() in the completion and all error/-EEXIST paths so
the connection stays referenced while the work runs and is released
afterwards.

Reported-by: syzbot+87badbb9094e008e0685@...kaller.appspotmail.com
Signed-off-by: Cihangir Akturk <cakturk@...il.com>
---
 net/bluetooth/hci_sync.c | 37 ++++++++++++++++++++++++++-----------
 1 file changed, 26 insertions(+), 11 deletions(-)

I am not entirely sure that removing the err == -ECANCELED early return
is strictly correct. My assumption is that, with the changes in this
patch, there does not appear to be another cancellation path that
reliably balances hci_conn_drop() and hci_conn_put() for this command.
Based on that assumption, keeping the early return could leave the
references taken before queuing unbalanced on cancellation, so I opted
to remove it to keep the lifetime handling consistent.

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index a9f5b1a68356..5a3d288e7015 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -776,14 +776,23 @@ _hci_cmd_sync_lookup_entry(struct hci_dev *hdev, hci_cmd_sync_work_func_t func,
  * - Lookup if an entry already exist and only if it doesn't creates a new entry
  *   and queue it.
  */
-int hci_cmd_sync_queue_once(struct hci_dev *hdev, hci_cmd_sync_work_func_t func,
+static int __hci_cmd_sync_queue_once(struct hci_dev *hdev, hci_cmd_sync_work_func_t func,
 			    void *data, hci_cmd_sync_work_destroy_t destroy)
 {
 	if (hci_cmd_sync_lookup_entry(hdev, func, data, destroy))
-		return 0;
+		return -EEXIST;
 
 	return hci_cmd_sync_queue(hdev, func, data, destroy);
 }
+
+int hci_cmd_sync_queue_once(struct hci_dev *hdev, hci_cmd_sync_work_func_t func,
+			    void *data, hci_cmd_sync_work_destroy_t destroy)
+{
+	int ret;
+
+	ret = __hci_cmd_sync_queue_once(hdev, func, data, destroy);
+	return ret == -EEXIST ? 0 : ret;
+}
 EXPORT_SYMBOL(hci_cmd_sync_queue_once);
 
 /* Run HCI command:
@@ -7338,10 +7347,8 @@ static void le_read_features_complete(struct hci_dev *hdev, void *data, int err)
 
 	bt_dev_dbg(hdev, "err %d", err);
 
-	if (err == -ECANCELED)
-		return;
-
 	hci_conn_drop(conn);
+	hci_conn_put(conn);
 }
 
 static int hci_le_read_all_remote_features_sync(struct hci_dev *hdev,
@@ -7408,12 +7415,20 @@ int hci_le_read_remote_features(struct hci_conn *conn)
 	 * role is possible. Otherwise just transition into the
 	 * connected state without requesting the remote features.
 	 */
-	if (conn->out || (hdev->le_features[0] & HCI_LE_PERIPHERAL_FEATURES))
-		err = hci_cmd_sync_queue_once(hdev,
-					      hci_le_read_remote_features_sync,
-					      hci_conn_hold(conn),
-					      le_read_features_complete);
-	else
+	if (conn->out || (hdev->le_features[0] & HCI_LE_PERIPHERAL_FEATURES)) {
+		hci_conn_get(conn);
+		hci_conn_hold(conn);
+		err = __hci_cmd_sync_queue_once(hdev,
+						hci_le_read_remote_features_sync,
+						conn,
+						le_read_features_complete);
+		if (err) {
+			hci_conn_drop(conn);
+			hci_conn_put(conn);
+			if (err == -EEXIST)
+				err = 0;
+		}
+	} else
 		err = -EOPNOTSUPP;
 
 	return err;
-- 
2.52.0