[PATCH v2] Bluetooth: L2CAP: Fix possible crash on l2cap_ecred_conn_rsp

[PATCH v2] Bluetooth: L2CAP: Fix possible crash on l2cap_ecred_conn_rsp

[Luiz Augusto von Dentz]

From: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>

If dcid is received for an already-assigned destination CID the spec
requires that both channels to be discarded, but calling l2cap_chan_del
may invalidate the tmp cursor created by list_for_each_entry_safe and in
fact it is the wrong procedure as the chan->dcid may be assigned
previously it really needs to be disconnected using
l2cap_send_disconn_req otherwise the remote peer would have no idea that
it shall consider to be disconnected.

Signed-off-by: Luiz Augusto von Dentz <luiz.von.dentz@intel.com>
---
 net/bluetooth/l2cap_core.c | 19 ++++++++++++-------
 1 file changed, 12 insertions(+), 7 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index fdccd62ccca8..4c68ee5cec7e 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -5242,7 +5242,7 @@ static inline int l2cap_ecred_conn_rsp(struct l2cap_conn *conn,
 	struct l2cap_ecred_conn_rsp *rsp = (void *) data;
 	struct hci_conn *hcon = conn->hcon;
 	u16 mtu, mps, credits, result;
-	struct l2cap_chan *chan, *tmp;
+	struct l2cap_chan *chan, *tmp, *orig = NULL;
 	int err = 0, sec_level;
 	int i = 0;
 
@@ -5262,7 +5262,7 @@ static inline int l2cap_ecred_conn_rsp(struct l2cap_conn *conn,
 	list_for_each_entry_safe(chan, tmp, &conn->chan_l, list) {
 		u16 dcid;
 
-		if (chan->ident != cmd->ident ||
+		if (orig == chan || chan->ident != cmd->ident ||
 		    chan->mode != L2CAP_MODE_EXT_FLOWCTL ||
 		    chan->state == BT_CONNECTED)
 			continue;
@@ -5281,8 +5281,10 @@ static inline int l2cap_ecred_conn_rsp(struct l2cap_conn *conn,
 
 		BT_DBG("dcid[%d] 0x%4.4x", i, dcid);
 
+		orig = __l2cap_get_chan_by_dcid(conn, dcid);
+
 		/* Check if dcid is already in use */
-		if (dcid && __l2cap_get_chan_by_dcid(conn, dcid)) {
+		if (dcid && orig) {
 			/* If a device receives a
 			 * L2CAP_CREDIT_BASED_CONNECTION_RSP packet with an
 			 * already-assigned Destination CID, then both the
@@ -5291,10 +5293,13 @@ static inline int l2cap_ecred_conn_rsp(struct l2cap_conn *conn,
 			 */
 			l2cap_chan_del(chan, ECONNREFUSED);
 			l2cap_chan_unlock(chan);
-			chan = __l2cap_get_chan_by_dcid(conn, dcid);
-			l2cap_chan_lock(chan);
-			l2cap_chan_del(chan, ECONNRESET);
-			l2cap_chan_unlock(chan);
+			l2cap_chan_lock(orig);
+			/* Disconnect the original channel as it may be
+			 * considered connected since dcid has already been
+			 * assigned.
+			 */
+			l2cap_chan_close(orig, ECONNRESET);
+			l2cap_chan_unlock(orig);
 			continue;
 		}
 
-- 
2.53.0

RE: [v2] Bluetooth: L2CAP: Fix possible crash on l2cap_ecred_conn_rsp

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 937 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1094370

---Test result---

Test Summary:
CheckPatch                    PASS      0.54 seconds
GitLint                       PASS      0.21 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      26.73 seconds
CheckAllWarning               PASS      28.99 seconds
CheckSparse                   PASS      27.94 seconds
BuildKernel32                 PASS      25.65 seconds
TestRunnerSetup               PASS      573.39 seconds
TestRunner_l2cap-tester       PASS      376.55 seconds
IncrementalBuild              PASS      25.42 seconds



https://github.com/bluez/bluetooth-next/pull/184

---
Regards,
Linux Bluetooth