Bluetooth: ISO: fix HUP on socket release/shutdown + UAF/locking fixes
Status from this message: This series was applied to bluetooth/bluetooth-next.git (master)
Get series with b4
b4 am [email protected]Versions newest first
Patches in v1 All 11 applied
Reviews trailers per patch
No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.
Cover letter [PATCH 00/11] Bluetooth: ISO: fix HUP on socket release/shutdown + UAF/locking fixes
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(-)
Signature · 1 line
2.55.0