[PATCH v2 8/8] Bluetooth: hci_sync: hold references in hci_sync callbacks

Pauli Virtanen
2025-11-02T16:23:10Z

hci_conn_valid() should not be used on potentially freed hci_conn
pointers, as relying on kmalloc not reusing addresses is bad practice.

Hold a hci_conn reference for queued jobs so the pointers are not freed
too early.

This also avoids potential UAF if the conn would be freed while the jobs
are running.

Signed-off-by: Pauli Virtanen <pav@iki.fi>
---

Notes:
v2:
- fix also hci_past_sync()

net/bluetooth/hci_sync.c | 66 +++++++++++++++++++++++++++++++---------
1 file changed, 51 insertions(+), 15 deletions(-)

diff --git a/net/bluetooth/hci_sync.c b/net/bluetooth/hci_sync.c
index dc95a1ebe65e..d4420d5882d6 100644
--- a/net/bluetooth/hci_sync.c
+++ b/net/bluetooth/hci_sync.c
@@ -6962,12 +6962,23 @@ static int hci_acl_create_conn_sync(struct hci_dev *hdev, void *data)
conn->conn_timeout, NULL);
}

+static void hci_acl_create_conn_sync_complete(struct hci_dev *hdev, void *data,
+ int err)
+{
+ struct hci_conn *conn = data;
+
+ 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_once(hdev, hci_acl_create_conn_sync,
+ hci_conn_get(conn),
+ hci_acl_create_conn_sync_complete);
+ if (err)
+ hci_conn_put(conn);
return (err == -EEXIST) ? 0 : err;
}

@@ -6978,36 +6989,41 @@ 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,
+ err = hci_cmd_sync_queue_once(hdev, hci_le_create_conn_sync,
+ hci_conn_get(conn),
create_le_conn_complete);
+ if (err)
+ hci_conn_put(conn);
return (err == -EEXIST) ? 0 : err;
}

@@ -7055,7 +7071,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);

@@ -7079,6 +7095,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,
@@ -7217,8 +7235,11 @@ 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,
+ err = hci_cmd_sync_queue_once(hdev, hci_le_pa_create_sync,
+ hci_conn_get(conn),
create_pa_complete);
+ if (err)
+ hci_conn_put(conn);
return (err == -EEXIST) ? 0 : err;
}

@@ -7229,10 +7250,17 @@ 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)
@@ -7283,8 +7311,11 @@ 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,
+ err = hci_cmd_sync_queue_once(hdev, hci_le_big_create_sync,
+ hci_conn_get(conn),
create_big_complete);
+ if (err)
+ hci_conn_put(conn);
return (err == -EEXIST) ? 0 : err;
}

@@ -7299,6 +7330,8 @@ static void past_complete(struct hci_dev *hdev, void *data, int err)

bt_dev_dbg(hdev, "err %d", err);

+ hci_conn_put(past->conn);
+ hci_conn_put(past->le);
kfree(past);
}

@@ -7363,8 +7396,8 @@ int hci_past_sync(struct hci_conn *conn, struct hci_conn *le)
if (!data)
return -ENOMEM;

- data->conn = conn;
- data->le = le;
+ data->conn = hci_conn_get(conn);
+ data->le = hci_conn_get(le);

if (conn->role == HCI_ROLE_MASTER)
err = hci_cmd_sync_queue_once(conn->hdev,
@@ -7374,8 +7407,11 @@ int hci_past_sync(struct hci_conn *conn, struct hci_conn *le)
err = hci_cmd_sync_queue_once(conn->hdev, hci_le_past_sync,
data, past_complete);

- if (err)
+ if (err) {
+ hci_conn_put(data->conn);
+ hci_conn_put(data->le);
kfree(data);
+ }

return (err == -EEXIST) ? 0 : err;
}
--
2.51.1