[PATCH] Bluetooth: RFCOMM: add length check in rfcomm_recv_mcc

[PATCH] Bluetooth: RFCOMM: add length check in rfcomm_recv_mcc

[Muhammad Bilal]

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.

Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Cc: stable@vger.kernel.org
Link: https://lore.kernel.org/linux-bluetooth/20260414010741.233892-1-suunj1331@gmail.com/
Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
---
 net/bluetooth/rfcomm/core.c | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index d11bd5337..4e8047012 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -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

RE: Bluetooth: RFCOMM: add length check in rfcomm_recv_mcc

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1042 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=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

Re: [PATCH] Bluetooth: RFCOMM: add length check in rfcomm_recv_mcc

[SeungJu Cheon]

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


On Sat, May 23, 2026 at 2:57 AM Muhammad Bilal <meatuni001@gmail.com> wrote:
>
> 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.
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Cc: stable@vger.kernel.org
> Link: https://lore.kernel.org/linux-bluetooth/20260414010741.233892-1-suunj1331@gmail.com/
> Signed-off-by: Muhammad Bilal <meatuni001@gmail.com>
> ---
>  net/bluetooth/rfcomm/core.c | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
>
> diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
> index d11bd5337..4e8047012 100644
> --- a/net/bluetooth/rfcomm/core.c
> +++ b/net/bluetooth/rfcomm/core.c
> @@ -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
>