Linux bluetooth development
 help / color / mirror / Atom feed
From: Pauli Virtanen <pav@iki.fi>
To: linux-bluetooth@vger.kernel.org
Cc: Pauli Virtanen <pav@iki.fi>,
	marcel@holtmann.org, luiz.dentz@gmail.com, oss@fourdim.xyz,
	error27@gmail.com, elver@google.com,
	linux-kernel@vger.kernel.org
Subject: [PATCH 02/16] Bluetooth: L2CAP: add l2cap_chan_close_unlocked() and locking helpers
Date: Sat, 29 Aug 2026 17:19:57 +0300	[thread overview]
Message-ID: <fc6ef10f3e62dcd8f1a9eb5b446755bf522b307f.1788013041.git.pav@iki.fi> (raw)
In-Reply-To: <cover.1788013041.git.pav@iki.fi>

l2cap_chan_close() requires holding chan->lock and chan->conn->lock if
associated chan->conn exists, to guard eg. conn->chan_l. Taking the
locks with right ordering requires handling a race condition.

Add helper function l2cap_chan_(un)lock_conn that do the locking right.

Add l2cap_chan_close_unlocked() that does not require locks to be held,
as all callsites do this lock -> close -> unlock pattern.

Link: https://syzkaller.appspot.com/bug?extid=0e4ebcc970728e056324
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---
 include/net/bluetooth/l2cap.h | 17 ++++++++++
 net/bluetooth/l2cap_core.c    | 61 ++++++++++++++++++++++++++++++++---
 2 files changed, 73 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index 43a67562b238..84f557d354ca 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -962,6 +962,8 @@ int l2cap_add_scid(struct l2cap_chan *chan,  __u16 scid);
 
 struct l2cap_chan *l2cap_chan_create(void);
 void l2cap_chan_close(struct l2cap_chan *chan, int reason);
+void l2cap_chan_close_unlocked(struct l2cap_chan *chan, int reason)
+	__must_not_hold(&chan->lock);
 int l2cap_chan_connect(struct l2cap_chan *chan, __le16 psm, u16 cid,
 		       bdaddr_t *dst, u8 dst_type, u16 timeout);
 int l2cap_chan_reconfigure(struct l2cap_chan *chan, __u16 mtu);
@@ -988,4 +990,19 @@ void l2cap_conn_put(struct l2cap_conn *conn);
 int l2cap_register_user(struct l2cap_conn *conn, struct l2cap_user *user);
 void l2cap_unregister_user(struct l2cap_conn *conn, struct l2cap_user *user);
 
+bool l2cap_chan_lock_conn(struct l2cap_chan *chan)
+	__acquires(&chan->lock) __cond_acquires(true, &chan->conn->lock);
+
+/* Release macro for l2cap_chan_lock_conn, so context analysis understands it */
+#define l2cap_chan_unlock_conn(chan, conn_locked)			\
+	({								\
+		struct l2cap_chan *__chan = (chan);			\
+		struct l2cap_conn *__conn = __chan->conn;		\
+		l2cap_chan_unlock(__chan);				\
+		if (conn_locked) {					\
+			mutex_unlock(&__conn->lock);			\
+			l2cap_conn_put(__conn);				\
+		}							\
+	})
+
 #endif /* __L2CAP_H */
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index adcf714ec1ed..58c88e116ddc 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -59,6 +59,7 @@ static void l2cap_tx(struct l2cap_chan *chan, struct l2cap_ctrl *control,
 static void l2cap_retrans_timeout(struct work_struct *work);
 static void l2cap_monitor_timeout(struct work_struct *work);
 static void l2cap_ack_timeout(struct work_struct *work);
+static void __l2cap_chan_close(struct l2cap_chan *chan, int reason);
 
 static inline u8 bdaddr_type(u8 link_type, u8 bdaddr_type)
 {
@@ -422,7 +423,7 @@ static void l2cap_chan_timeout(struct work_struct *work)
 	else
 		reason = ETIMEDOUT;
 
-	l2cap_chan_close(chan, reason);
+	__l2cap_chan_close(chan, reason);
 
 	chan->ops->close(chan);
 
@@ -829,7 +830,7 @@ static void l2cap_chan_connect_reject(struct l2cap_chan *chan)
 	l2cap_send_cmd(conn, chan->ident, L2CAP_CONN_RSP, sizeof(rsp), &rsp);
 }
 
-void l2cap_chan_close(struct l2cap_chan *chan, int reason)
+static void __l2cap_chan_close(struct l2cap_chan *chan, int reason)
 {
 	struct l2cap_conn *conn = chan->conn;
 
@@ -878,8 +879,58 @@ void l2cap_chan_close(struct l2cap_chan *chan, int reason)
 		break;
 	}
 }
+
+void l2cap_chan_close(struct l2cap_chan *chan, int reason)
+{
+	__l2cap_chan_close(chan, reason);
+}
 EXPORT_SYMBOL(l2cap_chan_close);
 
+/* Take chan->lock. If chan->conn is non-NULL, take new reference on it, take
+ * chan->conn->lock, and return true. Otherwise return false.
+ */
+bool l2cap_chan_lock_conn(struct l2cap_chan *chan)
+	__context_unsafe(/* conditional locking */)
+{
+	/* Handle conn->lock > chan->lock ordering + race on chan->conn */
+	for (;;) {
+		struct l2cap_conn *conn;
+
+		l2cap_chan_lock(chan);
+		conn = chan->conn;
+		if (conn)
+			l2cap_conn_get(conn);
+		l2cap_chan_unlock(chan);
+
+		if (conn)
+			mutex_lock(&conn->lock);
+
+		l2cap_chan_lock(chan);
+
+		if (chan->conn != conn) {
+			l2cap_chan_unlock(chan);
+			if (conn) {
+				mutex_unlock(&conn->lock);
+				l2cap_conn_put(conn);
+			}
+			schedule();
+			continue;
+		}
+
+		return chan->conn;
+	}
+}
+
+void l2cap_chan_close_unlocked(struct l2cap_chan *chan, int reason)
+{
+	bool have_conn;
+
+	have_conn = l2cap_chan_lock_conn(chan);
+	__l2cap_chan_close(chan, reason);
+	l2cap_chan_unlock_conn(chan, have_conn);
+}
+EXPORT_SYMBOL(l2cap_chan_close_unlocked);
+
 static inline u8 l2cap_get_auth_type(struct l2cap_chan *chan)
 {
 	switch (chan->chan_type) {
@@ -1563,7 +1614,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 			if (!l2cap_mode_supported(chan->mode, conn->feat_mask)
 			    && test_bit(CONF_STATE2_DEVICE,
 					&chan->conf_state)) {
-				l2cap_chan_close(chan, ECONNRESET);
+				__l2cap_chan_close(chan, ECONNRESET);
 				l2cap_chan_unlock(chan);
 				continue;
 			}
@@ -1571,7 +1622,7 @@ static void l2cap_conn_start(struct l2cap_conn *conn)
 			if (l2cap_check_enc_key_size(conn->hcon, chan))
 				l2cap_start_connection(chan);
 			else
-				l2cap_chan_close(chan, ECONNREFUSED);
+				__l2cap_chan_close(chan, ECONNREFUSED);
 
 		} else if (chan->state == BT_CONNECT2) {
 			struct l2cap_conn_rsp rsp;
@@ -7648,7 +7699,7 @@ static inline void l2cap_check_encryption(struct l2cap_chan *chan, u8 encrypt)
 			__set_chan_timer(chan, L2CAP_ENC_TIMEOUT);
 		} else if (chan->sec_level == BT_SECURITY_HIGH ||
 			   chan->sec_level == BT_SECURITY_FIPS)
-			l2cap_chan_close(chan, ECONNREFUSED);
+			__l2cap_chan_close(chan, ECONNREFUSED);
 	} else {
 		if (chan->sec_level == BT_SECURITY_MEDIUM)
 			__clear_chan_timer(chan);
-- 
2.55.0


  parent reply	other threads:[~2026-08-29 14:20 UTC|newest]

Thread overview: 21+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-29 14:19 [PATCH 00/16] Bluetooth: L2CAP: fix and annotate l2cap_conn::chan_l locking Pauli Virtanen
2026-08-29 14:19 ` [PATCH 01/16] Bluetooth: L2CAP: take chan->lock for l2cap_chan_add/ready/del Pauli Virtanen
2026-08-29 15:45   ` Bluetooth: L2CAP: fix and annotate l2cap_conn::chan_l locking bluez.test.bot
2026-08-29 14:19 ` Pauli Virtanen [this message]
2026-08-29 14:19 ` [PATCH 03/16] Bluetooth: L2CAP: fix race condition in l2cap_sock_shutdown() Pauli Virtanen
2026-08-29 14:19 ` [PATCH 04/16] Bluetooth: 6lowpan: use l2cap_chan_close_unlocked() Pauli Virtanen
2026-08-29 14:20 ` [PATCH 05/16] Bluetooth: L2CAP: remove unused l2cap_chan_close() Pauli Virtanen
2026-08-29 14:20 ` [PATCH 06/16] Bluetooth: 6lowpan: avoid concurrent peer_del() in bt_6lowpan_disconnect Pauli Virtanen
2026-08-29 14:20 ` [PATCH 07/16] Bluetooth: L2CAP: hold conn->lock for __l2cap_ecred_conn_rsp_defer Pauli Virtanen
2026-08-29 14:20 ` [PATCH 08/16] Bluetooth: L2CAP: hold l2cap_conn::lock in l2cap_connect_cfm() Pauli Virtanen
2026-08-29 14:20 ` [PATCH 09/16] Bluetooth: L2CAP: add annotations for l2cap_chan list locking Pauli Virtanen
2026-08-29 14:20 ` [PATCH 10/16] Bluetooth: L2CAP: take lock for l2cap_chan_del in l2cap_ecred_rsp_defer Pauli Virtanen
2026-08-29 14:20 ` [PATCH 11/16] Bluetooth: L2CAP: hold chan in l2cap_ecred_conn_rsp() Pauli Virtanen
2026-08-29 14:20 ` [PATCH 12/16] Bluetooth: L2CAP: annotate locking for l2cap_chan_del() Pauli Virtanen
2026-08-29 14:20 ` [PATCH 13/16] Bluetooth: L2CAP: annotate locking for l2cap_ops callbacks Pauli Virtanen
2026-08-29 14:20 ` [PATCH 14/16] Bluetooth: L2CAP: make concurrent l2cap_set_timer() refcounting safe Pauli Virtanen
2026-08-29 14:20 ` [PATCH 15/16] Bluetooth: L2CAP: remove conditional locking from l2cap_connect() Pauli Virtanen
2026-08-29 14:20 ` [PATCH 16/16] Bluetooth: L2CAP: refuse __l2cap_chan_add if chan already has conn Pauli Virtanen
2026-09-01 19:10 ` [PATCH 00/16] Bluetooth: L2CAP: fix and annotate l2cap_conn::chan_l locking patchwork-bot+bluetooth
2026-09-01 19:18   ` Luiz Augusto von Dentz
2026-09-02 21:30 ` 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=fc6ef10f3e62dcd8f1a9eb5b446755bf522b307f.1788013041.git.pav@iki.fi \
    --to=pav@iki.fi \
    --cc=elver@google.com \
    --cc=error27@gmail.com \
    --cc=linux-bluetooth@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=luiz.dentz@gmail.com \
    --cc=marcel@holtmann.org \
    --cc=oss@fourdim.xyz \
    /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