Bluetooth: RFCOMM: validate skb length in MCC handlers

PATCH Applied SeungJu Cheon 3 participants 3 messages May 25 – May 28

Subsystems (2)

Bluetooth drivers Bluetooth

lore

Share & tools

lore

SeungJu Cheon

author

2026-05-25 11:05 UTC #0 lore raw

PATCH v3 1/1 Bluetooth: RFCOMM: validate skb length in MCC handlers

The RFCOMM MCC handlers cast skb->data to protocol-specific structs without validating skb->len first. A malicious remote device can send truncated MCC frames and trigger out-of-bounds reads in these handlers.

Fix this by using skb_pull_data() to validate and access the required data before dereferencing it.

rfcomm_recv_rpn() requires special handling since ETSI TS 07.10 allows 1-byte RPN requests. Handle this by validating only the DLCI byte first, and validating the full struct only when len > 1.

Fixes 1da177e4c3f4 ("Linux-2.6.12-rc2")

Signed-off-by SeungJu Cheon <[email protected]>

---
v3:
- Apply skb_pull_data() in rfcomm_recv_mcc() before accessing the MCC header
- Return -EILSEQ instead of -EINVAL for malformed frames

v2:
- Use skb_pull_data() instead of manual length checks (Luiz)
- Add MCC header length validation in rfcomm_recv_mcc() (Paul)
- Allow 1-byte RPN requests per ETSI TS 07.10 (Paul)

Testing:
Tested under QEMU with VHCI and KASAN enabled by injecting both well-formed and malformed RFCOMM MCC frames.

Well-formed PN/MSC/RLS frames and both valid RPN variants (full 8-byte form and 1-byte query form) are parsed identically to the pre-patch behavior.

The following malformed frames are rejected before reaching the MCC sub-handlers:

- truncated MCC headers
- PN/MSC/RLS/RPN payloads shorter than the fixed struct size
- invalid intermediate RPN lengths (len != 1 && len < 8)
- MCC payload lengths larger than the actual skb data

Frames with cr=0 are still silently dropped, and oversized frames continue to ignore trailing bytes as before.

 net/bluetooth/rfcomm/core.c | 67 +++++++++++++++++++++++++++----------
 1 file changed, 49 insertions(+), 18 deletions(-)

net/bluetooth/rfcomm/core.c +49 ~~-18~~

index d11bd5337d57..364b9381c2dc 100644
@@ -1431,10 +1431,15 @@ static int rfcomm_apply_pn(struct rfcomm_dlc *d, int cr, struct rfcomm_pn *pn)
 
 static int rfcomm_recv_pn(struct rfcomm_session *s, int cr, struct sk_buff *skb)
 {
-	struct rfcomm_pn *pn = (void *) skb->data;
+	struct rfcomm_pn *pn;
 	struct rfcomm_dlc *d;
-	u8 dlci = pn->dlci;
+	u8 dlci;
+
+	pn = skb_pull_data(skb, sizeof(*pn));
+	if (!pn)
+		return -EILSEQ;
 
+	dlci = pn->dlci;
 	BT_DBG("session %p state %ld dlci %d", s, s->state, dlci);
 
 	if (!dlci)

@@ -1483,8 +1488,8 @@ static int rfcomm_recv_pn(struct rfcomm_session *s, int cr, struct sk_buff *skb)
 
 static int rfcomm_recv_rpn(struct rfcomm_session *s, int cr, int len, struct sk_buff *skb)
 {
-	struct rfcomm_rpn *rpn = (void *) skb->data;
-	u8 dlci = __get_dlci(rpn->dlci);
+	struct rfcomm_rpn *rpn;
+	u8 dlci;
 
 	u8 bit_rate  = 0;
 	u8 data_bits = 0;

@@ -1495,15 +1500,16 @@ static int rfcomm_recv_rpn(struct rfcomm_session *s, int cr, int len, struct sk_
 	u8 xoff_char = 0;
 	u16 rpn_mask = RFCOMM_RPN_PM_ALL;
 
-	BT_DBG("dlci %d cr %d len 0x%x bitr 0x%x line 0x%x flow 0x%x xonc 0x%x xoffc 0x%x pm 0x%x",
-		dlci, cr, len, rpn->bit_rate, rpn->line_settings, rpn->flow_ctrl,
-		rpn->xon_char, rpn->xoff_char, rpn->param_mask);
+	if (len == 1) {
+		rpn = skb_pull_data(skb, 1);
+		if (!rpn)
+			return -EILSEQ;
 
-	if (!cr)
-		return 0;
+		dlci = __get_dlci(rpn->dlci);
+
+		if (!cr)
+			return 0;
 
-	if (len == 1) {
-		/* This is a request, return default (according to ETSI TS 07.10) settings */
 		bit_rate  = RFCOMM_RPN_BR_9600;
 		data_bits = RFCOMM_RPN_DATA_8;
 		stop_bits = RFCOMM_RPN_STOP_1;

@@ -1514,6 +1520,19 @@ static int rfcomm_recv_rpn(struct rfcomm_session *s, int cr, int len, struct sk_
 		goto rpn_out;
 	}
 
+	rpn = skb_pull_data(skb, sizeof(*rpn));
+	if (!rpn)
+		return -EILSEQ;
+
+	dlci = __get_dlci(rpn->dlci);
+
+	BT_DBG("dlci %d cr %d len 0x%x bitr 0x%x line 0x%x flow 0x%x xonc 0x%x xoffc 0x%x pm 0x%x",
+	       dlci, cr, len, rpn->bit_rate, rpn->line_settings, rpn->flow_ctrl,
+	       rpn->xon_char, rpn->xoff_char, rpn->param_mask);
+
+	if (!cr)
+		return 0;
+
 	/* Check for sane values, ignore/accept bit_rate, 8 bits, 1 stop bit,
 	 * no parity, no flow control lines, normal XON/XOFF chars */

@@ -1589,9 +1608,14 @@ static int rfcomm_recv_rpn(struct rfcomm_session *s, int cr, int len, struct sk_
 
 static int rfcomm_recv_rls(struct rfcomm_session *s, int cr, struct sk_buff *skb)
 {
-	struct rfcomm_rls *rls = (void *) skb->data;
-	u8 dlci = __get_dlci(rls->dlci);
+	struct rfcomm_rls *rls;
+	u8 dlci;
 
+	rls = skb_pull_data(skb, sizeof(*rls));
+	if (!rls)
+		return -EILSEQ;
+
+	dlci = __get_dlci(rls->dlci);
 	BT_DBG("dlci %d cr %d status 0x%x", dlci, cr, rls->status);
 
 	if (!cr)

@@ -1608,10 +1632,15 @@ static int rfcomm_recv_rls(struct rfcomm_session *s, int cr, struct sk_buff *skb
 
 static int rfcomm_recv_msc(struct rfcomm_session *s, int cr, struct sk_buff *skb)
 {
-	struct rfcomm_msc *msc = (void *) skb->data;
+	struct rfcomm_msc *msc;
 	struct rfcomm_dlc *d;
-	u8 dlci = __get_dlci(msc->dlci);
+	u8 dlci;
+
+	msc = skb_pull_data(skb, sizeof(*msc));
+	if (!msc)
+		return -EILSEQ;
 
+	dlci = __get_dlci(msc->dlci);
 	BT_DBG("dlci %d cr %d v24 0x%x", dlci, cr, msc->v24_sig);
 
 	d = rfcomm_dlc_get(s, dlci);

@@ -1644,17 +1673,19 @@ 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;
 
+	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);

Signature · 1 line

2.52.0

[email protected]

2026-05-25 14:21 UTC #1 lore raw

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

---Test result---

Test Summary:

CheckPatch                    PASS      0.59 seconds
VerifyFixes                   PASS      0.07 seconds
VerifySignedoff               PASS      0.07 seconds
GitLint                       PASS      1.31 seconds
SubjectPrefix                 PASS      0.09 seconds
BuildKernel                   PASS      26.59 seconds
CheckAllWarning               PASS      29.34 seconds
CheckSparse                   PASS      28.00 seconds
BuildKernel32                 PASS      25.80 seconds
TestRunnerSetup               PASS      573.06 seconds
TestRunner_rfcomm-tester      PASS      25.38 seconds
IncrementalBuild              PASS      25.10 seconds

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

---
Regards,
Linux Bluetooth

[email protected]

CI Applied

2026-05-28 14:20 UTC #2 lore raw

In reply to SeungJu Cheon

This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <[email protected]>:

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master) by Luiz Augusto von Dentz <[email protected]>:

Quoted 8 lines · On Mon, 25 May 2026 20:04:43 +0900 you wrote:

The RFCOMM MCC handlers cast skb->data to protocol-specific structs without validating skb->len first. A malicious remote device can send truncated MCC frames and trigger out-of-bounds reads in these handlers.
Fix this by using skb_pull_data() to validate and access the required data before dereferencing it.
[...]

Here is the summary with links:
- [v3] Bluetooth: RFCOMM: validate skb length in MCC handlers

    https://git.kernel.org/bluetooth/bluetooth-next/c/a82ccb946ae5

You are awesome, thank you!

Signature · 2 lines

Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html