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 <pav@iki.fi>
---
 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;
 
 	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

Hi Pauli,

On Mon, Jul 20, 2026 at 2:30 PM Pauli Virtanen <pav@iki.fi> 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 <pav@iki.fi>
> ---
>  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

Hi Luiz,

ma, 2026-07-20 kello 15:37 -0400, Luiz Augusto von Dentz kirjoitti:
> Hi Pauli,
> 
> On Mon, Jul 20, 2026 at 2:30 PM Pauli Virtanen <pav@iki.fi> 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 <pav@iki.fi>
> > ---
> >  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.

It is serialized by iso_conn::lock + kref, so AFAICS it is safe. I can
change it to test_and_set_bit() regardless if still preferable.

The lock is not taken in iso_conn_free() --- at that point nobody is
holding a reference so there should be no concurrent modification, so
AFAIK it is taken care of by the refcount memory ordering (see comment
in include/linux/refcount.h), otherwise conn->hcon etc would also be
wrong.

The
comment https://sashiko.dev/#/patchset/4d96c545ad1a2fed02440a3478905853286aa0c7.1784571683.git.pav%40iki.fi
on Sashiko seems to miss that iso_connect_cfm / iso_disconn_cfm ->
iso_conn_del() are called before hci_conn is deleted, and should make
sure there's no dangling reference to the hci_conn. It would be same as
for remote disconnect. Maybe the prompts it uses in
https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/bluetooth.md
could be improved vs hci_conn and socket life cycles.

> >         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
> > 
> 

Hi Pauli,

On Mon, Jul 20, 2026 at 4:08 PM Pauli Virtanen <pauli.virtanen@iki.fi> wrote:
>
> Hi Luiz,
>
> ma, 2026-07-20 kello 15:37 -0400, Luiz Augusto von Dentz kirjoitti:
> > Hi Pauli,
> >
> > On Mon, Jul 20, 2026 at 2:30 PM Pauli Virtanen <pav@iki.fi> 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 <pav@iki.fi>
> > > ---
> > >  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.
>
> It is serialized by iso_conn::lock + kref, so AFAICS it is safe. I can
> change it to test_and_set_bit() regardless if still preferable.

I'm afraid in today's world that is sort of required; otherwise,
someone using an AI model might miss the serialization and send
patches to 'fix' it anyway, just creating more work for us to review.

> The lock is not taken in iso_conn_free() --- at that point nobody is
> holding a reference so there should be no concurrent modification, so
> AFAIK it is taken care of by the refcount memory ordering (see comment
> in include/linux/refcount.h), otherwise conn->hcon etc would also be
> wrong.
>
> The
> comment https://sashiko.dev/#/patchset/4d96c545ad1a2fed02440a3478905853286aa0c7.1784571683.git.pav%40iki.fi
> on Sashiko seems to miss that iso_connect_cfm / iso_disconn_cfm ->
> iso_conn_del() are called before hci_conn is deleted, and should make
> sure there's no dangling reference to the hci_conn. It would be same as
> for remote disconnect. Maybe the prompts it uses in
> https://github.com/masoncl/review-prompts/blob/main/kernel/subsystem/bluetooth.md
> could be improved vs hci_conn and socket life cycles.
>
> > >         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