[PATCH 3/5] Bluetooth: L2CAP: drain channel timers on connection teardown

[Cen Zhang]

L2CAP channel timers run on system_percpu_wq. Connection deletion cancels
them without waiting because callbacks may need conn->lock, but HCI
unregister does not drain that workqueue after releasing the lock. A
running callback can therefore outlive HCI-driver unregister. If it drops
the last channel reference and releases a protocol module, it still has
to return through Bluetooth code after those dependencies are gone.

Individual channel deletion has another cancellation race. It cancels
timers before calling the socket teardown callback, which can wait for
sk lock. A concurrent recvmsg holding that lock can clear local busy and
rearm the monitor timer. Deletion then unlinks the channel, so connection
teardown can no longer find that delayed work through the channel list.

Give each connection an ordered timer workqueue. Serialize timer queueing
with channel and connection stop flags, and stop each channel before
canceling its timers or calling teardown. At connection deletion, stop
queueing for the connection, cancel all four timer types, then drop
conn->lock and drain running callbacks before releasing the connection.
This lets callbacks acquire their locks, observe FLAG_DEL and return
before HCI unregister completes.

The channel stop flag also ensures that no unlinked channel can leave
delayed work behind on the new queue. The queue remains allocated until
the drained connection is freed. This adds one workqueue per connection
and serializes that connection's channel timers.

Assisted-by: LLM
Signed-off-by: Cen Zhang <zzzccc427@xxxxxxxxx>
---
 include/net/bluetooth/l2cap.h | 23 ++++++++++++++++++++++-
 net/bluetooth/l2cap_core.c    | 34 ++++++++++++++++++++++++++++++----
 2 files changed, 52 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h
index efb9b7f422d1..b4af087a0a81 100644
--- a/include/net/bluetooth/l2cap.h
+++ b/include/net/bluetooth/l2cap.h
@@ -611,6 +611,7 @@ struct l2cap_chan {
 
 	void			*data;
 	const struct l2cap_ops	*ops;
+	bool			timers_stopped; /* protected by conn->timer_lock */
 	struct mutex		lock;
 };
 
@@ -636,6 +637,10 @@ struct l2cap_conn {
 
 	struct sk_buff_head	pending_rx;
 	struct work_struct	pending_rx_work;
+	struct workqueue_struct	*timer_workqueue;
+	spinlock_t		timer_lock; /* protects timer scheduling */
+
+	bool			timers_stopped __guarded_by(&timer_lock);
 
 	struct delayed_work	id_addr_timer;
 
@@ -856,13 +861,29 @@ static inline void l2cap_chan_unlock(struct l2cap_chan *chan)
 static inline void l2cap_set_timer(struct l2cap_chan *chan,
 				   struct delayed_work *work, long timeout)
 {
+	struct l2cap_conn *conn = chan->conn;
+	unsigned long flags;
+	bool pending;
+
 	BT_DBG("chan %p state %s timeout %ld", chan,
 	       state_to_string(chan->state), timeout);
 
+	if (WARN_ON_ONCE(!conn))
+		return;
+
+	spin_lock_irqsave(&conn->timer_lock, flags);
+	if (conn->timers_stopped || chan->timers_stopped) {
+		spin_unlock_irqrestore(&conn->timer_lock, flags);
+		return;
+	}
+
 	l2cap_chan_hold(chan);
 
 	/* put(chan) if timer was already queued so it already has a ref */
-	if (mod_delayed_work(system_percpu_wq, work, timeout))
+	pending = mod_delayed_work(conn->timer_workqueue, work, timeout);
+	spin_unlock_irqrestore(&conn->timer_lock, flags);
+
+	if (pending)
 		l2cap_chan_put(chan);
 }
 
diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 65e957fdc7ae..0df7bda54473 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -686,9 +686,21 @@ void l2cap_chan_add(struct l2cap_conn *conn, struct l2cap_chan *chan)
 
 void l2cap_chan_del(struct l2cap_chan *chan, int err)
 {
+	struct l2cap_conn *conn = chan->conn;
+	unsigned long flags;
+
 	lockdep_assert(!chan->conn || lockdep_is_held(&chan->conn->lock));
 
+	if (conn) {
+		spin_lock_irqsave(&conn->timer_lock, flags);
+		chan->timers_stopped = true;
+		spin_unlock_irqrestore(&conn->timer_lock, flags);
+	}
+
 	__clear_chan_timer(chan);
+	__clear_retrans_timer(chan);
+	__clear_monitor_timer(chan);
+	__clear_ack_timer(chan);
 
 	BT_DBG("chan %p, err %d, state %s", chan, err,
 	       state_to_string(chan->state));
@@ -723,10 +735,6 @@ void l2cap_chan_del(struct l2cap_chan *chan, int err)
 		break;
 
 	case L2CAP_MODE_ERTM:
-		__clear_retrans_timer(chan);
-		__clear_monitor_timer(chan);
-		__clear_ack_timer(chan);
-
 		skb_queue_purge(&chan->srej_q);
 
 		l2cap_seq_list_free(&chan->srej_list);
@@ -1879,6 +1887,7 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 {
 	struct l2cap_conn *conn = hcon->l2cap_data;
 	struct l2cap_chan *chan, *l;
+	unsigned long flags;
 
 	if (!conn)
 		return;
@@ -1891,6 +1900,9 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 	cancel_work_sync(&conn->pending_rx_work);
 
 	mutex_lock(&conn->lock);
+	spin_lock_irqsave(&conn->timer_lock, flags);
+	conn->timers_stopped = true;
+	spin_unlock_irqrestore(&conn->timer_lock, flags);
 
 	kfree_skb(conn->rx_skb);
 
@@ -1925,6 +1937,11 @@ static void l2cap_conn_del(struct hci_conn *hcon, int err)
 	spin_unlock(&hcon->proto_lock);
 
 	mutex_unlock(&conn->lock);
+
+	/* Channel deletion canceled pending timers. Drop conn->lock before
+	 * waiting for running callbacks so they can acquire it and return.
+	 */
+	drain_workqueue(conn->timer_workqueue);
 	l2cap_conn_put(conn);
 }
 
@@ -1932,6 +1949,7 @@ static void l2cap_conn_free(struct kref *ref)
 {
 	struct l2cap_conn *conn = container_of(ref, struct l2cap_conn, ref);
 
+	destroy_workqueue(conn->timer_workqueue);
 	hci_conn_put(conn->hcon);
 	kfree(conn);
 }
@@ -7416,6 +7434,14 @@ static struct l2cap_conn *l2cap_conn_add(struct hci_conn *hcon)
 		return NULL;
 	}
 
+	conn->timer_workqueue = alloc_ordered_workqueue("l2cap", WQ_MEM_RECLAIM);
+	if (!conn->timer_workqueue) {
+		kfree(conn);
+		hci_chan_del(hchan);
+		return NULL;
+	}
+	spin_lock_init(&conn->timer_lock);
+
 	kref_init(&conn->ref);
 	conn->hchan = hchan;
 
-- 
2.43.0