Re: [PATCH] Bluetooth: RFCOMM: validate skb length in MCC handlers

[Luiz Augusto von Dentz]

Hi,

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

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