[PATCH v2] Bluetooth: L2CAP: Fix ECRED reconf rsp channel teardown race

Feng Ning posted 1 patch 2 months ago
There is a newer version of this series
net/bluetooth/l2cap_core.c | 8 ++++++++
1 file changed, 8 insertions(+)
[PATCH v2] Bluetooth: L2CAP: Fix ECRED reconf rsp channel teardown race
Posted by Feng Ning 2 months ago
From: feng <feng@innora.ai>

The ECRED reconfiguration response tears down all channels that were part
of a failed procedure. The handler iterates over conn->chan_l while
holding conn->lock but it called l2cap_chan_del() without taking an extra
reference and without the per-channel lock. Other paths (socket close,
timer expiry, etc.) may drop the final reference outside conn->lock,
causing a use-after-free when the response is handled.

Take a temporary reference with l2cap_chan_hold_unless_zero(), perform
the deletion under the channel lock, and drop the reference afterwards.
Add lockdep_assert_held(&conn->lock) to document the calling requirements.

Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
Cc: stable@vger.kernel.org # v5.7+
Signed-off-by: Feng Ning <feng@innora.ai>
---
 net/bluetooth/l2cap_core.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 95c65fece..08d2045ab 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5466,6 +5466,8 @@ static inline int l2cap_ecred_reconf_rsp(struct l2cap_conn *conn,
 
 	BT_DBG("result 0x%4.4x", result);
 
+	lockdep_assert_held(&conn->lock);
+
 	if (!result)
 		return 0;
 
@@ -5473,7 +5475,13 @@ static inline int l2cap_ecred_reconf_rsp(struct l2cap_conn *conn,
 		if (chan->ident != cmd->ident)
 			continue;
 
+		if (!l2cap_chan_hold_unless_zero(chan))
+			continue;
+
+		l2cap_chan_lock(chan);
 		l2cap_chan_del(chan, ECONNRESET);
+		l2cap_chan_unlock(chan);
+		l2cap_chan_put(chan);
 	}
 
 	return 0;
-- 
2.49.0
[PATCH v3] Bluetooth: L2CAP: Fix ECRED reconf rsp channel teardown race
Posted by Feng Ning 2 months ago
The ECRED reconfiguration response tears down all channels that were part
of a failed procedure. The handler iterates over conn->chan_l while
holding conn->lock but l2cap_chan_hold() is called without first checking
whether the reference count has already reached zero. A concurrent path
(socket close, timer expiry) may drop the final reference outside
conn->lock, causing a use-after-free when the response is handled.

Replace l2cap_chan_hold() with l2cap_chan_hold_unless_zero() so that
channels whose reference count has already been dropped are skipped
safely. Add lockdep_assert_held(&conn->lock) to document the calling
requirements.

Fixes: 15f02b910562 ("Bluetooth: L2CAP: Add initial code for Enhanced Credit Based Mode")
Cc: stable@vger.kernel.org # v5.7+
Signed-off-by: Feng Ning <feng@innora.ai>
---
 net/bluetooth/l2cap_core.c | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 77dec104a..191c38b4d 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5466,6 +5466,8 @@ static inline int l2cap_ecred_reconf_rsp(struct l2cap_conn *conn,
 
 	BT_DBG("result 0x%4.4x", result);
 
+	lockdep_assert_held(&conn->lock);
+
 	if (!result)
 		return 0;
 
@@ -5473,7 +5475,9 @@ static inline int l2cap_ecred_reconf_rsp(struct l2cap_conn *conn,
 		if (chan->ident != cmd->ident)
 			continue;
 
-		l2cap_chan_hold(chan);
+		if (!l2cap_chan_hold_unless_zero(chan))
+			continue;
+
 		l2cap_chan_lock(chan);
 
 		l2cap_chan_del(chan, ECONNRESET);
-- 
2.49.0