Linux bluetooth development
 help / color / mirror / Atom feed
* [PATCH v3] Bluetooth: MGMT: revalidate LOAD_CONN_PARAM queued update
@ 2026-07-07  4:15 Cen Zhang
  2026-07-07  6:25 ` [v3] " bluez.test.bot
  2026-07-07 17:30 ` [PATCH v3] " patchwork-bot+bluetooth
  0 siblings, 2 replies; 3+ messages in thread
From: Cen Zhang @ 2026-07-07  4:15 UTC (permalink / raw)
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: linux-bluetooth, baijiaju1990, zzzccc427

MGMT_OP_LOAD_CONN_PARAM queues conn_update_sync() when a single parameter
update changes an existing LE central connection. The queued work currently
stores a borrowed hci_conn_params entry from hdev->le_conn_params. A later
LOAD_CONN_PARAM request can clear disabled parameters and free that entry
before hci_cmd_sync_work() runs the queued callback.

Do not keep the borrowed hci_conn_params pointer in queued work. Queue the
hci_conn instead and hold a reference until the queued callback completes.
When the work runs, revalidate that the connection is still present, look
up the current hci_conn_params entry, and cancel the update if userspace
removed that entry while the work was pending.

Copy the interval values from the current params entry under hdev->lock,
then drop the lock and keep using hci_le_conn_update_sync() to issue the
update.

Validation reproduced this kernel report:
BUG: KASAN: slab-use-after-free in conn_update_sync+0x2a/0xf0 [bluetooth]
Read of size 1 at addr ffff88810c697126 by task kworker/u17:0/377
Workqueue: hci0 hci_cmd_sync_work [bluetooth]

Call Trace:
 <TASK>
 dump_stack_lvl+0x66/0xa0
 print_report+0xce/0x5f0
 kasan_report+0xe0/0x110
 conn_update_sync+0x2a/0xf0 [bluetooth]
 hci_cmd_sync_work+0x187/0x210 [bluetooth]
 process_one_work+0x4fd/0xbc0
 worker_thread+0x2d8/0x570
 kthread+0x1ad/0x1f0
 ret_from_fork+0x3c9/0x540
 ret_from_fork_asm+0x1a/0x30

Allocated by task 466:
 hci_conn_params_add+0xa6/0x240 [bluetooth]
 load_conn_param+0x4e1/0x850 [bluetooth]
 hci_sock_sendmsg+0x96b/0xf80 [bluetooth]

Freed by task 474:
 kfree+0x313/0x590
 hci_conn_params_clear_disabled+0x9b/0xc0 [bluetooth]
 load_conn_param+0x4bf/0x850 [bluetooth]
 hci_sock_sendmsg+0x96b/0xf80 [bluetooth]

Fixes: 0ece498c27d8c ("Bluetooth: MGMT: Make MGMT_OP_LOAD_CONN_PARAM update existing connection")
Suggested-by: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Assisted-by: Codex:gpt-5.5
Signed-off-by: Cen Zhang <zzzccc427@gmail.com>
---
v3:
- Keep using hci_le_conn_update_sync() instead of open-coding the HCI
  command in mgmt.c.
- Queue a referenced hci_conn instead of allocating an address-key wrapper.
- Revalidate the connection and current hci_conn_params under hdev->lock,
  then pass a stack copy of the interval fields to hci_le_conn_update_sync().

v2:
- Re-check hci_conn_params in conn_update_sync() and skip the queued
  update if the entry was removed while the work was pending, as
  suggested by Luiz.
- Queue only the address key and copy the current params and connection
  handle under hdev->lock before issuing the HCI command.

 net/bluetooth/mgmt.c | 44 +++++++++++++++++++++++++++++++++++---------
 1 file changed, 35 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 733a4b70e10c..a0a491e5311e 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -7937,14 +7937,36 @@ static int remove_device(struct sock *sk, struct hci_dev *hdev,
 
 static int conn_update_sync(struct hci_dev *hdev, void *data)
 {
-	struct hci_conn_params *params = data;
-	struct hci_conn *conn;
+	struct hci_conn *conn = data;
+	struct hci_conn_params *params;
+	struct hci_conn_params local = {};
 
-	conn = hci_conn_hash_lookup_le(hdev, &params->addr, params->addr_type);
-	if (!conn)
-		return -ECANCELED;
+	hci_dev_lock(hdev);
+
+	if (!hci_conn_valid(hdev, conn) || conn->role != HCI_ROLE_MASTER)
+		goto cancel;
+
+	params = hci_conn_params_lookup(hdev, &conn->dst, conn->dst_type);
+	if (!params)
+		goto cancel;
+
+	local.conn_min_interval = params->conn_min_interval;
+	local.conn_max_interval = params->conn_max_interval;
+	local.conn_latency = params->conn_latency;
+	local.supervision_timeout = params->supervision_timeout;
 
-	return hci_le_conn_update_sync(hdev, conn, params);
+	hci_dev_unlock(hdev);
+
+	return hci_le_conn_update_sync(hdev, conn, &local);
+
+cancel:
+	hci_dev_unlock(hdev);
+	return -ECANCELED;
+}
+
+static void conn_update_sync_destroy(struct hci_dev *hdev, void *data, int err)
+{
+	hci_conn_put(data);
 }
 
 static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
@@ -8054,9 +8076,13 @@ static int load_conn_param(struct sock *sk, struct hci_dev *hdev, void *data,
 			    (conn->le_conn_min_interval != min ||
 			     conn->le_conn_max_interval != max ||
 			     conn->le_conn_latency != latency ||
-			     conn->le_supv_timeout != timeout))
-				hci_cmd_sync_queue(hdev, conn_update_sync,
-						   hci_param, NULL);
+			     conn->le_supv_timeout != timeout)) {
+				hci_conn_get(conn);
+				if (hci_cmd_sync_queue(hdev, conn_update_sync,
+						       conn,
+						       conn_update_sync_destroy) < 0)
+					hci_conn_put(conn);
+			}
 		}
 	}
 
-- 
2.43.0

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

end of thread, other threads:[~2026-07-07 17:30 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-07  4:15 [PATCH v3] Bluetooth: MGMT: revalidate LOAD_CONN_PARAM queued update Cen Zhang
2026-07-07  6:25 ` [v3] " bluez.test.bot
2026-07-07 17:30 ` [PATCH v3] " patchwork-bot+bluetooth

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