Re: [PATCH v2] Bluetooth: bnep: reject short frames before parsing

[Luiz Augusto von Dentz]

Hi,

On Fri, May 15, 2026 at 8:44 PM Zhang Cen <rollkingzzc@xxxxxxxxx> wrote:
>
> An L2CAP peer can deliver an empty BNEP payload or a payload that contains
> only the outer type byte. bnep_rx_frame() currently reads the BNEP type
> byte and, for control packets, the control opcode before it proves that
> the skb contains those bytes. The BNEP_SETUP_CONN_REQ path can also read
> the setup size byte before that byte is present, and bnep_rx_control()
> dereferences the control opcode before checking that its control payload
> is non-empty.
>
> Reject empty skbs before reading the outer type byte, require a control
> opcode before parsing BNEP_CONTROL, require the setup size byte before
> using it, and make bnep_rx_control() fail zero-length control payloads.
>
> Validation reproduced this kernel report:
> KASAN slab-out-of-bounds in bnep_rx_frame()
> Read of size 1
> Call trace:
>   dump_stack_lvl() (?:?)
>   print_address_description() (mm/kasan/report.c:373)
>   bnep_rx_frame() (net/bluetooth/bnep/core.c:306)
>   print_report() (?:?)
>   __virt_addr_valid() (?:?)
>   srso_alias_return_thunk() (arch/x86/include/asm/nospec-branch.h:375)
>   kasan_addr_to_slab() (mm/kasan/common.c:45)
>   kasan_report() (?:?)
>   process_one_work() (kernel/workqueue.c:3200)
>   worker_thread() (?:?)
>   __kthread_parkme() (kernel/kthread.c:259)
>   kthread() (?:?)
>   _raw_spin_unlock_irq() (kernel/locking/spinlock.c:204)
>   ret_from_fork() (?:?)
>   __switch_to() (?:?)
>   ret_from_fork_asm() (?:?)
>   kasan_save_stack() (mm/kasan/common.c:52)
>   kasan_save_track() (mm/kasan/common.c:74)
>   __kasan_kmalloc() (?:?)
>   vpanic() (kernel/panic.c:576)
>   panic() (?:?)
>   preempt_schedule_common() (kernel/sched/core.c:7352)
>   preempt_schedule_thunk() (?:?)
>   end_report() (mm/kasan/report.c:219)
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Zhang Cen <rollkingzzc@xxxxxxxxx>
>
> ---
> diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
> index d44987d4515c..f5070bbd6b57 100644
> --- a/net/bluetooth/bnep/core.c
> +++ b/net/bluetooth/bnep/core.c
> @@ -208,9 +208,14 @@ static int bnep_ctrl_set_mcfilter(struct bnep_session *s, u8 *data, int len)
>
>  static int bnep_rx_control(struct bnep_session *s, void *data, int len)
>  {
> -       u8  cmd = *(u8 *)data;
> +       u8  cmd;
>         int err = 0;
>
> +       if (len < 1)
> +               return -EILSEQ;
> +
> +       cmd = *(u8 *)data;
> +
>         data++;
>         len--;
>
> @@ -303,14 +308,21 @@ static int bnep_rx_frame(struct bnep_session *s, struct sk_buff *skb)
>
>         dev->stats.rx_bytes += skb->len;
>
> +       if (skb->len < 1)
> +               goto badframe;

Lets stop using skb->len directly and just skb_pull_data and theck if
the return is NULL.

>         type = *(u8 *) skb->data;
>         skb_pull(skb, 1);
> -       ctrl_type = *(u8 *)skb->data;
>
>         if ((type & BNEP_TYPE_MASK) >= sizeof(__bnep_rx_hlen))
>                 goto badframe;
>
>         if ((type & BNEP_TYPE_MASK) == BNEP_CONTROL) {
> +               if (skb->len < 1)
> +                       goto badframe;
> +
> +               ctrl_type = *(u8 *)skb->data;
> +
>                 if (bnep_rx_control(s, skb->data, skb->len) < 0) {
>                         dev->stats.tx_errors++;
>                         kfree_skb(skb);
> @@ -326,6 +338,9 @@ static int bnep_rx_frame(struct bnep_session *s, struct sk_buff *skb)
>                 switch (ctrl_type) {
>                 case BNEP_SETUP_CONN_REQ:
>                         /* Pull: ctrl type (1 b), len (1 b), data (len bytes) */
> +                       if (skb->len < 2)
> +                               goto badframe;
> +
>                         if (!skb_pull(skb, 2 + *(u8 *)(skb->data + 1) * 2))
>                                 goto badframe;
>                         break;



-- 
Luiz Augusto von Dentz