[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: [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
>