Re: [PATCH v2] Bluetooth: ISO: fix UAF on socket close before shutdown completes

Hi Pauli,

On Mon, Jul 20, 2026 at 2:30 PM Pauli Virtanen <[email protected]> wrote:
>
> iso_sock_disconn() aims to disconnect the hcon by dropping it, which
> triggers iso_conn_del() once the HCI operation completes, requiring
> valid hcon->iso_data to do socket cleanup.  iso_sock_disconn() sets
> conn->hcon = NULL to avoid a second drop, but also preventing clearing
> hcon->iso_data on socket release. Closing the socket before
> iso_conn_del() runs then results to UAF.
>
> Fix by using a separate flag to track the hcon drop status, instead of
> clearing conn->hcon.
>
> Log: (BlueZ iso-tester ISO Connect Close - Success)
> BUG: KASAN: slab-use-after-free in iso_conn_hold_unless_zero
> ...
>  iso_conn_hold_unless_zero (net/bluetooth/iso.c:138)
>  iso_conn_del (net/bluetooth/iso.c:270)
>  hci_conn_failed (net/bluetooth/hci_conn.c:1408)
>  hci_abort_conn_sync (net/bluetooth/hci_sync.c:5817)
>
> Allocated by task 34:
>  iso_conn_add (net/bluetooth/iso.c:216)
>  iso_connect_cis (net/bluetooth/iso.c:507)
>  iso_sock_connect (net/bluetooth/iso.c:1211)
>  __sys_connect (net/socket.c:2148)
>
> Freed by task 34:
>  iso_chan_del (net/bluetooth/iso.c:248)
>  iso_sock_close (net/bluetooth/iso.c:885)
>  iso_sock_release (net/bluetooth/iso.c:2022)
>  sock_close (net/socket.c:722)
>
> Fixes: fbdc4bc47268 ("Bluetooth: ISO: Use defer setup to separate PA sync and BIG sync")
> Signed-off-by: Pauli Virtanen <[email protected]>
> ---
>  net/bluetooth/iso.c | 11 ++++++++---
>  1 file changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
> index 2e95a153912c..6abc2b1f59bc 100644
> --- a/net/bluetooth/iso.c
> +++ b/net/bluetooth/iso.c
> @@ -30,6 +30,7 @@ struct iso_conn {
>         /* @lock: spinlock protecting changes to iso_conn fields */
>         spinlock_t      lock;
>         struct sock     *sk;
> +       bool            hcon_dropped;

It might be safer to add a flag or something so we can perform atomic
operations to check if dropping is necessary.

>         struct delayed_work     timeout_work;
>
> @@ -107,7 +108,8 @@ static void iso_conn_free(struct kref *ref)
>
>         if (conn->hcon) {
>                 conn->hcon->iso_data = NULL;
> -               hci_conn_drop(conn->hcon);
> +               if (!conn->hcon_dropped)
> +                       hci_conn_drop(conn->hcon);
>         }
>
>         /* Ensure no more work items will run since hci_conn has been dropped */
> @@ -306,6 +308,7 @@ static int __iso_chan_add(struct iso_conn *conn, struct sock *sk,
>
>         iso_pi(sk)->conn = conn;
>         conn->sk = sk;
> +       conn->hcon_dropped = false;
>
>         if (parent)
>                 bt_accept_enqueue(parent, sk, true);
> @@ -836,8 +839,10 @@ static void iso_sock_disconn(struct sock *sk)
>
>         sk->sk_state = BT_DISCONN;
>         iso_conn_lock(iso_pi(sk)->conn);
> -       hci_conn_drop(iso_pi(sk)->conn->hcon);
> -       iso_pi(sk)->conn->hcon = NULL;
> +       if (!iso_pi(sk)->conn->hcon_dropped) {
> +               iso_pi(sk)->conn->hcon_dropped = true;
> +               hci_conn_drop(iso_pi(sk)->conn->hcon);
> +       }
>         iso_conn_unlock(iso_pi(sk)->conn);
>  }
>
> --
> 2.55.0
>


-- 
Luiz Augusto von Dentz