mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Pauli Virtanen <pauli.virtanen@iki.fi>
To: linux-bluetooth@vger.kernel.org
Cc: marcel@holtmann.org, luiz.dentz@gmail.com, linux-kernel@vger.kernel.org
Subject: Re: [PATCH 00/11] Bluetooth: ISO: fix HUP on socket release/shutdown + UAF/locking fixes
Date: Sat, 25 Jul 2026 01:10:59 +0300	[thread overview]
Message-ID: <b9341fe78dbe1ddd550b5099e6c87006fdb4b3e5.camel@iki.fi> (raw)
In-Reply-To: <cover.1784923689.git.pav@iki.fi>

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

  parent reply	other threads:[~2026-07-24 22:11 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-07-24 20:20 Pauli Virtanen
2026-07-24 20:20 ` [PATCH 01/11] Bluetooth: ISO: fix CONNECTED -> CLOSED transition on shutdown/release Pauli Virtanen
2026-07-24 20:20 ` [PATCH 02/11] Bluetooth: ISO: lock sk in iso_sock_getname Pauli Virtanen
2026-07-24 20:20 ` [PATCH 03/11] Bluetooth: ISO: lock sk in iso_connect_ind Pauli Virtanen
2026-07-24 20:20 ` [PATCH 04/11] Bluetooth: ISO: fix timeout vs sync_timeout typo in check_bcast_qos Pauli Virtanen
2026-07-24 20:20 ` [PATCH 05/11] Bluetooth: ISO: validate sockaddr_iso first in iso_sock_rebind_bis() Pauli Virtanen
2026-07-24 20:20 ` [PATCH 06/11] Bluetooth: ISO: hold sk properly in iso_conn_ready Pauli Virtanen
2026-07-24 20:20 ` [PATCH 07/11] Bluetooth: ISO: fix leaking sk after socket release Pauli Virtanen
2026-07-24 20:20 ` [PATCH 08/11] Bluetooth: ISO: avoid deadlocks in iso_sock_timeout Pauli Virtanen
2026-07-24 20:20 ` [PATCH 09/11] Bluetooth: ISO: ensure no dangling hcon references in iso_conn Pauli Virtanen
2026-07-24 20:20 ` [PATCH 10/11] Bluetooth: ISO: fix refcounting of iso_conn Pauli Virtanen
2026-07-24 20:20 ` [PATCH 11/11] Bluetooth: ISO: fix race of kfree vs kref_get_unless_zero Pauli Virtanen
2026-07-24 22:10 ` Pauli Virtanen [this message]
2026-07-27 17:10 ` [PATCH 00/11] Bluetooth: ISO: fix HUP on socket release/shutdown + UAF/locking fixes patchwork-bot+bluetooth

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=b9341fe78dbe1ddd550b5099e6c87006fdb4b3e5.camel@iki.fi \
    --to=pauli.virtanen@iki.fi \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®