From dc1ddc65f06df44dfcfe51955c82a132acb3ab8d Mon Sep 17 00:00:00 2001 From: Pauli Virtanen Date: Wed, 2 Sep 2026 00:04:33 +0300 Subject: [PATCH] Bluetooth: L2CAP: take lock for l2cap_chan_del in l2cap_ecred_rsp_defer l2cap_ecred_rsp_defer() calls l2cap_chan_del without holding chan->lock, which ends up calling ops->teardown() with wrong lock context. Fix by taking chan->lock in l2cap_ecred_rsp_defer(). AB-BA deadlocks between sibling l2cap_chan are avoided here via requiring l2cap_conn::lock to serialize all nested l2cap_chan locking on same nesting level. In current code, there is no nested l2cap_chan locking on same nesting level, so we can add this new requirement. Also return early from __l2cap_ecred_conn_rsp_defer() if chan did not have FLAG_DEFER_SETUP, as then no RSP shall be sent for it, to make sure SMP channels are excluded. Also hold chan reference over l2cap_chan_del(), in case chan_l reference was the last. Signed-off-by: Pauli Virtanen Signed-off-by: Luiz Augusto von Dentz --- include/net/bluetooth/l2cap.h | 4 +++ net/bluetooth/l2cap_core.c | 60 +++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+) diff --git a/include/net/bluetooth/l2cap.h b/include/net/bluetooth/l2cap.h index 2315a3993c7b..c7e642abe404 100644 --- a/include/net/bluetooth/l2cap.h +++ b/include/net/bluetooth/l2cap.h @@ -758,6 +758,10 @@ enum { * otherwise considers all channels equal and will e.g. complain about a * connection oriented channel triggering SMP procedures or a listening * channel creating and locking a child channel. + * + * Lock nesting of channels at the same nesting level is allowed if the channels + * have the same l2cap_chan::conn and l2cap_chan::conn.lock is taken before the + * nested locks. l2cap_chan_try_sibling_lock() must be used. */ enum { L2CAP_NESTING_SMP, diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c index 4294cab61d3e..6cd909bba1c7 100644 --- a/net/bluetooth/l2cap_core.c +++ b/net/bluetooth/l2cap_core.c @@ -848,6 +848,8 @@ static void __l2cap_chan_close(struct l2cap_chan *chan, int reason) BT_DBG("chan %p state %s", chan, state_to_string(chan->state)); + lockdep_assert_held(&chan->lock); + switch (chan->state) { case BT_LISTEN: chan->ops->teardown(chan, 0); @@ -3950,6 +3952,7 @@ static void l2cap_ecred_list_defer(struct l2cap_chan *chan, void *data) } struct l2cap_ecred_rsp_data { + struct l2cap_chan *locked_chan; struct { struct l2cap_ecred_conn_rsp_hdr rsp; __le16 scid[L2CAP_ECRED_MAX_CID]; @@ -3957,11 +3960,42 @@ struct l2cap_ecred_rsp_data { int count; }; +/* Lock @chan if it is not @locked_chan, and has same or lower nesting level. + * + * They must have the same chan->conn, and conn->lock must be held. + * + * Caller must ensure @chan has lock nesting level <= that of @locked_chan, as + * nested locking of l2cap_chan of different levels is allowed also without + * holding conn->lock. + * + * See l2cap.h for the global l2cap_chan locking rules. + */ +static bool l2cap_chan_try_sibling_lock(struct l2cap_chan *chan, + struct l2cap_chan *locked_chan) + __must_hold(&locked_chan->lock) + __must_hold(&locked_chan->conn->lock) + __cond_acquires(true, &chan->lock) +{ + if (chan == locked_chan) + return false; + + if (WARN_ON_ONCE(locked_chan->conn != chan->conn)) + return false; + + if (WARN_ON_ONCE(atomic_read(&locked_chan->nesting) + < atomic_read(&chan->nesting))) + return false; + + mutex_lock_nest_lock(&chan->lock, &locked_chan->conn->lock); + return true; +} + static void l2cap_ecred_rsp_defer(struct l2cap_chan *chan, void *data) { struct l2cap_ecred_rsp_data *rsp = data; struct l2cap_ecred_conn_rsp *rsp_flex = container_of(&rsp->pdu.rsp, struct l2cap_ecred_conn_rsp, hdr); + bool locked; if (chan->mode != L2CAP_MODE_EXT_FLOWCTL) return; @@ -3973,6 +4007,22 @@ static void l2cap_ecred_rsp_defer(struct l2cap_chan *chan, void *data) !test_and_clear_bit(FLAG_DEFER_SETUP, &chan->flags)) return; + lockdep_assert_held(&rsp->locked_chan->lock); + lockdep_assert_held(&rsp->locked_chan->conn->lock); + + l2cap_chan_hold(chan); + + locked = l2cap_chan_try_sibling_lock(chan, rsp->locked_chan); + + /* Cannot occur: PARENT channels do not appear in chan_l, and SMP + * channels never have FLAG_DEFER_SETUP. + */ + if (context_unsafe(!locked && chan != rsp->locked_chan)) + goto done; + + lockdep_assert_held(&chan->lock); + lockdep_assert_held(&chan->conn->lock); + /* Reset ident so only one response is sent */ chan->ident = 0; @@ -3985,6 +4035,12 @@ static void l2cap_ecred_rsp_defer(struct l2cap_chan *chan, void *data) rsp_flex->dcid[rsp->count++] = cpu_to_le16(chan->scid); else l2cap_chan_del(chan, ECONNRESET); + +done: + if (locked) + l2cap_chan_unlock(chan); + + l2cap_chan_put(chan); } void __l2cap_ecred_conn_rsp_defer(struct l2cap_chan *chan) @@ -3996,11 +4052,15 @@ void __l2cap_ecred_conn_rsp_defer(struct l2cap_chan *chan) if (!id) return; + if (!test_bit(FLAG_DEFER_SETUP, &chan->flags)) + return; BT_DBG("chan %p id %d", chan, id); memset(&data, 0, sizeof(data)); + data.locked_chan = chan; + data.pdu.rsp.mtu = cpu_to_le16(chan->imtu); data.pdu.rsp.mps = cpu_to_le16(chan->mps); data.pdu.rsp.credits = cpu_to_le16(chan->rx_credits);