1. Patchew
  2. linux
  3. View series

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

Zhang Cen posted 1 patch 1 week, 2 days ago
There is a newer version of this series
[PATCH v2] Bluetooth: bnep: reject short frames before parsing
Posted by Zhang Cen 1 week, 2 days ago 
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@gmail.com>

---
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;
+
 	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;
Re: [PATCH v2] Bluetooth: bnep: reject short frames before parsing
Posted by Luiz Augusto von Dentz 6 days, 9 hours ago 
Hi,

On Fri, May 15, 2026 at 8:44 PM Zhang Cen <rollkingzzc@gmail.com> 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@gmail.com>
>
> ---
> 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
Re: [PATCH v2] Bluetooth: bnep: reject short frames before parsing
Posted by Cen Zhang 6 days, 4 hours ago 
Hi Luiz,

Thanks for the review.

On Tue, May 19, 2026 at 05:39:00AM +0000, Luiz Augusto von Dentz wrote:

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

Sure, that makes sense. I will send a v3 using skb_pull_data() as
suggested.

Best regards,
Zhang Cen

Patchew 2.1-devel-ea86937

© 2016 - 2026 Red Hat, Inc.