[PATCH v2] Bluetooth: ISO: fix UAF in iso_conn_ready

[PATCH v2] Bluetooth: ISO: fix UAF in iso_conn_ready

[Jiale Yao]

When iso_connect_cfm() calls iso_conn_ready(), the latter reads
conn->sk without holding a reference. A concurrent close() on the
ISO socket triggers iso_sock_kill() which frees sk, leading to a
use-after-free when iso_conn_ready() subsequently accesses the
freed sk via lock_sock() and iso_sock_ready().

This was triggered by a vhci-based PoC that races
LE Setup ISO Data Path (0x206e) command completion against
close() on the ISO socket. The crash manifests as a KASAN
null-ptr-deref in iso_connect_cfm() at offset 0x3c from a NULL
pointer, with the call trace:

 iso_connect_cfm+0x311/0x15f0
 hci_cc_le_setup_iso_path+0x46d/0x6f0
 hci_cmd_complete_evt+0x26d/0x990
 hci_event_packet+0x468/0xb40
 hci_rx_work+0x255/0x6e0

Fix by replacing the bare pointer read with iso_sock_hold(conn)
under iso_conn_lock, which atomically elevates the refcount,
and add sock_put() on the exit path where the hold succeeded.

Fixes: ccf74f2390d60 ("Bluetooth: Add BTPROTO_ISO socket type")
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
V1 -> V2: Re-check conn->sk after acquiring lock_sock to avoid state
          transition on closed socket. Moved conn->sk = NULL in
          iso_sock_kill() under lock_sock protection. Adjusted
          iso_sock_ready() to require caller holds lock_sock with
          lockdep_assert, and added BT_DISCONN/BT_CLOSED state guard.
---
 net/bluetooth/iso.c | 35 ++++++++++++++++++++++++++++-------
 1 file changed, 28 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/iso.c b/net/bluetooth/iso.c
index 2e95a153912c..9a5628f3940e 100644
--- a/net/bluetooth/iso.c
+++ b/net/bluetooth/iso.c
@@ -797,11 +797,13 @@ static void iso_sock_kill(struct sock *sk)
 	BT_DBG("sk %p state %d", sk, sk->sk_state);
 
 	/* Sock is dead, so set conn->sk to NULL to avoid possible UAF */
+	lock_sock(sk);
 	if (iso_pi(sk)->conn) {
 		iso_conn_lock(iso_pi(sk)->conn);
 		iso_pi(sk)->conn->sk = NULL;
 		iso_conn_unlock(iso_pi(sk)->conn);
 	}
+	release_sock(sk);
 
 	/* Kill poor orphan */
 	bt_sock_unlink(&iso_sk_list, sk);
@@ -2037,14 +2039,17 @@ static void iso_sock_ready(struct sock *sk)
 {
 	BT_DBG("sk %p", sk);
 
-	if (!sk)
+	lockdep_assert(lockdep_sock_is_held(sk));
+
+	switch (sk->sk_state) {
+	case BT_DISCONN:
+	case BT_CLOSED:
 		return;
+	}
 
-	lock_sock(sk);
 	iso_sock_clear_timer(sk);
 	sk->sk_state = BT_CONNECTED;
 	sk->sk_state_change(sk);
-	release_sock(sk);
 }
 
 static bool iso_match_big(struct sock *sk, void *data)
@@ -2074,7 +2079,7 @@ static bool iso_match_dst(struct sock *sk, void *data)
 static void iso_conn_ready(struct iso_conn *conn)
 {
 	struct sock *parent = NULL;
-	struct sock *sk = conn->sk;
+	struct sock *sk;
 	struct hci_ev_le_big_sync_established *ev = NULL;
 	struct hci_ev_le_pa_sync_established *ev2 = NULL;
 	struct hci_ev_le_per_adv_report *ev3 = NULL;
@@ -2083,7 +2088,22 @@ static void iso_conn_ready(struct iso_conn *conn)
 
 	BT_DBG("conn %p", conn);
 
+	iso_conn_lock(conn);
+	sk = iso_sock_hold(conn);
+	iso_conn_unlock(conn);
+
 	if (sk) {
+		lock_sock(sk);
+
+		/* conn->sk may have become NULL if racing with sk close, but
+		 * due to held hdev->lock, it can't become different sk.
+		 */
+		if (!conn->sk) {
+			release_sock(sk);
+			sock_put(sk);
+			return;
+		}
+
 		/* Attempt to update source address in case of BIS Sender if
 		 * the advertisement is using a random address.
 		 */
@@ -2096,14 +2116,15 @@ static void iso_conn_ready(struct iso_conn *conn)
 			adv = hci_find_adv_instance(bis->hdev,
 						    bis->iso_qos.bcast.bis);
 			if (adv && bacmp(&adv->random_addr, BDADDR_ANY)) {
-				lock_sock(sk);
 				iso_pi(sk)->src_type = BDADDR_LE_RANDOM;
 				bacpy(&iso_pi(sk)->src, &adv->random_addr);
-				release_sock(sk);
 			}
 		}
 
-		iso_sock_ready(conn->sk);
+		iso_sock_ready(sk);
+
+		release_sock(sk);
+		sock_put(sk);
 	} else {
 		hcon = conn->hcon;
 		if (!hcon)
-- 
2.34.1

RE: [v2] Bluetooth: ISO: fix UAF in iso_conn_ready

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1981 bytes --]

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/project/bluetooth/list/?series=1134987

---Test result---

Test Summary:
CheckPatch                    PASS      0.65 seconds
VerifyFixes                   PASS      0.10 seconds
VerifySignedoff               PASS      0.10 seconds
GitLint                       PASS      0.27 seconds
SubjectPrefix                 PASS      0.10 seconds
BuildKernel                   PASS      25.26 seconds
CheckAllWarning               PASS      28.40 seconds
CheckSparse                   PASS      26.53 seconds
BuildKernel32                 PASS      24.64 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      462.18 seconds
TestRunner_iso-tester         FAIL      94.00 seconds
IncrementalBuild              PASS      23.68 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
Total: 141, Passed: 134 (95.0%), Failed: 7, Not Run: 0

Failed Test Cases
ISO Disconnect - Success                             Timed out    2.500 seconds
ISO Reconnect - Success                              Timed out    1.995 seconds
ISO Reconnect Send and Receive #16 - Success         Timed out    1.995 seconds
ISO Reconnect AC 6(i) - Success                      Timed out    2.372 seconds
ISO Reconnect AC 6(ii) - Success                     Timed out    1.998 seconds
ISO Broadcaster Reconnect - Success                  Timed out    2.792 seconds
ISO Broadcaster Receiver Defer Reconnect - Success   Timed out    2.333 seconds


https://github.com/bluez/bluetooth-next/pull/499

---
Regards,
Linux Bluetooth