[PATCH v7 RESEND 1/1] Bluetooth: L2CAP: Fix slab-use-after-free in l2cap_sock_cleanup_listen()

[Siwei Zhang <oss@fourdim.xyz>]

l2cap_sock_cleanup_listen() calls l2cap_chan_close() without holding
conn->lock. A concurrent task iterating conn->chan_l under conn->lock
can access a channel that has been removed from the list and freed.
That can result in a slab-use-after-free.

Split cleanup into two phases. Drain the accept queue under the parent's
sk_lock onto a local list, taking a sock reference on each child so it
survives the lock drop. Then release the parent and close every drained
child under conn->lock + chan_lock, using
l2cap_chan_hold_unless_zero()/l2cap_conn_hold_unless_zero() to cope with
a teardown that has already started, and skipping any chan whose
->data has been cleared. Reacquire the parent's sk_lock at the end so
the caller's contract is preserved.

Noted that commit ab4eedb790ca
("Bluetooth: L2CAP: Fix corrupted list in hci_chan_del")
renamed chan_lock to lock in l2cap_conn.

Fixes: 3df91ea20e74 ("Bluetooth: Revert to mutexes from RCU list")
Cc: stable@kernel.org
Assisted-by: Claude:claude-opus-4-7
Signed-off-by: Siwei Zhang <oss@fourdim.xyz>
---
 net/bluetooth/l2cap_sock.c | 57 ++++++++++++++++++++++++++++++++------
 1 file changed, 49 insertions(+), 8 deletions(-)

diff --git a/net/bluetooth/l2cap_sock.c b/net/bluetooth/l2cap_sock.c
index 71e8c1b45bce..48b018991ced 100644
--- a/net/bluetooth/l2cap_sock.c
+++ b/net/bluetooth/l2cap_sock.c
@@ -1471,27 +1471,68 @@ static int l2cap_sock_release(struct socket *sock)
 static void l2cap_sock_cleanup_listen(struct sock *parent)
 {
 	struct sock *sk;
+	struct bt_sock *bs, *tmp;
+	LIST_HEAD(killable);
 
 	BT_DBG("parent %p state %s", parent,
 	       state_to_string(parent->sk_state));
 
-	/* Close not yet accepted channels */
+	/* Drain unaccepted children to a local list, pinning each so it
+	 * survives the parent lock-drop below.
+	 */
 	while ((sk = bt_accept_dequeue(parent, NULL))) {
-		struct l2cap_chan *chan = l2cap_pi(sk)->chan;
+		sock_hold(sk);
+		list_add_tail(&bt_sk(sk)->accept_q, &killable);
+	}
 
-		BT_DBG("child chan %p state %s", chan,
-		       state_to_string(chan->state));
+	/* l2cap_chan_close() must run under conn->lock, but the rx path
+	 * (l2cap_sock_new_connection_cb) takes conn->lock then lock_sock(parent),
+	 * so parent must be released before we close.  Draining the queue
+	 * first means a concurrent cleanup_listen() on the same parent finds
+	 * it empty and is a no-op.
+	 */
+	release_sock(parent);
+
+	list_for_each_entry_safe(bs, tmp, &killable, accept_q) {
+		struct l2cap_chan *chan;
+		struct l2cap_conn *conn;
+
+		sk = (struct sock *)bs;
+		list_del_init(&bs->accept_q);
+
+		chan = l2cap_chan_hold_unless_zero(l2cap_pi(sk)->chan);
+		if (!chan) {
+			sock_put(sk);
+			continue;
+		}
 
-		l2cap_chan_hold(chan);
+		l2cap_chan_lock(chan);
+		conn = l2cap_conn_hold_unless_zero(chan->conn);
+		l2cap_chan_unlock(chan);
+
+		if (conn)
+			mutex_lock(&conn->lock);
 		l2cap_chan_lock(chan);
 
-		__clear_chan_timer(chan);
-		l2cap_chan_close(chan, ECONNRESET);
-		l2cap_sock_kill(sk);
+		BT_DBG("child chan %p state %s", chan,
+		       state_to_string(chan->state));
+
+		if (chan->data) {
+			__clear_chan_timer(chan);
+			l2cap_chan_close(chan, ECONNRESET);
+			l2cap_sock_kill(sk);
+		}
 
 		l2cap_chan_unlock(chan);
+		if (conn) {
+			mutex_unlock(&conn->lock);
+			l2cap_conn_put(conn);
+		}
 		l2cap_chan_put(chan);
+		sock_put(sk);
 	}
+
+	lock_sock_nested(parent, L2CAP_NESTING_PARENT);
 }
 
 static struct l2cap_chan *l2cap_sock_new_connection_cb(struct l2cap_chan *chan)
-- 
2.54.0