[PATCH] Bluetooth: RFCOMM: Reject short EA=0 frames in rfcomm_recv_frame()

[PATCH] Bluetooth: RFCOMM: Reject short EA=0 frames in rfcomm_recv_frame()

[Hui Peng]

While rfcomm_recv_frame() verifies that skb->len is at least
sizeof(*hdr) + 1 (4 bytes: 3-byte header + 1-byte FCS), an RFCOMM frame
with an extended 2-byte length field (!__test_ea(hdr->len)) has a 4-byte
header plus a 1-byte FCS (5 bytes minimum, sizeof(*hdr) + 2).

When a 4-byte RFCOMM frame with EA == 0 arrives:
1. The initial skb->len < sizeof(*hdr) + 1 check passes (4 < 4 is false).
2. Trimming the FCS byte decrements skb->len to 3.
3. If __check_fcs() succeeds, skb_pull(skb, 4) fails (4 > 3) and returns
   NULL without advancing skb->data.
4. Because the return value of skb_pull() is ignored, the un-pulled
   3-byte struct rfcomm_hdr remains at skb->data and is either queued as
   application payload via rfcomm_recv_data() or parsed as a multiplexer
   control command via rfcomm_recv_mcc() on DLCI 0.

Fix this by extending the length check in rfcomm_recv_frame() to also
require skb->len >= sizeof(*hdr) + 2 when !__test_ea(hdr->len).

Signed-off-by: Hui Peng <benquike@gmail.com>
---
 net/bluetooth/rfcomm/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index f7463f092..d91e2a6ee 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1817,7 +1817,8 @@ static struct rfcomm_session *rfcomm_recv_frame(struct rfcomm_session *s,
 		return s;
 	}
 
-	if (skb->len < sizeof(*hdr) + 1) {
+	if (skb->len < sizeof(*hdr) + 1 ||
+	    (!__test_ea(hdr->len) && skb->len < sizeof(*hdr) + 2)) {
 		kfree_skb(skb);
 		return s;
 	}
-- 
2.55.0.1082.g2b9226bbc0-goog

[PATCH v2] Bluetooth: RFCOMM: Reject short EA=0 frames in rfcomm_recv_frame()

[Hui Peng]

While rfcomm_recv_frame() verifies that skb->len is at least
sizeof(*hdr) + 1 (4 bytes: 3-byte header + 1-byte FCS), an RFCOMM frame
with an extended 2-byte length field (!__test_ea(hdr->len)) has a 4-byte
header plus a 1-byte FCS (5 bytes minimum, sizeof(*hdr) + 2).

When a 4-byte RFCOMM frame with EA == 0 arrives:
1. The initial skb->len < sizeof(*hdr) + 1 check passes (4 < 4 is false).
2. Trimming the FCS byte decrements skb->len to 3.
3. If __check_fcs() succeeds, skb_pull(skb, 4) fails (4 > 3) and returns
   NULL without advancing skb->data.
4. Because the return value of skb_pull() is ignored, the un-pulled
   3-byte struct rfcomm_hdr remains at skb->data and is either queued as
   application payload via rfcomm_recv_data() or parsed as a multiplexer
   control command via rfcomm_recv_mcc() on DLCI 0.

Fix this by extending the length check in rfcomm_recv_frame() to also
require skb->len >= sizeof(*hdr) + 2 when !__test_ea(hdr->len).

Fixes: b230e5bf501c ("Bluetooth: RFCOMM: validate skb length in rfcomm_recv_frame")
Assisted-by: LLM
Signed-off-by: Hui Peng <benquike@gmail.com>
---
v2: Add a Fixes: tag, and add the Assisted-by: LLM tag that v1 was
missing - apologies, v1 predated my reading of
Documentation/process/coding-assistants.rst.

The tag points at b230e5bf501c ("Bluetooth: RFCOMM: validate skb length
in rfcomm_recv_frame"), which added the skb->len < sizeof(*hdr) + 1
bound that this patch widens. Its changelog justifies the bound as "the
minimum frame must have a 3-byte header and a 1-byte FCS", which holds
only for EA=1; an EA=0 frame has a 4-byte header plus the FCS.

To be upfront: b230e5bf501c is not where the underlying exposure began.
Before it there was no length check at all, so a 4-byte EA=0 frame
already reached skb_pull(skb, 4) with skb->len == 3 and the ignored
return value already left the un-pulled header to be parsed. That goes
back to the initial Bluetooth stack import, 1da177e4c3f4
("Linux-2.6.12-rc2"). I have tagged b230e5bf501c instead because it is
the commit that introduced the insufficient bound this patch corrects,
and because the single line of context this hunk relies on does not
exist in any tree older than v7.2 - pointing stable at 2.6.12 would be
actively unhelpful.

 net/bluetooth/rfcomm/core.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index f7463f092..d91e2a6ee 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1817,7 +1817,8 @@ static struct rfcomm_session *rfcomm_recv_frame(struct rfcomm_session *s,
 		return s;
 	}
 
-	if (skb->len < sizeof(*hdr) + 1) {
+	if (skb->len < sizeof(*hdr) + 1 ||
+	    (!__test_ea(hdr->len) && skb->len < sizeof(*hdr) + 2)) {
 		kfree_skb(skb);
 		return s;
 	}
-- 
2.55.0.1082.g2b9226bbc0-goog

Re: [PATCH v2] Bluetooth: RFCOMM: Reject short EA=0 frames in rfcomm_recv_frame()

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Sat, 19 Sep 2026 11:25:14 +0000 you wrote:
> While rfcomm_recv_frame() verifies that skb->len is at least
> sizeof(*hdr) + 1 (4 bytes: 3-byte header + 1-byte FCS), an RFCOMM frame
> with an extended 2-byte length field (!__test_ea(hdr->len)) has a 4-byte
> header plus a 1-byte FCS (5 bytes minimum, sizeof(*hdr) + 2).
> 
> When a 4-byte RFCOMM frame with EA == 0 arrives:
> 1. The initial skb->len < sizeof(*hdr) + 1 check passes (4 < 4 is false).
> 2. Trimming the FCS byte decrements skb->len to 3.
> 3. If __check_fcs() succeeds, skb_pull(skb, 4) fails (4 > 3) and returns
>    NULL without advancing skb->data.
> 4. Because the return value of skb_pull() is ignored, the un-pulled
>    3-byte struct rfcomm_hdr remains at skb->data and is either queued as
>    application payload via rfcomm_recv_data() or parsed as a multiplexer
>    control command via rfcomm_recv_mcc() on DLCI 0.
> 
> [...]

Here is the summary with links:
  - [v2] Bluetooth: RFCOMM: Reject short EA=0 frames in rfcomm_recv_frame()
    https://git.kernel.org/bluetooth/bluetooth-next/c/6d91041bb38b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html