Skip to content

Bluetooth: ISO: fix HUP on socket release/shutdown + UAF/locking fixes

Pauli Virtanen linux-bluetooth lkml 1 version Applied

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

  1. v1 Latest11 patches 2 repliesApplied 11/11

Patches in v1 All 11 applied

#Subject and filesSizeRepliesApplied
1/11Bluetooth: ISO: fix CONNECTED -> CLOSED transition on shutdown/releasenet/bluetooth/iso.c+20 -61 replyApplied
2/11Bluetooth: ISO: lock sk in iso_sock_getnamenet/bluetooth/iso.c+4 -00 repliesApplied
3/11Bluetooth: ISO: lock sk in iso_connect_indnet/bluetooth/iso.c+11 -80 repliesApplied
4/11Bluetooth: ISO: fix timeout vs sync_timeout typo in check_bcast_qosnet/bluetooth/iso.c+1 -10 repliesApplied
5/11Bluetooth: ISO: validate sockaddr_iso first in iso_sock_rebind_bis()net/bluetooth/iso.c+3 -30 repliesApplied
6/11Bluetooth: ISO: hold sk properly in iso_conn_readynet/bluetooth/iso.c+23 -110 repliesApplied
7/11Bluetooth: ISO: fix leaking sk after socket releasenet/bluetooth/iso.c+18 -40 repliesApplied
8/11Bluetooth: ISO: avoid deadlocks in iso_sock_timeoutnet/bluetooth/iso.c+29 -310 repliesApplied
9/11Bluetooth: ISO: ensure no dangling hcon references in iso_connnet/bluetooth/iso.c+21 -30 repliesApplied
10/11Bluetooth: ISO: fix refcounting of iso_connnet/bluetooth/iso.c+17 -120 repliesApplied
11/11Bluetooth: ISO: fix race of kfree vs kref_get_unless_zeroinclude/net/bluetooth/hci_core.h net/bluetooth/hci_conn.c net/bluetooth/iso.c+58 -120 repliesApplied

Reviews trailers per patch

No Reviewed-by, Acked-by or Tested-by trailers for v1 yet.

Read the v1 thread

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