Bluetooth: ISO: Serialize concurrent connect calls
Subsystems (3)
PATCH 1/1 Bluetooth: ISO: Serialize concurrent connect calls
iso_sock_connect() checks the socket state before taking the socket lock and drops the lock again before setting up a connection. Two callers can both pass the admission check while the socket is open or bound.
Caller A can copy its destination for route selection, then caller B can replace the socket destination before A binds or connects the CIS. A then uses B's destination with the route selected for its own request. B can also proceed after A attaches a connection and sets BT_CONNECT. For deferred BIS setup, B can bind a second BIS and overwrite iso_pi(sk)->conn, leaving the first connection's reference and conn->sk back-pointer stranded. Concurrent CIS connects can likewise replace the socket's connection. Later teardown only detaches the current connection, so a callback on the old connection can race with socket release and access a freed socket.
KASAN reported:
BUG: KASAN: slab-use-after-free in iso_sock_hold+0xf7/0x1b0 Call Trace: iso_sock_hold+0xf7/0x1b0 iso_conn_del+0x7b/0x1d0 iso_connect_cfm+0x186/0x16a0 hci_conn_failed+0x154/0x280 hci_abort_conn_sync+0x3dc/0x7d0 hci_cmd_sync_work+0x173/0x300 Allocated by task 121: sk_alloc+0x2b/0x6d0 bt_sock_alloc+0x29/0x370 iso_sock_alloc.constprop.0+0x19/0x300 iso_sock_create+0x94/0x100 Freed by task 121: kfree+0x121/0x3c0 __sk_destruct+0x42b/0x540 iso_sock_release+0x29d/0x340 __sock_release+0xa1/0x260
Check admission under the socket lock and mark the connect operation in progress before publishing its destination. Keep that flag set across route lookup and connection setup, rejecting another connect with -EBADFD before it can change the destination or attach a connection. Clear the flag after either helper returns, including on failure, so a failed setup can be retried.
A separate flag is needed because the socket lock must be released before acquiring the HCI device lock, while BT_CONNECT requires an attached connection. Keep the existing state transitions and the deferred CIS completion through iso_sock_recvmsg().
--- net/bluetooth/iso.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-)
net/bluetooth/iso.c +22 -10
@@ -61,6 +61,7 @@ enum {BT_SK_BIG_SYNC ,BT_SK_PA_SYNC ,BT_SK_KILLED , + BT_SK_CONNECTING, }; struct iso_pinfo {
@@ -1269,17 +1270,26 @@ static int iso_sock_connect(struct socket *sock, struct sockaddr_unsized *addr, addr->sa_family != AF_BLUETOOTH)return -EINVAL ; -if (sk->sk_state !=BT_OPEN && sk->sk_state !=BT_BOUND ) -return -EBADFD ; +lock_sock (sk); -if (sk->sk_type !=SOCK_SEQPACKET ) -return -EINVAL ; +if ((sk->sk_state !=BT_OPEN && sk->sk_state !=BT_BOUND ) || +test_bit (BT_SK_CONNECTING , &iso_pi (sk)->flags)) { + err = -EBADFD ; + goto done; + } + +if (sk->sk_type !=SOCK_SEQPACKET ) { + err = -EINVAL ; + goto done; + }/* Check if the address type is of LE type */ -if (!bdaddr_type_is_le (sa->iso_bdaddr_type)) -return -EINVAL ; +if (!bdaddr_type_is_le (sa->iso_bdaddr_type)) { + err = -EINVAL ; + goto done; + } -lock_sock (sk); +set_bit (BT_SK_CONNECTING , &iso_pi (sk)->flags);bacpy (&iso_pi (sk)->dst, &sa->iso_bdaddr);iso_pi (sk)->dst_type = sa->iso_bdaddr_type;
@@ -1291,16 +1301,18 @@ static int iso_sock_connect(struct socket *sock, struct sockaddr_unsized *addr,else err =iso_connect_bis (sk); -if (err) -return err; -lock_sock (sk); +clear_bit (BT_SK_CONNECTING , &iso_pi (sk)->flags); +if (err) + goto done; +if (!test_bit (BT_SK_DEFER_SETUP , &bt_sk (sk)->flags)) { err =bt_sock_wait_state (sk,BT_CONNECTED ,sock_sndtimeo (sk, flags &O_NONBLOCK )); } +done:release_sock (sk);return err; }
Signature · 1 line
2.43.0
Hi,
ma, 2026-10-05 kello 00:24 +0800, Chengfeng Ye kirjoitti:
Quoted 11 lines
iso_sock_connect()checks the socket state before taking the socket lock and drops the lock again before setting up a connection. Two callers can both pass the admission check while the socket is open or bound.Caller A can copy its destination for route selection, then caller B can replace the socket destination before A binds or connects the CIS. A then uses B's destination with the route selected for its own request. B can also proceed after A attaches a connection and sets
BT_CONNECT. For deferred BIS setup, B can bind a second BIS and overwrite iso_pi(sk)->conn, leaving the first connection's reference andconn->skback-pointer stranded. Concurrent CIS connects can likewise replace the
Memory safety probably should be enforced somewhat down the calls, likely __iso_chan_add() should reject adding a different iso_conn to sk if it already has one, since that looks like it leaks the iso_conn_hold() reference and the back pointer association.
Quoted 115 lines
socket's connection. Later teardown only detaches the current connection, so a callback on the old connection can race with socket release and access a freed socket.
KASAN reported:
BUG: KASAN: slab-use-after-free in iso_sock_hold+0xf7/0x1b0 Call Trace: iso_sock_hold+0xf7/0x1b0 iso_conn_del+0x7b/0x1d0 iso_connect_cfm+0x186/0x16a0 hci_conn_failed+0x154/0x280 hci_abort_conn_sync+0x3dc/0x7d0 hci_cmd_sync_work+0x173/0x300 Allocated by task 121: sk_alloc+0x2b/0x6d0 bt_sock_alloc+0x29/0x370 iso_sock_alloc.constprop.0+0x19/0x300 iso_sock_create+0x94/0x100 Freed by task 121: kfree+0x121/0x3c0 __sk_destruct+0x42b/0x540 iso_sock_release+0x29d/0x340 __sock_release+0xa1/0x260Check admission under the socket lock and mark the connect operation in progress before publishing its destination. Keep that flag set across route lookup and connection setup, rejecting another connect with
-EBADFDbefore it can change the destination or attach a connection. Clear the flag after either helper returns, including on failure, so a failed setup can be retried.A separate flag is needed because the socket lock must be released before acquiring the HCI device lock, while
BT_CONNECTrequires an attached connection. Keep the existing state transitions and the deferred CIS completion throughiso_sock_recvmsg().Fixes ccf74f2390d6 ("Bluetooth: Add BTPROTO_ISO socket type")Assisted-by GPT-6 AstraSigned-off-by Chengfeng Ye <[email protected]>--- net/bluetooth/iso.c | 32 ++++++++++++++++++++++---------- 1 file changed, 22 insertions(+), 10 deletions(-)net/bluetooth/iso.c +22
-10@@ -61,6 +61,7 @@ enum {BT_SK_BIG_SYNC ,BT_SK_PA_SYNC ,BT_SK_KILLED , + BT_SK_CONNECTING, }; struct iso_pinfo {@@ -1269,17 +1270,26 @@ static int iso_sock_connect(struct socket *sock, struct sockaddr_unsized *addr, addr->sa_family != AF_BLUETOOTH)return -EINVAL ; -if (sk->sk_state !=BT_OPEN && sk->sk_state !=BT_BOUND ) -return -EBADFD ; +lock_sock (sk); -if (sk->sk_type !=SOCK_SEQPACKET ) -return -EINVAL ; +if ((sk->sk_state !=BT_OPEN && sk->sk_state !=BT_BOUND ) || +test_bit (BT_SK_CONNECTING , &iso_pi (sk)->flags)) { + err = -EBADFD ; + goto done; + } + +if (sk->sk_type !=SOCK_SEQPACKET ) { + err = -EINVAL ; + goto done; + }/* Check if the address type is of LE type */ -if (!bdaddr_type_is_le (sa->iso_bdaddr_type)) -return -EINVAL ; +if (!bdaddr_type_is_le (sa->iso_bdaddr_type)) { + err = -EINVAL ; + goto done; + } -lock_sock (sk); +set_bit (BT_SK_CONNECTING , &iso_pi (sk)->flags);bacpy (&iso_pi (sk)->dst, &sa->iso_bdaddr);iso_pi (sk)->dst_type = sa->iso_bdaddr_type;@@ -1291,16 +1301,18 @@ static int iso_sock_connect(struct socket *sock, struct sockaddr_unsized *addr,else err =iso_connect_bis (sk); -if (err) -return err; -lock_sock (sk); +clear_bit (BT_SK_CONNECTING , &iso_pi (sk)->flags); +if (err) + goto done; +if (!test_bit (BT_SK_DEFER_SETUP , &bt_sk (sk)->flags)) { err =bt_sock_wait_state (sk,BT_CONNECTED ,sock_sndtimeo (sk, flags &O_NONBLOCK )); } +done:release_sock (sk);return err; }
Signature · 1 line
Pauli Virtanen
Quoted 16 lines · Pauli Virtanen
Hi,
ma, 2026-10-05 kello 00:24 +0800, Chengfeng Ye kirjoitti:
Quoted 11 lines
iso_sock_connect()checks the socket state before taking the socket lock and drops the lock again before setting up a connection. Two callers can both pass the admission check while the socket is open or bound.Caller A can copy its destination for route selection, then caller B can replace the socket destination before A binds or connects the CIS. A then uses B's destination with the route selected for its own request. B can also proceed after A attaches a connection and sets
BT_CONNECT. For deferred BIS setup, B can bind a second BIS and overwrite iso_pi(sk)->conn, leaving the first connection's reference andconn->skback-pointer stranded. Concurrent CIS connects can likewise replace the
Quoted 4 lines
Memory safety probably should be enforced somewhat down the calls, likely
__iso_chan_add()should reject adding a differentiso_connto sk if it already has one, since that looks like it leaks theiso_conn_hold()reference and the back pointer association.
Hi Pauli,
Thanks for the review. I have just sent v2 with the check in __iso_chan_add().
I also adjusted the outgoing BIS and CIS callers to handle conflicting attachments early. The BIS check prevents modifying a reusable connection before attachment is rejected. The CIS check releases the HCI hold acquired by the current attempt on rejection, while preserving same-connection reuse for deferred setup.
Best regards,
Chengfeng
This is automated email and please do not reply to this email!
Dear submitter,
Thank you for submitting the patches to the linux bluetooth mailing list. This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/series/1179169/
---Test result---
Test Summary:
CheckPatch PASS 0.54 seconds VerifyFixes PASS 0.11 seconds VerifySignedoff PASS 0.11 seconds GitLint PASS 0.25 seconds SubjectPrefix PASS 0.10 seconds BuildKernel PASS 17.71 seconds CheckAllWarning PASS 20.39 seconds CheckSparse PASS 21.54 seconds BuildKernel32 PASS 17.60 seconds CheckKernelLLVM PASS 18.90 seconds TestRunnerSetup PASS 430.23 seconds TestRunner_iso-tester PASS 11.48 seconds IncrementalBuild PASS 17.02 seconds
https://github.com/bluez/bluetooth-next/pull/856
---
Regards,
Linux Bluetooth