Bluetooth: RFCOMM: add length check in rfcomm_recv_mcc

rfcomm_recv_mcc() casts skb->data to struct rfcomm_mcc * and reads mcc->type and mcc->len without first checking that skb->len is at least sizeof(*mcc) (2 bytes). A remote device can send a crafted UIH frame with a one-byte or zero-byte MCC payload to trigger an out-of-bounds read of the second byte.

The unconditional skb_pull(skb, 2) that follows compounds the
problem: if skb->len is less than 2, skb->data and skb->len are corrupted for all downstream MCC sub-handlers.

Replace the open-coded cast and skb_pull() with skb_pull_data(), which atomically validates skb->len against sizeof(*mcc) and advances skb->data. Return -EILSEQ on failure.

SeungJu Cheon's v2 patch added a manual skb->len size check in rfcomm_recv_mcc() before an open-coded cast, but removed the subsequent skb_pull(skb, 2) without replacing it. This leaves skb->data pointing at the MCC header when the sub-handlers are called, causing them to parse from the wrong offset. Using skb_pull_data() here avoids this problem: it validates, casts, and advances skb->data atomically, and is consistent with how the sub-handlers themselves were fixed in Cheon's patch.

---
 net/bluetooth/rfcomm/core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

index d11bd5337..4e8047012 100644
@@ -1644,17 +1644,20 @@ static int rfcomm_recv_msc(struct rfcomm_session *s, int cr, struct sk_buff *skb
 
 static int rfcomm_recv_mcc(struct rfcomm_session *s, struct sk_buff *skb)
 {
-	struct rfcomm_mcc *mcc = (void *) skb->data;
+	struct rfcomm_mcc *mcc;
 	u8 type, cr, len;
 
+	/* Minimum MCC frame: type(1) + len(1) */
+	mcc = skb_pull_data(skb, sizeof(*mcc));
+	if (!mcc)
+		return -EILSEQ;
+
 	cr   = __test_cr(mcc->type);
 	type = __get_mcc_type(mcc->type);
 	len  = __get_mcc_len(mcc->len);
 
 	BT_DBG("%p type 0x%x cr %d", s, type, cr);
 
-	skb_pull(skb, 2);
-
 	switch (type) {
 	case RFCOMM_PN:
 		rfcomm_recv_pn(s, cr, skb);

2.54.0

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=1099548

---Test result---

Test Summary:

CheckPatch                    PASS      0.76 seconds
VerifyFixes                   PASS      0.14 seconds
VerifySignedoff               PASS      0.14 seconds
GitLint                       PASS      0.34 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      25.77 seconds
CheckAllWarning               PASS      29.02 seconds
CheckSparse                   PASS      26.55 seconds
BuildKernel32                 PASS      24.89 seconds
TestRunnerSetup               PASS      524.54 seconds
TestRunner_rfcomm-tester      PASS      25.67 seconds
IncrementalBuild              PASS      24.39 seconds

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

---
Regards,
Linux Bluetooth

Hi Muhammad,

Thanks for catching this. You're right — my v2 added the length check but accidentally dropped the skb_pull(skb, 2), which leaves skb->data pointing at the MCC header when the sub-handlers run.

Since my v2 isn't merged yet, I'll fold your fix into a v3 of my original series so everything stays in one patch. I'll add a Suggested-by tag to credit you.

Thanks again for the review.

Regards,
SeungJu