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

[Cen Zhang]

Hi Luiz,

Gentle ping on this patch.  The current version follows the previous
review feedback and the bluetooth test bot reported no failures.

Please let me know if you would prefer any further changes.

Thanks,
Zhang Cen

Zhang Cen <rollkingzzc@xxxxxxxxx> 于2026年5月21日周四 13:26写道：
>
> A BNEP peer can send a short BNEP SDU. bnep_rx_frame() reads the
> packet type byte immediately and, for control packets, reads the control
> opcode and setup UUID-size byte before proving that those bytes are
> present. bnep_rx_control() also dereferences the control opcode without
> rejecting an empty control payload.
>
> Use skb_pull_data() for the fixed fields in bnep_rx_frame() so a NULL
> return gates each dereference. Split the control handler so the frame
> path can pass an opcode that has already been pulled, and keep the
> byte-buffer wrapper for extension control payloads.
>
> Validation reproduced this kernel report:
> KASAN slab-out-of-bounds in bnep_rx_frame.isra.0+0x130c/0x1790
> The buggy address belongs to the object at ffff88800c0f7908 which belongs
> to the cache kmalloc-8 of size 8
> The buggy address is located 0 bytes to the right of allocated 1-byte
> region [ffff88800c0f7908, ffff88800c0f7909)
> Read of size 1
> Call trace:
>   dump_stack_lvl+0xb3/0x140 (?:?)
>   print_address_description+0x57/0x3a0 (?:?)
>   bnep_rx_frame+0x130c/0x1790 (net/bluetooth/bnep/core.c:306)
>   print_report+0xb9/0x2b0 (?:?)
>   __virt_addr_valid+0x1ba/0x3a0 (?:?)
>   srso_alias_return_thunk+0x5/0xfbef5 (?:?)
>   kasan_addr_to_slab+0x21/0x60 (?:?)
>   kasan_report+0xe0/0x110 (?:?)
>   process_one_work+0xfce/0x17e0 (kernel/workqueue.c:3200)
>   worker_thread+0x65c/0xe40 (?:?)
>   __kthread_parkme+0x184/0x230 (?:?)
>   kthread+0x35e/0x470 (?:?)
>   _raw_spin_unlock_irq+0x28/0x50 (?:?)
>   ret_from_fork+0x586/0x870 (?:?)
>   __switch_to+0x74f/0xdc0 (?:?)
>   ret_from_fork_asm+0x1a/0x30 (?:?)
>
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: Zhang Cen <rollkingzzc@xxxxxxxxx>
> ---
>  net/bluetooth/bnep/core.c | 51 ++++++++++++++++++++++++---------------
>  1 file changed, 31 insertions(+), 20 deletions(-)
>
> diff --git a/net/bluetooth/bnep/core.c b/net/bluetooth/bnep/core.c
> index 853c8d764..156cd3026 100644
> --- a/net/bluetooth/bnep/core.c
> +++ b/net/bluetooth/bnep/core.c
> @@ -206,14 +206,11 @@ static int bnep_ctrl_set_mcfilter(struct bnep_session *s, u8 *data, int len)
>         return 0;
>  }
>
> -static int bnep_rx_control(struct bnep_session *s, void *data, int len)
> +static int bnep_rx_control_cmd(struct bnep_session *s, u8 cmd, void *data,
> +                              int len)
>  {
> -       u8  cmd = *(u8 *)data;
>         int err = 0;
>
> -       data++;
> -       len--;
> -
>         switch (cmd) {
>         case BNEP_CMD_NOT_UNDERSTOOD:
>         case BNEP_SETUP_CONN_RSP:
> @@ -254,6 +251,14 @@ static int bnep_rx_control(struct bnep_session *s, void *data, int len)
>         return err;
>  }
>
> +static int bnep_rx_control(struct bnep_session *s, void *data, int len)
> +{
> +       if (len < 1)
> +               return -EILSEQ;
> +
> +       return bnep_rx_control_cmd(s, *(u8 *)data, data + 1, len - 1);
> +}
> +
>  static int bnep_rx_extension(struct bnep_session *s, struct sk_buff *skb)
>  {
>         struct bnep_ext_hdr *h;
> @@ -299,19 +304,26 @@ static int bnep_rx_frame(struct bnep_session *s, struct sk_buff *skb)
>  {
>         struct net_device *dev = s->dev;
>         struct sk_buff *nskb;
> +       u8 *data;
>         u8 type, ctrl_type;
>
>         dev->stats.rx_bytes += skb->len;
>
> -       type = *(u8 *) skb->data;
> -       skb_pull(skb, 1);
> -       ctrl_type = *(u8 *)skb->data;
> +       data = skb_pull_data(skb, sizeof(type));
> +       if (!data)
> +               goto badframe;
> +       type = *data;
>
>         if ((type & BNEP_TYPE_MASK) >= sizeof(__bnep_rx_hlen))
>                 goto badframe;
>
>         if ((type & BNEP_TYPE_MASK) == BNEP_CONTROL) {
> -               if (bnep_rx_control(s, skb->data, skb->len) < 0) {
> +               data = skb_pull_data(skb, sizeof(ctrl_type));
> +               if (!data)
> +                       goto badframe;
> +               ctrl_type = *data;
> +
> +               if (bnep_rx_control_cmd(s, ctrl_type, skb->data, skb->len) < 0) {
>                         dev->stats.tx_errors++;
>                         kfree_skb(skb);
>                         return 0;
> @@ -325,23 +337,22 @@ static int bnep_rx_frame(struct bnep_session *s, struct sk_buff *skb)
>                 /* Verify and pull ctrl message since it's already processed */
>                 switch (ctrl_type) {
>                 case BNEP_SETUP_CONN_REQ:
> -                       /* Pull: ctrl type (1 b), len (1 b), data (len bytes) */
> -                       if (!skb_pull(skb, 2 + *(u8 *)(skb->data + 1) * 2))
> +                       /* Pull: len (1 b), data (len * 2 bytes) */
> +                       data = skb_pull_data(skb, 1);
> +                       if (!data)
> +                               goto badframe;
> +                       if (!skb_pull(skb, *data * 2))
>                                 goto badframe;
>                         break;
>                 case BNEP_FILTER_MULTI_ADDR_SET:
> -               case BNEP_FILTER_NET_TYPE_SET: {
> -                       u8 *hdr;
> -
> -                       /* Pull ctrl type (1 b) + len (2 b) */
> -                       hdr = skb_pull_data(skb, 3);
> -                       if (!hdr)
> +               case BNEP_FILTER_NET_TYPE_SET:
> +                       /* Pull: len (2 b), data (len bytes) */
> +                       data = skb_pull_data(skb, sizeof(u16));
> +                       if (!data)
>                                 goto badframe;
> -                       /* Pull data (len bytes); length is big-endian */
> -                       if (!skb_pull(skb, get_unaligned_be16(&hdr[1])))
> +                       if (!skb_pull(skb, get_unaligned_be16(data)))
>                                 goto badframe;
>                         break;
> -               }
>                 default:
>                         kfree_skb(skb);
>                         return 0;
> --
> 2.43.0
>