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

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

[Zhang Cen]

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

RE: [v4] Bluetooth: bnep: reject short frames before parsing

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 936 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1098453

---Test result---

Test Summary:
CheckPatch                    PASS      0.49 seconds
GitLint                       PASS      0.19 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      19.75 seconds
CheckAllWarning               PASS      25.20 seconds
CheckSparse                   PASS      22.00 seconds
BuildKernel32                 PASS      19.79 seconds
TestRunnerSetup               PASS      412.06 seconds
TestRunner_bnep-tester        PASS      14.59 seconds
IncrementalBuild              PASS      22.14 seconds



https://github.com/bluez/bluetooth-next/pull/228

---
Regards,
Linux Bluetooth

Re: [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

<bluez.test.bot@gmail.com> 于2026年5月21日周四 16:07写道：
>
> This is automated email and please do not reply to this email!
>
> Dear submitter,
>
> Thank you for submitting the patches to the linux bluetooth mailing list.
> This is a CI test results with your patch series:
> PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1098453
>
> ---Test result---
>
> Test Summary:
> CheckPatch                    PASS      0.49 seconds
> GitLint                       PASS      0.19 seconds
> SubjectPrefix                 PASS      0.06 seconds
> BuildKernel                   PASS      19.75 seconds
> CheckAllWarning               PASS      25.20 seconds
> CheckSparse                   PASS      22.00 seconds
> BuildKernel32                 PASS      19.79 seconds
> TestRunnerSetup               PASS      412.06 seconds
> TestRunner_bnep-tester        PASS      14.59 seconds
> IncrementalBuild              PASS      22.14 seconds
>
>
>
> https://github.com/bluez/bluetooth-next/pull/228
>
> ---
> Regards,
> Linux Bluetooth
>

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

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

[Luiz Augusto von Dentz]

Hi Zhang,

On Thu, May 21, 2026 at 1:26 AM Zhang Cen <rollkingzzc@gmail.com> wrote:
>
> 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@gmail.com>
> ---
>  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))

This still looks incorrect to me, why should we do * 2? If look at
BNEP_FILTER_NET_TYPE_SET it just use the header to pull using
get_unaligned, I know this is an existing problem but except if the
BNEP_SETUP_CONN_REQ header length works on a multiple of 2 this makes
no sense to me.

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


-- 
Luiz Augusto von Dentz

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

[Cen Zhang]

Hi Luiz,

Thanks for taking another look.

> This still looks incorrect to me, why should we do * 2? If look at
> BNEP_FILTER_NET_TYPE_SET it just use the header to pull using
> get_unaligned, I know this is an existing problem but except if the
> BNEP_SETUP_CONN_REQ header length works on a multiple of 2 this makes
> no sense to me.

The reason I used `* 2` there is that I read that byte as the UUID size,
not as a total payload length.

For BNEP_SETUP_CONN_REQ, the byte after the control type is the UUID
Size field.  The setup request then carries two UUIDs of that size: the
destination service UUID and the source service UUID.  So the intended
skip was:

  UUID Size field itself
  destination service UUID, uuid_size bytes
  source service UUID, uuid_size bytes

That is different from BNEP_FILTER_NET_TYPE_SET, where the header has a
16-bit length field and the value from get_unaligned_be16() is already
the total filter-list payload length.

That said, I agree the patch makes this much less clear than it should.
The comment says "data (len * 2 bytes)", which makes it look like a
generic length field, and the raw `* 2` is too opaque.

I can send a v5 that names the byte `uuid_size` and uses
`uuid_size + uuid_size` with a comment explaining that the two parts are
the destination and source service UUIDs.If I have misunderstood anything
here, sorry for the confusion. I would be happy to help with any
further changes
or testing.

Thanks,
Zhang Cen

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

[Luiz Augusto von Dentz]

Hi Cen,

On Thu, May 28, 2026 at 9:30 AM Cen Zhang <rollkingzzc@gmail.com> wrote:
>
> Hi Luiz,
>
> Thanks for taking another look.
>
> > This still looks incorrect to me, why should we do * 2? If look at
> > BNEP_FILTER_NET_TYPE_SET it just use the header to pull using
> > get_unaligned, I know this is an existing problem but except if the
> > BNEP_SETUP_CONN_REQ header length works on a multiple of 2 this makes
> > no sense to me.
>
> The reason I used `* 2` there is that I read that byte as the UUID size,
> not as a total payload length.
>
> For BNEP_SETUP_CONN_REQ, the byte after the control type is the UUID
> Size field.  The setup request then carries two UUIDs of that size: the
> destination service UUID and the source service UUID.  So the intended
> skip was:
>
>   UUID Size field itself
>   destination service UUID, uuid_size bytes
>   source service UUID, uuid_size bytes

This is what you get if you don't know where to look at:

struct bnep_setup_conn_req {
    uint8_t  type;
    uint8_t  ctrl;
    uint8_t  uuid_size;
    uint8_t  service[0];
} __attribute__((packed));
https://github.com/bluez/bluez/blob/master/lib/bluetooth/bnep.h#L79

It is in fact a tuple of src + dst service as parsed by btmon:

https://github.com/bluez/bluez/blob/master/monitor/bnep.c#L104

> That is different from BNEP_FILTER_NET_TYPE_SET, where the header has a
> 16-bit length field and the value from get_unaligned_be16() is already
> the total filter-list payload length.
>
> That said, I agree the patch makes this much less clear than it should.
> The comment says "data (len * 2 bytes)", which makes it look like a
> generic length field, and the raw `* 2` is too opaque.
>
> I can send a v5 that names the byte `uuid_size` and uses
> `uuid_size + uuid_size` with a comment explaining that the two parts are
> the destination and source service UUIDs.If I have misunderstood anything
> here, sorry for the confusion. I would be happy to help with any
> further changes
> or testing.

Probably just reference the above, but the fact that you couldn't
mention any of the BlueZ references really bothers me. Although the
result was correct, the lack of references makes me believe I'm just
arguing with an AI prompt which doesn't have BlueZ userspace loaded as
context.


-- 
Luiz Augusto von Dentz

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

[Cen Zhang]

Hi Luiz,

On Fri, May 29, 2026 at 4:48 AM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> This is what you get if you don't know where to look at:
>
> struct bnep_setup_conn_req {
>     uint8_t  type;
>     uint8_t  ctrl;
>     uint8_t  uuid_size;
>     uint8_t  service[0];
> } __attribute__((packed));
> https://github.com/bluez/bluez/blob/master/lib/bluetooth/bnep.h#L79
>
> It is in fact a tuple of src + dst service as parsed by btmon:
> https://github.com/bluez/bluez/blob/master/monitor/bnep.c#L104

Right, service[] is the dst + src service tuple, each uuid_size, so the
skip after the control type is 1 + 2 * uuid_size.  For v5, I'll name the
byte uuid_size and make the setup-request layout explicit in the comment
so it is not an opaque length.

> Probably just reference the above, but the fact that you couldn't
> mention any of the BlueZ references really bothers me. Although the
> result was correct, the lack of references makes me believe I'm just
> arguing with an AI prompt which doesn't have BlueZ userspace loaded as
> context.

You are right to call this out, and I'm sorry for the noise.

The initial lead for this issue did come from AI assistance.  I then
reproduced the bug under KASAN and checked the kernel-side code, but I
should have done a better job before replying.

I've also been reading the recent kernel discussions around AI-assisted
reports and patches, and I understand the concern better now.  A patch
is not just a bug report saying what can go wrong; it also has to
explain why the change is correct in the subsystem context, so reviewers
are not left doing that work from scratch.  I was thinking too much like
a bug reporter there and not carefully enough as a patch author.  I will
not rely on LLM output as a substitute for reading and understanding the
code.

I'll send v5 with the above and will be more careful with Bluetooth
patches and follow-ups going forward.

Thanks for taking the time to point this out.

Zhang Cen