[PATCH 00/11] Bluetooth: ISO: fix HUP on socket release/shutdown + UAF/locking fixes

Pauli Virtanen posted 11 patches 9 hours ago
include/net/bluetooth/hci_core.h |   4 +-
net/bluetooth/hci_conn.c         |   2 +
net/bluetooth/iso.c              | 274 ++++++++++++++++++++++---------
3 files changed, 197 insertions(+), 83 deletions(-)
[PATCH 00/11] Bluetooth: ISO: fix HUP on socket release/shutdown + UAF/locking fixes
Posted by Pauli Virtanen 9 hours ago
The retracted patch in commit e824c0bbe0ec9 ("Bluetooth: ISO: clear
iso_data always when detaching conn from hcon") merged in
bluetooth-next/master broke ISO socket transition to BT_CLOSED so they
don't send POLLHUP properly any more, as seen in "ISO Disconnect -
Success" test.

This is rebased version of and supercedes
https://lore.kernel.org/linux-bluetooth/fbd9dd573bb1ce5f38370128dd35447e2ddfed9c.1784625576.git.pav@iki.fi/

The first commit in this series fixes that, and subsequent mostly
independent commits other minor issues found looking at the ISO socket
code.

The last three fix UAF issues. The locking/lifetime of iso_conn
complicates the code here and the last three patches can be replaced by
somewhat simpler solution with bigger diff that removes iso_conn
https://github.com/pv/linux/commit/e4b460b5f7aaba060b958229aaec6b2217642178

Fixed issues aside BT_CONNECT, these appear to be pre-existing in
several previous kernel releases:

- sk is always leaked on socket release

- timeout_work may deadlock under certain conditions

- correctness of iso_conn_del() vs UAF requires somewhat too complex
  reasoning about race conditions, and it's not quite correct

        [Task 1]                      [Task hdev->workqueue]
        iso_sock_timeout              iso_conn_del
          iso_conn_hold_unless_zero     iso_chan_del
                           `------------> iso_conn_put
                                      caller frees hcon
          iso_conn_put
            iso_conn_free
              conn->hcon->iso_data = NULL; /* UAF */

- attempt to free iso_conn in iso_conn_del() races with iso_conn_del
  with potential UAF

- iso_sock_ready() gets conn->sk without requisite locks, this should
  be done like in sco.c

- iso_sock_getname, iso_connect_ind are missing lock_sock and NULL
  checks

- kref_get_unless_zero(&((struct iso_conn *)hcon->iso_data)->ref)
  requires synchronization primitive.

Tested vs iso-tester, Pipewire Qemu ucast/bcast audio tests, and real
ucast audio streaming, which pass without KASAN/locking splats. Also
checked iso-tester produces balanced iso_sock_init / iso_sock_destruct
kprints.

Pauli Virtanen (11):
  Bluetooth: ISO: fix CONNECTED -> CLOSED transition on shutdown/release
  Bluetooth: ISO: lock sk in iso_sock_getname
  Bluetooth: ISO: lock sk in iso_connect_ind
  Bluetooth: ISO: fix timeout vs sync_timeout typo in check_bcast_qos
  Bluetooth: ISO: validate sockaddr_iso first in iso_sock_rebind_bis()
  Bluetooth: ISO: hold sk properly in iso_conn_ready
  Bluetooth: ISO: fix leaking sk after socket release
  Bluetooth: ISO: avoid deadlocks in iso_sock_timeout
  Bluetooth: ISO: ensure no dangling hcon references in iso_conn
  Bluetooth: ISO: fix refcounting of iso_conn
  Bluetooth: ISO: fix race of kfree vs kref_get_unless_zero

 include/net/bluetooth/hci_core.h |   4 +-
 net/bluetooth/hci_conn.c         |   2 +
 net/bluetooth/iso.c              | 274 ++++++++++++++++++++++---------
 3 files changed, 197 insertions(+), 83 deletions(-)

-- 
2.55.0
Re: [PATCH 00/11] Bluetooth: ISO: fix HUP on socket release/shutdown + UAF/locking fixes
Posted by Pauli Virtanen 7 hours ago
pe, 2026-07-24 kello 23:20 +0300, Pauli Virtanen kirjoitti:
> The retracted patch in commit e824c0bbe0ec9 ("Bluetooth: ISO: clear
> iso_data always when detaching conn from hcon") merged in
> bluetooth-next/master broke ISO socket transition to BT_CLOSED so they
> don't send POLLHUP properly any more, as seen in "ISO Disconnect -
> Success" test.

On Sashiko comments:
https://sashiko.dev/#/patchset/cover.1784923689.git.pav%40iki.fi

Patch 3: pre-existing issue, not UAF/locking issue so maybe should be
in separate series.

Patch 9: the comment appears to be the UAF which is fixed by the next
Patch 10 as described in its commit message. This was also pre-existing
(SCO has the same race).

Patch 10: these comments seem confused (no rcu_read_lock in
iso_conn_put in this patch, doesn't seem to notice synchronize_rcu in
hci_conn_hash_del?), but remaining UAF/locking issues should be fixed
by Patch 11.

Patch 11: the comment is about hci_conn_hold/drop refcount when
iso_conn_free and iso_conn_add happen to execute exactly at the same
time, with the same pre-existing hci_conn.

hci_connect_cis/bis don't give a new refcount when the hci_conn is pre-
existing and usable, which should mitigate this, the refcount stays at
1 and becomes owned by the new iso_conn. IIUC it should be OK as it is.


> This is rebased version of and supercedes
> https://lore.kernel.org/linux-bluetooth/fbd9dd573bb1ce5f38370128dd35447e2ddfed9c.1784625576.git.pav@iki.fi/
> 
> The first commit in this series fixes that, and subsequent mostly
> independent commits other minor issues found looking at the ISO socket
> code.
> 
> The last three fix UAF issues. The locking/lifetime of iso_conn
> complicates the code here and the last three patches can be replaced by
> somewhat simpler solution with bigger diff that removes iso_conn
> https://github.com/pv/linux/commit/e4b460b5f7aaba060b958229aaec6b2217642178
> 
> Fixed issues aside BT_CONNECT, these appear to be pre-existing in
> several previous kernel releases:
> 
> - sk is always leaked on socket release
> 
> - timeout_work may deadlock under certain conditions
> 
> - correctness of iso_conn_del() vs UAF requires somewhat too complex
>   reasoning about race conditions, and it's not quite correct
> 
>         [Task 1]                      [Task hdev->workqueue]
>         iso_sock_timeout              iso_conn_del
>           iso_conn_hold_unless_zero     iso_chan_del
>                            `------------> iso_conn_put
>                                       caller frees hcon
>           iso_conn_put
>             iso_conn_free
>               conn->hcon->iso_data = NULL; /* UAF */
> 
> - attempt to free iso_conn in iso_conn_del() races with iso_conn_del
>   with potential UAF
> 
> - iso_sock_ready() gets conn->sk without requisite locks, this should
>   be done like in sco.c
> 
> - iso_sock_getname, iso_connect_ind are missing lock_sock and NULL
>   checks
> 
> - kref_get_unless_zero(&((struct iso_conn *)hcon->iso_data)->ref)
>   requires synchronization primitive.
> 
> Tested vs iso-tester, Pipewire Qemu ucast/bcast audio tests, and real
> ucast audio streaming, which pass without KASAN/locking splats. Also
> checked iso-tester produces balanced iso_sock_init / iso_sock_destruct
> kprints.
> 
> Pauli Virtanen (11):
>   Bluetooth: ISO: fix CONNECTED -> CLOSED transition on shutdown/release
>   Bluetooth: ISO: lock sk in iso_sock_getname
>   Bluetooth: ISO: lock sk in iso_connect_ind
>   Bluetooth: ISO: fix timeout vs sync_timeout typo in check_bcast_qos
>   Bluetooth: ISO: validate sockaddr_iso first in iso_sock_rebind_bis()
>   Bluetooth: ISO: hold sk properly in iso_conn_ready
>   Bluetooth: ISO: fix leaking sk after socket release
>   Bluetooth: ISO: avoid deadlocks in iso_sock_timeout
>   Bluetooth: ISO: ensure no dangling hcon references in iso_conn
>   Bluetooth: ISO: fix refcounting of iso_conn
>   Bluetooth: ISO: fix race of kfree vs kref_get_unless_zero
> 
>  include/net/bluetooth/hci_core.h |   4 +-
>  net/bluetooth/hci_conn.c         |   2 +
>  net/bluetooth/iso.c              | 274 ++++++++++++++++++++++---------
>  3 files changed, 197 insertions(+), 83 deletions(-)

-- 
Pauli Virtanen