[PATCH bluetooth 0/4] Bluetooth: fix cmd_sync payload lifetimes on the cancel path

[PATCH bluetooth 0/4] Bluetooth: fix cmd_sync payload lifetimes on the cancel path

[Linmao Li]

The cmd_sync worker calls entry->destroy() after running a command, and
_hci_cmd_sync_cancel_entry() does the same when an entry is cancelled --
but only if a destroy callback was supplied. Without one it frees the
work entry and leaves entry->data unreleased.

These four call sites pass a heap payload with a NULL destroy callback
and free it inside the sync function instead, so each of them leaks when
the entry is cancelled rather than run. hci_cmd_sync_clear() cancels
every pending entry when the controller is unregistered.

Patch 1 additionally holds the connection, as the payload stores a bare
hci_conn pointer. Patch 2 additionally frees the payload when queueing
fails, which it currently does not check for. Patches 3 and 4 also leak
the socket reference taken by mgmt_pending_new().

Each patch moves the release into a destroy callback; apart from holding
the connection in patch 1, no behaviour changes.

Found by inspection while reading the recent cmd_sync lifetime fixes.
Not tested on hardware: these paths need an adapter removal racing a
queued command, or allocation failure, to reach.

Linmao Li (4):
  Bluetooth: hci_conn: fix the SCO setup context lifetime
  Bluetooth: hci_sync: free the advertising instance on the failure and
    cancel paths
  Bluetooth: MGMT: free the mesh send cancel command when it is
    cancelled
  Bluetooth: MGMT: free the HCI command when it is cancelled

 net/bluetooth/hci_conn.c | 20 +++++++++++++++-----
 net/bluetooth/hci_sync.c | 12 +++++++++---
 net/bluetooth/mgmt.c     | 22 +++++++++++++++-------
 3 files changed, 39 insertions(+), 15 deletions(-)


base-commit: abd93c85c8667add738ee82aeab95dd9fc8265a2
-- 
2.25.1

[PATCH bluetooth 1/4] Bluetooth: hci_conn: fix the SCO setup context lifetime

[Linmao Li]

hci_setup_sync() queues a conn_handle_t with a NULL destroy callback, so
the context is only freed if hci_enhanced_setup_sync() actually runs. An
entry that is cancelled instead is leaked, as
_hci_cmd_sync_cancel_entry() does not release entry->data when there is
no destroy callback, and hci_cmd_sync_clear() cancels every pending entry
when the controller is unregistered.

The context also stores a bare hci_conn pointer, so the connection can be
freed while the work is queued. The dequeue in hci_conn_del() does not
cover it either, as it matches on entry->data == conn and entry->data is
the wrapper here. Same problem as commit 2f5d635ad590 ("Bluetooth:
hci_sync: hold conn in hci_connect_acl/le_sync() callbacks").

Hold the connection and release both from a destroy callback. The
submission failure path drops both, since hci_cmd_sync_submit() does not
call the destroy callback when it fails to queue.

Fixes: e07a06b4eb41 ("Bluetooth: Convert SCO configure_datapath to hci_sync")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 net/bluetooth/hci_conn.c | 20 +++++++++++++++-----
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
index b1f911fd4ad6a..19b7629b1cc10 100644
--- a/net/bluetooth/hci_conn.c
+++ b/net/bluetooth/hci_conn.c
@@ -283,8 +283,6 @@ static int hci_enhanced_setup_sync(struct hci_dev *hdev, void *data)
 	struct hci_cp_enhanced_setup_sync_conn cp;
 	const struct sco_param *param;
 
-	kfree(conn_handle);
-
 	if (!hci_conn_valid(hdev, conn))
 		return -ECANCELED;
 
@@ -453,6 +451,15 @@ static bool hci_setup_sync_conn(struct hci_conn *conn, __u16 handle)
 	return true;
 }
 
+static void hci_enhanced_setup_sync_destroy(struct hci_dev *hdev, void *data,
+					    int err)
+{
+	struct conn_handle_t *conn_handle = data;
+
+	hci_conn_put(conn_handle->conn);
+	kfree(conn_handle);
+}
+
 bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
 {
 	int result;
@@ -464,12 +471,15 @@ bool hci_setup_sync(struct hci_conn *conn, __u16 handle)
 		if (!conn_handle)
 			return false;
 
-		conn_handle->conn = conn;
+		conn_handle->conn = hci_conn_get(conn);
 		conn_handle->handle = handle;
 		result = hci_cmd_sync_queue(conn->hdev, hci_enhanced_setup_sync,
-					    conn_handle, NULL);
-		if (result < 0)
+					    conn_handle,
+					    hci_enhanced_setup_sync_destroy);
+		if (result < 0) {
+			hci_conn_put(conn);
 			kfree(conn_handle);
+		}
 
 		return result == 0;
 	}
-- 
2.25.1

[PATCH bluetooth 2/4] Bluetooth: hci_sync: free the advertising instance on the failure and cancel paths

[Linmao Li]

adv_timeout_expire() hands a kmalloc()ed instance byte to
hci_cmd_sync_queue() with a NULL destroy callback, and only
adv_timeout_expire_sync() frees it. That leaks on two paths:

 - the return value is not checked, and hci_cmd_sync_queue() does not
   take ownership when it fails (-ENETDOWN, -ENODEV, -ENOMEM);

 - a cancelled entry is not released, as _hci_cmd_sync_cancel_entry()
   does not free entry->data when there is no destroy callback.
   hci_cmd_sync_clear() cancels every pending entry when the controller
   is unregistered.

Free the buffer from a destroy callback, and in the caller when the entry
could not be queued at all.

Fixes: c249ea9b4309 ("Bluetooth: Move Adv Instance timer to hci_sync")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 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 c8d14128c363d..d21b7c8877545 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -540,8 +540,6 @@ static int adv_timeout_expire_sync(struct hci_dev *hdev, void *data)
 {
 	u8 instance = *(u8 *)data;
 
-	kfree(data);
-
 	hci_clear_adv_instance_sync(hdev, NULL, instance, false);
 
 	if (list_empty(&hdev->adv_instances))
@@ -550,6 +548,12 @@ static int adv_timeout_expire_sync(struct hci_dev *hdev, void *data)
 	return 0;
 }
 
+static void adv_timeout_expire_destroy(struct hci_dev *hdev, void *data,
+				       int err)
+{
+	kfree(data);
+}
+
 static void adv_timeout_expire(struct work_struct *work)
 {
 	u8 *inst_ptr;
@@ -570,7 +574,9 @@ static void adv_timeout_expire(struct work_struct *work)
 		goto unlock;
 
 	*inst_ptr = hdev->cur_adv_instance;
-	hci_cmd_sync_queue(hdev, adv_timeout_expire_sync, inst_ptr, NULL);
+	if (hci_cmd_sync_queue(hdev, adv_timeout_expire_sync, inst_ptr,
+			       adv_timeout_expire_destroy) < 0)
+		kfree(inst_ptr);
 
 unlock:
 	hci_dev_unlock(hdev);
-- 
2.25.1

[PATCH bluetooth 3/4] Bluetooth: MGMT: free the mesh send cancel command when it is cancelled

[Linmao Li]

mesh_send_cancel() queues the pending command with a NULL destroy
callback, so it is only freed if send_cancel() runs. A cancelled entry is
leaked, as _hci_cmd_sync_cancel_entry() does not release entry->data when
there is no destroy callback, and hci_cmd_sync_clear() cancels every
pending entry when the controller is unregistered. Nothing else reclaims
it either: mgmt_pending_new() does not put the command on
hdev->mgmt_pending.

The leak also pins the socket reference taken by mgmt_pending_new(), so
the mgmt socket is never released.

Free the command from a destroy callback.

Fixes: b338d91703fa ("Bluetooth: Implement support for Mesh")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 net/bluetooth/mgmt.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index 167d75e345266..a80653b5b875d 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2431,11 +2431,15 @@ static int send_cancel(struct hci_dev *hdev, void *data)
 
 	mgmt_cmd_complete(cmd->sk, hdev->id, MGMT_OP_MESH_SEND_CANCEL,
 			  0, NULL, 0);
-	mgmt_pending_free(cmd);
 
 	return 0;
 }
 
+static void send_cancel_destroy(struct hci_dev *hdev, void *data, int err)
+{
+	mgmt_pending_free(data);
+}
+
 static int mesh_send_cancel(struct sock *sk, struct hci_dev *hdev,
 			    void *data, u16 len)
 {
@@ -2456,7 +2460,8 @@ static int mesh_send_cancel(struct sock *sk, struct hci_dev *hdev,
 	if (!cmd)
 		err = -ENOMEM;
 	else
-		err = hci_cmd_sync_queue(hdev, send_cancel, cmd, NULL);
+		err = hci_cmd_sync_queue(hdev, send_cancel, cmd,
+					 send_cancel_destroy);
 
 	if (err < 0) {
 		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_MESH_SEND_CANCEL,
-- 
2.25.1

[PATCH bluetooth 4/4] Bluetooth: MGMT: free the HCI command when it is cancelled

[Linmao Li]

mgmt_hci_cmd_sync() queues the pending command with a NULL destroy
callback, so it is only freed if send_hci_cmd_sync() runs. A cancelled
entry is leaked, as _hci_cmd_sync_cancel_entry() does not release
entry->data when there is no destroy callback, and hci_cmd_sync_clear()
cancels every pending entry when the controller is unregistered. Nothing
else reclaims it either: mgmt_pending_new() does not put the command on
hdev->mgmt_pending.

The leak also pins the socket reference taken by mgmt_pending_new(), so
the mgmt socket is never released.

Free the command from a destroy callback. The now-empty done label is
replaced by a direct return.

Fixes: 827af4787e74 ("Bluetooth: MGMT: Add initial implementation of MGMT_OP_HCI_CMD_SYNC")
Signed-off-by: Linmao Li <lilinmao@kylinos.cn>
---
 net/bluetooth/mgmt.c | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/mgmt.c b/net/bluetooth/mgmt.c
index a80653b5b875d..7e9d27eefb504 100644
--- a/net/bluetooth/mgmt.c
+++ b/net/bluetooth/mgmt.c
@@ -2647,7 +2647,7 @@ static int send_hci_cmd_sync(struct hci_dev *hdev, void *data)
 	if (IS_ERR(skb)) {
 		mgmt_cmd_status(cmd->sk, hdev->id, MGMT_OP_HCI_CMD_SYNC,
 				mgmt_status(PTR_ERR(skb)));
-		goto done;
+		return 0;
 	}
 
 	mgmt_cmd_complete(cmd->sk, hdev->id, MGMT_OP_HCI_CMD_SYNC, 0,
@@ -2655,12 +2655,14 @@ static int send_hci_cmd_sync(struct hci_dev *hdev, void *data)
 
 	kfree_skb(skb);
 
-done:
-	mgmt_pending_free(cmd);
-
 	return 0;
 }
 
+static void send_hci_cmd_sync_destroy(struct hci_dev *hdev, void *data, int err)
+{
+	mgmt_pending_free(data);
+}
+
 static int mgmt_hci_cmd_sync(struct sock *sk, struct hci_dev *hdev,
 			     void *data, u16 len)
 {
@@ -2678,7 +2680,8 @@ static int mgmt_hci_cmd_sync(struct sock *sk, struct hci_dev *hdev,
 	if (!cmd)
 		err = -ENOMEM;
 	else
-		err = hci_cmd_sync_queue(hdev, send_hci_cmd_sync, cmd, NULL);
+		err = hci_cmd_sync_queue(hdev, send_hci_cmd_sync, cmd,
+					 send_hci_cmd_sync_destroy);
 
 	if (err < 0) {
 		err = mgmt_cmd_status(sk, hdev->id, MGMT_OP_HCI_CMD_SYNC,
-- 
2.25.1

Re: [PATCH bluetooth 0/4] Bluetooth: fix cmd_sync payload lifetimes on the cancel path

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Thu,  6 Aug 2026 20:59:53 +0800 you wrote:
> The cmd_sync worker calls entry->destroy() after running a command, and
> _hci_cmd_sync_cancel_entry() does the same when an entry is cancelled --
> but only if a destroy callback was supplied. Without one it frees the
> work entry and leaves entry->data unreleased.
> 
> These four call sites pass a heap payload with a NULL destroy callback
> and free it inside the sync function instead, so each of them leaks when
> the entry is cancelled rather than run. hci_cmd_sync_clear() cancels
> every pending entry when the controller is unregistered.
> 
> [...]

Here is the summary with links:
  - [1/4] Bluetooth: hci_conn: fix the SCO setup context lifetime
    https://git.kernel.org/bluetooth/bluetooth-next/c/7645e748704a
  - [2/4] Bluetooth: hci_sync: free the advertising instance on the failure and cancel paths
    https://git.kernel.org/bluetooth/bluetooth-next/c/19aa86a8d171
  - [3/4] Bluetooth: MGMT: free the mesh send cancel command when it is cancelled
    https://git.kernel.org/bluetooth/bluetooth-next/c/ac0965fb4b86
  - [4/4] Bluetooth: MGMT: free the HCI command when it is cancelled
    https://git.kernel.org/bluetooth/bluetooth-next/c/f2afd8eadf5d

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html