[PATCH] Bluetooth: L2CAP: fix out-of-bounds write in l2cap_ecred_connect

Pauli Virtanen posted 1 patch 3 weeks, 6 days ago
net/bluetooth/l2cap_core.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
[PATCH] Bluetooth: L2CAP: fix out-of-bounds write in l2cap_ecred_connect
Posted by Pauli Virtanen 3 weeks, 6 days ago
l2cap_chan_connect() tries to ensure there are no more than
L2CAP_ECRED_CONN_SCID_MAX pending ECRED channels, so they fit in the
same L2CAP_ECRED_CONN_REQ that l2cap_ecred_connect() constructs.

However, the check only counts deferred channels.  If 6 L2CAP sockets
are connected at the same time in order DDDDND (D=deferred,
N=non-deferred), the last can bump the total to max+1.  It results to
one __le16 written out of bounds of the scid array, and an invalid
ECRED_CONN_REQ being sent.

Fix by including non-deferred pending ECRED channels in the counting in
l2cap_chan_connect(), so the limit can't be exceeded.

Also add WARN_ON_ONCE check in l2cap_ecred_defer_connect() to make this
less brittle.

Fixes: da49b602f7f7 ("Bluetooth: L2CAP: Use DEFER_SETUP to group ECRED connections")
Signed-off-by: Pauli Virtanen <pav@iki.fi>
---

Notes:
    Bug found as pre-existing in sashiko.dev report

 net/bluetooth/l2cap_core.c | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/l2cap_core.c b/net/bluetooth/l2cap_core.c
index 358b11eabd4f..6edf207e6345 100644
--- a/net/bluetooth/l2cap_core.c
+++ b/net/bluetooth/l2cap_core.c
@@ -1329,7 +1329,7 @@ static void l2cap_le_connect(struct l2cap_chan *chan)
 struct l2cap_ecred_conn_data {
 	struct {
 		struct l2cap_ecred_conn_req_hdr req;
-		__le16 scid[5];
+		__le16 scid[L2CAP_ECRED_CONN_SCID_MAX];
 	} __packed pdu;
 	struct l2cap_chan *chan;
 	struct pid *pid;
@@ -1357,6 +1357,9 @@ static void l2cap_ecred_defer_connect(struct l2cap_chan *chan, void *data)
 	if (test_and_set_bit(FLAG_ECRED_CONN_REQ_SENT, &chan->flags))
 		return;
 
+	if (WARN_ON_ONCE(conn->count >= ARRAY_SIZE(conn->pdu.scid)))
+		return;
+
 	l2cap_ecred_init(chan, 0);
 
 	/* Set the same ident so we can match on the rsp */
@@ -7240,12 +7243,9 @@ static void l2cap_chan_by_pid(struct l2cap_chan *chan, void *data)
 	if (chan == d->chan)
 		return;
 
-	if (!test_bit(FLAG_DEFER_SETUP, &chan->flags))
-		return;
-
 	pid = chan->ops->get_peer_pid(chan);
 
-	/* Only count deferred channels with the same PID/PSM */
+	/* Count all channels to be gathered into same ECRED_CONN_REQ */
 	if (d->pid != pid || chan->psm != d->chan->psm || chan->ident ||
 	    chan->mode != L2CAP_MODE_EXT_FLOWCTL || chan->state != BT_CONNECT)
 		return;
-- 
2.55.0