Re: [PATCH] Bluetooth: RFCOMM: validate skb length in MCC handlers
From: Paul Menzel
Date: Sun Apr 12 2026 - 03:42:12 EST
Dear SeungJu,
Thank you for the patch.
Am 12.04.26 um 06:54 schrieb SeungJu Cheon:
rfcomm_recv_pn(), rfcomm_recv_rpn(), rfcomm_recv_rls(), and
rfcomm_recv_msc() cast skb->data to their respective structs
without first checking skb->len. A remote device can send a
short MCC frame, causing out-of-bounds reads from the skb buffer.
Nice catch. Do you have a reproducer to create such a short MCC frame?
For rfcomm_recv_pn(), the uninitialized pn->mtu value is stored
in d->mtu via rfcomm_apply_pn(), then echoed back to the remote
device in the PN response, leaking kernel heap data.
This results in use of uninitialized memory, as reported by KMSAN.
Add explicit skb->len checks against the expected structure size
at the start of each handler before accessing the payload.
=====================================================
BUG: KMSAN: uninit-value in rfcomm_run+0x7eae/0xee90
rfcomm_run+0x7eae/0xee90
kthread+0x53f/0x600
ret_from_fork+0x20f/0x910
ret_from_fork_asm+0x1a/0x30
Uninit was created at:
kmem_cache_alloc_node_noprof+0x3cd/0x12d0
__alloc_skb+0x855/0x1190
vhci_write+0x125/0x960
vfs_write+0xbe1/0x15c0
ksys_write+0x1d9/0x470
__x64_sys_write+0x97/0xf0
x64_sys_call+0x2ff0/0x3ea0
do_syscall_64+0x134/0xf80
entry_SYSCALL_64_after_hwframe+0x77/0x7f
CPU: 0 UID: 0 PID: 3374 Comm: krfcommd Tainted: G W 7.0.0-rc7
Hardware name: QEMU Standard PC (i440FX + PIIX, 1996)
Kernel panic - not syncing: kmsan.panic set ...
=====================================================
Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
Signed-off-by: SeungJu Cheon <suunj1331@xxxxxxxxx>
---
net/bluetooth/rfcomm/core.c | 40 +++++++++++++++++++++++++++++--------
1 file changed, 32 insertions(+), 8 deletions(-)
diff --git a/net/bluetooth/rfcomm/core.c b/net/bluetooth/rfcomm/core.c
index 611a9a94151e..daeba71a1514 100644
--- a/net/bluetooth/rfcomm/core.c
+++ b/net/bluetooth/rfcomm/core.c
@@ -1431,9 +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;
+
+ if (skb->len < sizeof(*pn))
+ return -EINVAL;
+
+ pn = (void *) skb->data;
+ dlci = pn->dlci;
gemini/gemini-3.1-pro-preview comments [1]. No idea if it’s a valid comment:
Before these handlers are reached, rfcomm_recv_mcc() unconditionally casts
skb->data to struct rfcomm_mcc * and reads mcc->type and mcc->len. Does
this leave an out-of-bounds read unpatched if a remote device sends an MCC
frame with 0 or 1 bytes of payload?
Additionally, if rfcomm_recv_frame() unconditionally trims the FCS with
skb->len--; skb->tail--;, could a 0-byte payload cause skb->len to
underflow to UINT_MAX?
Since skb->len is unsigned, this new check (UINT_MAX < 8) would evaluate to
false, potentially bypassing this protection entirely.
BT_DBG("session %p state %ld dlci %d", s, s->state, dlci);
@@ -1483,8 +1489,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,6 +1501,12 @@ 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;
+ if (skb->len < sizeof(*rpn))
+ return -EINVAL;
gemini/gemini-3.1-pro-preview comments [1]:
Does this unconditionally drop legitimate 1-byte Remote Port Negotiation
(RPN) requests?
Looking at the existing code further down in rfcomm_recv_rpn():
if (len == 1) {
/* This is a request, return default (according to ETSI TS 07.10) settings */
...
Since sizeof(struct rfcomm_rpn) is 8 bytes, enforcing this minimum length
would reject valid 1-byte queries allowed by the ETSI TS 07.10 standard.
+
+ rpn = (void *) skb->data;
+ 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);
@@ -1589,8 +1601,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;
+
+ if (skb->len < sizeof(*rls))
+ return -EINVAL;
+
+ rls = (void *) skb->data;
+ dlci = __get_dlci(rls->dlci);
BT_DBG("dlci %d cr %d status 0x%x", dlci, cr, rls->status);
@@ -1608,9 +1626,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;
+
+ if (skb->len < sizeof(*msc))
+ return -EINVAL;
+
+ msc = (void *) skb->data;
+ dlci = __get_dlci(msc->dlci);
BT_DBG("dlci %d cr %d v24 0x%x", dlci, cr, msc->v24_sig);
Kind regards,
Paul
[1]: https://sashiko.dev/#/patchset/20260412045457.53100-1-suunj1331%40gmail.com