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

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