Re: [PATCH] Bluetooth: L2CAP: access chan->conn safely in get/setsockopt

Re: [PATCH] Bluetooth: L2CAP: access chan->conn safely in get/setsockopt

[syzbot]

> Since commit b66774b48dd9 ("Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref")
> l2cap_chan::conn has held reference and remains non-NULL also after the
> corresponding hci_conn is deleted.  In this state accessing various
> fields eg. hci_conn::hdev is invalid, which leads to KASAN crash in
> l2cap_sock_setsockopt() access of conn->hcon->hdev.
>
> Check l2cap_chan::conn.hcon corresponds to an alive hci_conn before
> trying to use it in l2cap_sock.c.  This can be guaranteed by
> synchronizing with l2cap_sock_teardown_cb().  Make l2cap_chan::conn
> readable without l2cap_chan::lock, so we can access with only sk lock.
> Also move sock_set_flag() inside lock_sock() since it is not atomic.
>
> Fixes: b66774b48dd9 ("Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref")
> Reported-by: syzbot+b106284c2a0b7bc80cf9@syzkaller.appspotmail.com
> ---
>
> #syz test

This crash does not have a reproducer. I cannot test it.

>
>  net/bluetooth/l2cap_core.c |  2 +-
>  net/bluetooth/l2cap_sock.c | 64 ++++++++++++++++++++++++++++----------
>  2 files changed, 48 insertions(+), 18 deletions(-)
>
> diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
> index ee459dd411f5..7327ebb1e710 100644
> --- a/net/bluetooth/l2cap_core.c
> +++ b/net/bluetooth/l2cap_core.c
> @@ -624,7 +624,7 @@ void __l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
>  
>  	conn->disc_reason = HCI_ERROR_REMOTE_USER_TERM;
>  
> -	chan->conn = l2cap_conn_get(conn);
> +	WRITE_ONCE(chan->conn, l2cap_conn_get(conn));
>  
>  	switch (chan->chan_type) {
>  	case L2CAP_CHAN_CONN_ORIENTED:
> diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
> index 735167f73f31..c3d9f7721963 100644
> --- a/net/bluetooth/l2cap_sock.c
> +++ b/net/bluetooth/l2cap_sock.c
> @@ -436,11 +436,33 @@ static int l2cap_get_mode(struct l2cap_chan *chan)
>  	return -EINVAL;
>  }
>  
> +static struct l2cap_conn *l2cap_sock_conn(struct sock *sk)
> +{
> +	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
> +	struct l2cap_conn *conn = READ_ONCE(chan->conn);
> +
> +	lockdep_assert(lockdep_sock_is_held(sk));
> +
> +	/* chan holds refcount on conn during its lifetime, so if non-NULL
> +	 * observed, it is valid.
> +	 *
> +	 * conn holds refcount on conn->hcon, but the hci_conn may be in deleted
> +	 * state.  l2cap_sock_teardown_cb() is called before associated hci_conn
> +	 * is deleted, so it is alive if sk is not zapped, as long as sk lock is
> +	 * held.
> +	 */
> +	if (sock_flag(sk, SOCK_ZAPPED))
> +		return NULL;
> +
> +	return conn;
> +}
> +
>  static int l2cap_sock_getsockopt_old(struct socket *sock, int optname,
>  				     sockopt_t *sopt)
>  {
>  	struct sock *sk = sock->sk;
>  	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
> +	struct l2cap_conn *conn;
>  	struct l2cap_options opts;
>  	struct l2cap_conninfo cinfo;
>  	int err = 0;
> @@ -537,9 +559,15 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname,
>  			break;
>  		}
>  
> +		conn = l2cap_sock_conn(sk);
> +		if (!conn) {
> +			err = -ENOTCONN;
> +			break;
> +		}
> +
>  		memset(&cinfo, 0, sizeof(cinfo));
> -		cinfo.hci_handle = chan->conn->hcon->handle;
> -		memcpy(cinfo.dev_class, chan->conn->hcon->dev_class, 3);
> +		cinfo.hci_handle = conn->hcon->handle;
> +		memcpy(cinfo.dev_class, conn->hcon->dev_class, 3);
>  
>  		len = min(len, sizeof(cinfo));
>  		if (copy_to_iter(&cinfo, len, &sopt->iter_out) != len)
> @@ -561,6 +589,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
>  {
>  	struct sock *sk = sock->sk;
>  	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
> +	struct l2cap_conn *conn;
>  	struct bt_security sec;
>  	struct bt_power pwr;
>  	int len, mode, err = 0;
> @@ -589,12 +618,14 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
>  			break;
>  		}
>  
> +		conn = l2cap_sock_conn(sk);
> +
>  		memset(&sec, 0, sizeof(sec));
> -		if (chan->conn) {
> -			sec.level = chan->conn->hcon->sec_level;
> +		if (conn) {
> +			sec.level = conn->hcon->sec_level;
>  
>  			if (sk->sk_state == BT_CONNECTED)
> -				sec.key_size = chan->conn->hcon->enc_key_size;
> +				sec.key_size = conn->hcon->enc_key_size;
>  		} else {
>  			sec.level = chan->sec_level;
>  		}
> @@ -678,12 +709,14 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
>  		break;
>  
>  	case BT_PHY:
> -		if (sk->sk_state != BT_CONNECTED) {
> +		conn = l2cap_sock_conn(sk);
> +
> +		if (sk->sk_state != BT_CONNECTED || !conn) {
>  			err = -ENOTCONN;
>  			break;
>  		}
>  
> -		opt = hci_conn_get_phy(chan->conn->hcon);
> +		opt = hci_conn_get_phy(conn->hcon);
>  
>  		if (copy_to_iter(&opt, sizeof(opt), &sopt->iter_out) !=
>  		    sizeof(opt))
> @@ -938,11 +971,10 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>  
>  		chan->sec_level = sec.level;
>  
> -		if (!chan->conn)
> +		conn = l2cap_sock_conn(sk);
> +		if (!conn)
>  			break;
>  
> -		conn = chan->conn;
> -
>  		/* change security for LE channels */
>  		if (chan->scid == L2CAP_CID_ATT) {
>  			if (smp_conn_security(conn->hcon, sec.level)) {
> @@ -997,7 +1029,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>  		}
>  
>  		if (opt == BT_FLUSHABLE_OFF) {
> -			conn = chan->conn;
> +			conn = l2cap_sock_conn(sk);
> +
>  			/* proceed further only when we have l2cap_conn and
>  			   No Flush support in the LM */
>  			if (!conn || !lmp_no_flush_capable(conn->hcon->hdev)) {
> @@ -1083,7 +1116,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>  		break;
>  
>  	case BT_PHY:
> -		if (sk->sk_state != BT_CONNECTED) {
> +		conn = l2cap_sock_conn(sk);
> +		if (sk->sk_state != BT_CONNECTED || !conn) {
>  			err = -ENOTCONN;
>  			break;
>  		}
> @@ -1093,10 +1127,6 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
>  		if (err)
>  			break;
>  
> -		if (!chan->conn)
> -			break;
> -
> -		conn = chan->conn;
>  		err = hci_conn_set_phy(conn->hcon, phys);
>  		break;
>  
> @@ -1716,11 +1746,11 @@ static void l2cap_sock_teardown_cb(struct l2cap_chan *chan, int err)
>  
>  		break;
>  	}
> -	release_sock(sk);
>  
>  	/* Only zap after cleanup to avoid use after free race */
>  	sock_set_flag(sk, SOCK_ZAPPED);
>  
> +	release_sock(sk);
>  }
>  
>  static void l2cap_sock_state_change_cb(struct l2cap_chan *chan, int state,
> -- 
> 2.55.0
>

[PATCH] Bluetooth: L2CAP: access chan->conn safely in get/setsockopt

[Pauli Virtanen]

Since commit b66774b48dd9 ("Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref")
l2cap_chan::conn has held reference and remains non-NULL also after the
corresponding hci_conn is deleted.  In this state accessing various
fields eg. hci_conn::hdev is invalid, which leads to KASAN crash in
l2cap_sock_setsockopt() access of conn->hcon->hdev.

Check l2cap_chan::conn.hcon corresponds to an alive hci_conn before
trying to use it in l2cap_sock.c.  Hold l2cap_chan_lock() in
getsockopt/setsockopt to ensure it stays alive, and to avoid data races
in l2cap_chan fields.

Fixes: b66774b48dd9 ("Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref")
Reported-by: syzbot+b106284c2a0b7bc80cf9@syzkaller.appspotmail.com
Link: https://syzkaller.appspot.com/bug?extid=b106284c2a0b7bc80cf9
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 net/bluetooth/l2cap_sock.c | 64 ++++++++++++++++++++++++++++----------
 1 file changed, 48 insertions(+), 16 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 735167f73f31..cca6201f9cdb 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -436,11 +436,26 @@ static int l2cap_get_mode(struct l2cap_chan *chan)
 	return -EINVAL;
 }
 
+static struct l2cap_conn *l2cap_chan_conn(struct l2cap_chan *chan)
+{
+	lockdep_assert_held(&chan->lock);
+
+	/* l2cap_conn_del() sets FLAG_DEL while holding chan->lock before
+	 * conn->hcon is deleted. If not set and conn is non-NULL, conn->hcon
+	 * remains alive during this chan->lock critical section.
+	 */
+	if (test_bit(FLAG_DEL, &chan->flags))
+		return NULL;
+
+	return chan->conn;
+}
+
 static int l2cap_sock_getsockopt_old(struct socket *sock, int optname,
 				     sockopt_t *sopt)
 {
 	struct sock *sk = sock->sk;
 	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
+	struct l2cap_conn *conn;
 	struct l2cap_options opts;
 	struct l2cap_conninfo cinfo;
 	int err = 0;
@@ -451,6 +466,7 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname,
 
 	len = sopt->optlen;
 
+	l2cap_chan_lock(chan);
 	lock_sock(sk);
 
 	switch (optname) {
@@ -537,9 +553,15 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname,
 			break;
 		}
 
+		conn = l2cap_chan_conn(chan);
+		if (!conn) {
+			err = -ENOTCONN;
+			break;
+		}
+
 		memset(&cinfo, 0, sizeof(cinfo));
-		cinfo.hci_handle = chan->conn->hcon->handle;
-		memcpy(cinfo.dev_class, chan->conn->hcon->dev_class, 3);
+		cinfo.hci_handle = conn->hcon->handle;
+		memcpy(cinfo.dev_class, conn->hcon->dev_class, 3);
 
 		len = min(len, sizeof(cinfo));
 		if (copy_to_iter(&cinfo, len, &sopt->iter_out) != len)
@@ -553,6 +575,8 @@ static int l2cap_sock_getsockopt_old(struct socket *sock, int optname,
 	}
 
 	release_sock(sk);
+	l2cap_chan_unlock(chan);
+
 	return err;
 }
 
@@ -561,6 +585,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
 {
 	struct sock *sk = sock->sk;
 	struct l2cap_chan *chan = l2cap_pi(sk)->chan;
+	struct l2cap_conn *conn;
 	struct bt_security sec;
 	struct bt_power pwr;
 	int len, mode, err = 0;
@@ -578,6 +603,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
 
 	len = sopt->optlen;
 
+	l2cap_chan_lock(chan);
 	lock_sock(sk);
 
 	switch (optname) {
@@ -589,12 +615,14 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
 			break;
 		}
 
+		conn = l2cap_chan_conn(chan);
+
 		memset(&sec, 0, sizeof(sec));
-		if (chan->conn) {
-			sec.level = chan->conn->hcon->sec_level;
+		if (conn) {
+			sec.level = conn->hcon->sec_level;
 
 			if (sk->sk_state == BT_CONNECTED)
-				sec.key_size = chan->conn->hcon->enc_key_size;
+				sec.key_size = conn->hcon->enc_key_size;
 		} else {
 			sec.level = chan->sec_level;
 		}
@@ -678,12 +706,14 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
 		break;
 
 	case BT_PHY:
-		if (sk->sk_state != BT_CONNECTED) {
+		conn = l2cap_chan_conn(chan);
+
+		if (sk->sk_state != BT_CONNECTED || !conn) {
 			err = -ENOTCONN;
 			break;
 		}
 
-		opt = hci_conn_get_phy(chan->conn->hcon);
+		opt = hci_conn_get_phy(conn->hcon);
 
 		if (copy_to_iter(&opt, sizeof(opt), &sopt->iter_out) !=
 		    sizeof(opt))
@@ -719,6 +749,7 @@ static int l2cap_sock_getsockopt(struct socket *sock, int level, int optname,
 	}
 
 	release_sock(sk);
+	l2cap_chan_unlock(chan);
 	return err;
 }
 
@@ -749,6 +780,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname,
 
 	BT_DBG("sk %p", sk);
 
+	l2cap_chan_lock(chan);
 	lock_sock(sk);
 
 	switch (optname) {
@@ -850,6 +882,7 @@ static int l2cap_sock_setsockopt_old(struct socket *sock, int optname,
 	}
 
 	release_sock(sk);
+	l2cap_chan_unlock(chan);
 	return err;
 }
 
@@ -913,6 +946,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 	if (level != SOL_BLUETOOTH)
 		return -ENOPROTOOPT;
 
+	l2cap_chan_lock(chan);
 	lock_sock(sk);
 
 	switch (optname) {
@@ -938,11 +972,10 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 
 		chan->sec_level = sec.level;
 
-		if (!chan->conn)
+		conn = l2cap_chan_conn(chan);
+		if (!conn)
 			break;
 
-		conn = chan->conn;
-
 		/* change security for LE channels */
 		if (chan->scid == L2CAP_CID_ATT) {
 			if (smp_conn_security(conn->hcon, sec.level)) {
@@ -997,7 +1030,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 		}
 
 		if (opt == BT_FLUSHABLE_OFF) {
-			conn = chan->conn;
+			conn = l2cap_chan_conn(chan);
+
 			/* proceed further only when we have l2cap_conn and
 			   No Flush support in the LM */
 			if (!conn || !lmp_no_flush_capable(conn->hcon->hdev)) {
@@ -1083,7 +1117,8 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 		break;
 
 	case BT_PHY:
-		if (sk->sk_state != BT_CONNECTED) {
+		conn = l2cap_chan_conn(chan);
+		if (sk->sk_state != BT_CONNECTED || !conn) {
 			err = -ENOTCONN;
 			break;
 		}
@@ -1093,10 +1128,6 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 		if (err)
 			break;
 
-		if (!chan->conn)
-			break;
-
-		conn = chan->conn;
 		err = hci_conn_set_phy(conn->hcon, phys);
 		break;
 
@@ -1139,6 +1170,7 @@ static int l2cap_sock_setsockopt(struct socket *sock, int level, int optname,
 	}
 
 	release_sock(sk);
+	l2cap_chan_unlock(chan);
 	return err;
 }
 
-- 
2.55.0

RE: Bluetooth: L2CAP: access chan->conn safely in get/setsockopt

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2308 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=1142960

---Test result---

Test Summary:
CheckPatch                    FAIL      1.45 seconds
VerifyFixes                   PASS      0.13 seconds
VerifySignedoff               PASS      0.13 seconds
GitLint                       FAIL      0.33 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      25.87 seconds
CheckAllWarning               PASS      28.73 seconds
CheckSparse                   PASS      40.08 seconds
BuildKernel32                 PASS      30.49 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      464.22 seconds
TestRunner_l2cap-tester       PASS      62.49 seconds
IncrementalBuild              PASS      24.31 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: L2CAP: access chan->conn safely in get/setsockopt
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#86: 
Since commit b66774b48dd9 ("Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref")

total: 0 errors, 1 warnings, 0 checks, 181 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14739667.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: L2CAP: access chan->conn safely in get/setsockopt

3: B1 Line exceeds max length (94>80): "Since commit b66774b48dd9 ("Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref")"
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


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

---
Regards,
Linux Bluetooth

Re: [PATCH] Bluetooth: L2CAP: access chan->conn safely in get/setsockopt

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Sun,  9 Aug 2026 20:42:41 +0300 you wrote:
> Since commit b66774b48dd9 ("Bluetooth: L2CAP: Fix UAF in channel timeout by holding conn ref")
> l2cap_chan::conn has held reference and remains non-NULL also after the
> corresponding hci_conn is deleted.  In this state accessing various
> fields eg. hci_conn::hdev is invalid, which leads to KASAN crash in
> l2cap_sock_setsockopt() access of conn->hcon->hdev.
> 
> Check l2cap_chan::conn.hcon corresponds to an alive hci_conn before
> trying to use it in l2cap_sock.c.  Hold l2cap_chan_lock() in
> getsockopt/setsockopt to ensure it stays alive, and to avoid data races
> in l2cap_chan fields.
> 
> [...]

Here is the summary with links:
  - Bluetooth: L2CAP: access chan->conn safely in get/setsockopt
    https://git.kernel.org/bluetooth/bluetooth-next/c/d1b752f55289

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html