Re: [PATCH v2 net-next] Bluetooth: hci_conn: fix potential UAF in create_big_sync

Paul Menzel
2026-04-10T20:25:59Z

Dear David,

Thank you for the patch.

Am 10.04.26 um 19:34 schrieb David Carlier:
> Add hci_conn_valid() check in create_big_sync() to detect stale
> connections before proceeding with BIG creation. Fix
> create_big_complete() to handle the resulting -ECANCELED error
> and validate the connection under hci_dev_lock() before
> dereferencing, following the established pattern used by
> create_le_conn_complete() and create_pa_complete().

(Using 75 characters per line would save a line.)

> Without this, create_big_complete() would unconditionally
> dereference the stale conn pointer on error, causing a
> use-after-free via hci_connect_cfm() and hci_conn_del().
>
> Fixes: eca0ae4aea66 ("Bluetooth: Add initial implementation of BIS connections")
> Cc: stable@vger.kernel.org
> Signed-off-by: David Carlier <devnexen@gmail.com>
> ---
>
> v1 -> v2: fix create_big_complete() to handle -ECANCELED and
> validate conn under hci_dev_lock(), matching the pattern in
> create_le_conn_complete() and create_pa_complete().
> v1: https://lore.kernel.org/r/20260408155638.95927-1-devnexen@gmail.com
> net/bluetooth/hci_conn.c | 14 ++++++++++++++
> 1 file changed, 14 insertions(+)
>
> diff --git a/net/bluetooth/hci_conn.c b/net/bluetooth/hci_conn.c
> index 11d3ad8d2551..feebe933efc8 100644
> --- a/net/bluetooth/hci_conn.c
> +++ b/net/bluetooth/hci_conn.c
> @@ -2130,6 +2130,9 @@ static int create_big_sync(struct hci_dev *hdev, void *data)
> u32 flags = 0;
> int err;
>
> + if (!hci_conn_valid(hdev, conn))
> + return -ECANCELED;
> +

I wonder if a debug message about the stale connection would be useful.

gemini/gemini-3.1-pro-preview comments [1]:

> Could this introduce a time-of-check to time-of-use race condition?
> Because create_big_sync() executes asynchronously in the cmd_sync_work
> workqueue without holding hci_dev_lock(), hci_conn_valid() only protects
> the lookup for the duration of its internal rcu_read_lock().
> If a concurrent thread acquires hci_dev_lock() and calls hci_conn_del()
> immediately after hci_conn_valid() returns true:
> cpu 1
> create_big_sync()
> if (!hci_conn_valid(hdev, conn))
> return -ECANCELED;
> cpu 2
> hci_dev_lock(hdev);
> hci_conn_del(conn);
> hci_dev_unlock(hdev);
> cpu 1
> if (qos->bcast.out.phys == BIT(1))
> Will this lead to a use-after-free on the conn pointer since qos resolves
> to &conn->iso_qos?
> Additionally, since conn is passed to the workqueue without holding a
> reference via hci_conn_get(), could this be susceptible to a pointer reuse
> problem?
> If the original connection is freed and the memory is reallocated for a
> new, unrelated connection before the work runs, hci_conn_valid() might
> incorrectly return true. This could cause the new connection to be wrongly
> operated on or deleted later in create_big_complete().
> Does the caller queueing the work need to take a proper reference with
> hci_conn_get() and release it in the completion callback instead?
> [ ... ]

> if (qos->bcast.out.phys == BIT(1))
> flags |= MGMT_ADV_FLAG_SEC_2M;
>
> @@ -2204,11 +2207,22 @@ static void create_big_complete(struct hci_dev *hdev, void *data, int err)
>
> bt_dev_dbg(hdev, "conn %p", conn);
>
> + if (err == -ECANCELED)
> + return;

Should the error message still be printed in this case?

bt_dev_err(hdev, "Unable to create BIG: ECANCELED");

> +
> + hci_dev_lock(hdev);
> +
> + if (!hci_conn_valid(hdev, conn))
> + goto done;
> +
> if (err) {
> bt_dev_err(hdev, "Unable to create BIG: %d", err);
> hci_connect_cfm(conn, err);
> hci_conn_del(conn);
> }
> +
> +done:
> + hci_dev_unlock(hdev);
> }
>
> struct hci_conn *hci_bind_bis(struct hci_dev *hdev, bdaddr_t *dst, __u8 sid,

Kind regards,

Paul

[1]: