Bluetooth: HIDP: add missing length check for incoming frames

[PATCH] Bluetooth: HIDP: add missing length check for incoming frames

Posted by Jiale Yao 1 week ago

In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
delivered via the HIDP control or interrupt channel causes an
out-of-bounds read.

Add pskb_may_pull(skb, 1) guards before both reads, matching the fix
in commit 6770d3a8acdf ("Bluetooth: bnep: reject short frames before
parsing") which addressed the same class of bug in BNEP.

Assisted-by: Claude:deepseek-v4-pro
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 net/bluetooth/hidp/core.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 0e24c5e2955e..6c7da8aca732 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -565,6 +565,8 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
 
 	BT_DBG("session %p skb %p len %u", session, skb, skb->len);
 
+	if (!pskb_may_pull(skb, 1))
+		return;
 	hdr = skb->data[0];
 	skb_pull(skb, 1);
 
@@ -601,6 +603,8 @@ static void hidp_recv_intr_frame(struct hidp_session *session,
 
 	BT_DBG("session %p skb %p len %u", session, skb, skb->len);
 
+	if (!pskb_may_pull(skb, 1))
+		return;
 	hdr = skb->data[0];
 	skb_pull(skb, 1);
 
-- 
2.34.1

Re: [PATCH] Bluetooth: HIDP: add missing length check for incoming frames

Posted by Luiz Augusto von Dentz 1 week ago

Hi,

On Fri, Jul 17, 2026 at 11:33 AM Jiale Yao <yaojiale02@163.com> wrote:
>
> In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
> read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
> delivered via the HIDP control or interrupt channel causes an
> out-of-bounds read.
>
> Add pskb_may_pull(skb, 1) guards before both reads, matching the fix
> in commit 6770d3a8acdf ("Bluetooth: bnep: reject short frames before
> parsing") which addressed the same class of bug in BNEP.
>
> Assisted-by: Claude:deepseek-v4-pro
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  net/bluetooth/hidp/core.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 0e24c5e2955e..6c7da8aca732 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -565,6 +565,8 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
>
>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>
> +       if (!pskb_may_pull(skb, 1))
> +               return;

We can probably replace this with skb_pull_data.

>         hdr = skb->data[0];
>         skb_pull(skb, 1);
>
> @@ -601,6 +603,8 @@ static void hidp_recv_intr_frame(struct hidp_session *session,
>
>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>
> +       if (!pskb_may_pull(skb, 1))
> +               return;

Ditto.

>         hdr = skb->data[0];
>         skb_pull(skb, 1);
>
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz

Re:Re: [PATCH v2] Bluetooth: HIDP: add missing length check for incoming frames

Posted by jiale yao 5 days, 22 hours ago






In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
delivered via the HIDP control or interrupt channel causes an
out-of-bounds read.

Use skb_pull_data(skb, 1) which combines the length check and pull
in one operation, matching the existing pattern in hidp_input_report()
and the fix in commit 6770d3a8acdf ("Bluetooth: bnep: reject short
frames before parsing") which addressed the same class of bug in BNEP.

Assisted-by: Claude:deepseek-v4-pro
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 net/bluetooth/hidp/core.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 0e24c5e2955e..bb61602b02e1 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -560,16 +560,18 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
 static void hidp_recv_ctrl_frame(struct hidp_session *session,
 					struct sk_buff *skb)
 {
-	unsigned char hdr, type, param;
+	unsigned char *hdr;
+	unsigned char type, param;
 	int free_skb = 1;
 
 	BT_DBG("session %p skb %p len %u", session, skb, skb->len);
 
-	hdr = skb->data[0];
-	skb_pull(skb, 1);
+	hdr = skb_pull_data(skb, 1);
+	if (!hdr)
+		return;
 
-	type = hdr & HIDP_HEADER_TRANS_MASK;
-	param = hdr & HIDP_HEADER_PARAM_MASK;
+	type = *hdr & HIDP_HEADER_TRANS_MASK;
+	param = *hdr & HIDP_HEADER_PARAM_MASK;
 
 	switch (type) {
 	case HIDP_TRANS_HANDSHAKE:
@@ -597,14 +599,15 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
 static void hidp_recv_intr_frame(struct hidp_session *session,
 				struct sk_buff *skb)
 {
-	unsigned char hdr;
+	unsigned char *hdr;
 
 	BT_DBG("session %p skb %p len %u", session, skb, skb->len);
 
-	hdr = skb->data[0];
-	skb_pull(skb, 1);
+	hdr = skb_pull_data(skb, 1);
+	if (!hdr)
+		return;
 
-	if (hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
+	if (*hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
 		hidp_set_timer(session);
 
 		if (session->input)
-- 
2.34.1












At 2026-07-17 23:40:11, "Luiz Augusto von Dentz" <luiz.dentz@gmail.com> wrote:
>Hi,
>
>On Fri, Jul 17, 2026 at 11:33 AM Jiale Yao <yaojiale02@163.com> wrote:
>>
>> In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
>> read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
>> delivered via the HIDP control or interrupt channel causes an
>> out-of-bounds read.
>>
>> Add pskb_may_pull(skb, 1) guards before both reads, matching the fix
>> in commit 6770d3a8acdf ("Bluetooth: bnep: reject short frames before
>> parsing") which addressed the same class of bug in BNEP.
>>
>> Assisted-by: Claude:deepseek-v4-pro
>> Signed-off-by: Jiale Yao <yaojiale02@163.com>
>> ---
>>  net/bluetooth/hidp/core.c | 4 ++++
>>  1 file changed, 4 insertions(+)
>>
>> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
>> index 0e24c5e2955e..6c7da8aca732 100644
>> --- a/net/bluetooth/hidp/core.c
>> +++ b/net/bluetooth/hidp/core.c
>> @@ -565,6 +565,8 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
>>
>>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>>
>> +       if (!pskb_may_pull(skb, 1))
>> +               return;
>
>We can probably replace this with skb_pull_data.
>
>>         hdr = skb->data[0];
>>         skb_pull(skb, 1);
>>
>> @@ -601,6 +603,8 @@ static void hidp_recv_intr_frame(struct hidp_session *session,
>>
>>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>>
>> +       if (!pskb_may_pull(skb, 1))
>> +               return;
>
>Ditto.
>
>>         hdr = skb->data[0];
>>         skb_pull(skb, 1);
>>
>> --
>> 2.34.1
>>
>
>
>-- 
>Luiz Augusto von Dentz

Re: Re: [PATCH v2] Bluetooth: HIDP: add missing length check for incoming frames

Posted by Luiz Augusto von Dentz 4 days, 8 hours ago

Hi,

On Sun, Jul 19, 2026 at 1:38 AM jiale yao <19888972804@163.com> wrote:
>
>
>
>
>
>
>
> In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
> read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
> delivered via the HIDP control or interrupt channel causes an
> out-of-bounds read.
>
> Use skb_pull_data(skb, 1) which combines the length check and pull
> in one operation, matching the existing pattern in hidp_input_report()
> and the fix in commit 6770d3a8acdf ("Bluetooth: bnep: reject short
> frames before parsing") which addressed the same class of bug in BNEP.
>
> Assisted-by: Claude:deepseek-v4-pro
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  net/bluetooth/hidp/core.c | 21 ++++++++++++---------
>  1 file changed, 12 insertions(+), 9 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 0e24c5e2955e..bb61602b02e1 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -560,16 +560,18 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
>  static void hidp_recv_ctrl_frame(struct hidp_session *session,
>                                         struct sk_buff *skb)
>  {
> -       unsigned char hdr, type, param;
> +       unsigned char *hdr;
> +       unsigned char type, param;
>         int free_skb = 1;
>
>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>
> -       hdr = skb->data[0];
> -       skb_pull(skb, 1);
> +       hdr = skb_pull_data(skb, 1);
> +       if (!hdr)
> +               return;

https://sashiko.dev/#/patchset/20260717153238.2002330-1-yaojiale02%40163.com

It seems that by early returning we are leaking the skb.

>
> -       type = hdr & HIDP_HEADER_TRANS_MASK;
> -       param = hdr & HIDP_HEADER_PARAM_MASK;
> +       type = *hdr & HIDP_HEADER_TRANS_MASK;
> +       param = *hdr & HIDP_HEADER_PARAM_MASK;
>
>         switch (type) {
>         case HIDP_TRANS_HANDSHAKE:
> @@ -597,14 +599,15 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
>  static void hidp_recv_intr_frame(struct hidp_session *session,
>                                 struct sk_buff *skb)
>  {
> -       unsigned char hdr;
> +       unsigned char *hdr;
>
>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>
> -       hdr = skb->data[0];
> -       skb_pull(skb, 1);
> +       hdr = skb_pull_data(skb, 1);
> +       if (!hdr)
> +               return;
>
> -       if (hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
> +       if (*hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
>                 hidp_set_timer(session);
>
>                 if (session->input)
> --
> 2.34.1
>
>
>
>
>
>
>
>
>
>
>
>
> At 2026-07-17 23:40:11, "Luiz Augusto von Dentz" <luiz.dentz@gmail.com> wrote:
> >Hi,
> >
> >On Fri, Jul 17, 2026 at 11:33 AM Jiale Yao <yaojiale02@163.com> wrote:
> >>
> >> In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
> >> read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
> >> delivered via the HIDP control or interrupt channel causes an
> >> out-of-bounds read.
> >>
> >> Add pskb_may_pull(skb, 1) guards before both reads, matching the fix
> >> in commit 6770d3a8acdf ("Bluetooth: bnep: reject short frames before
> >> parsing") which addressed the same class of bug in BNEP.
> >>
> >> Assisted-by: Claude:deepseek-v4-pro
> >> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> >> ---
> >>  net/bluetooth/hidp/core.c | 4 ++++
> >>  1 file changed, 4 insertions(+)
> >>
> >> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> >> index 0e24c5e2955e..6c7da8aca732 100644
> >> --- a/net/bluetooth/hidp/core.c
> >> +++ b/net/bluetooth/hidp/core.c
> >> @@ -565,6 +565,8 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
> >>
> >>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
> >>
> >> +       if (!pskb_may_pull(skb, 1))
> >> +               return;
> >
> >We can probably replace this with skb_pull_data.
> >
> >>         hdr = skb->data[0];
> >>         skb_pull(skb, 1);
> >>
> >> @@ -601,6 +603,8 @@ static void hidp_recv_intr_frame(struct hidp_session *session,
> >>
> >>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
> >>
> >> +       if (!pskb_may_pull(skb, 1))
> >> +               return;
> >
> >Ditto.
> >
> >>         hdr = skb->data[0];
> >>         skb_pull(skb, 1);
> >>
> >> --
> >> 2.34.1
> >>
> >
> >
> >--
> >Luiz Augusto von Dentz



-- 
Luiz Augusto von Dentz

Re:Re: Re: [PATCH v3] Bluetooth: HIDP: add missing length check for incoming frames

Posted by jiale yao 3 days, 22 hours ago

In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
delivered via the HIDP control or interrupt channel causes an
out-of-bounds read.

Use skb_pull_data(skb, 1) which combines the length check and pull
in one operation, matching the existing pattern in hidp_input_report()
and the fix in commit 6770d3a8acdf ("Bluetooth: bnep: reject short
frames before parsing") which addressed the same class of bug in BNEP.

Assisted-by: Claude:deepseek-v4-pro
Signed-off-by: Jiale Yao <yaojiale02@163.com>
---
 net/bluetooth/hidp/core.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
index 0e24c5e2955e..e29fe66f01d4 100644
--- a/net/bluetooth/hidp/core.c
+++ b/net/bluetooth/hidp/core.c
@@ -560,16 +560,18 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
 static void hidp_recv_ctrl_frame(struct hidp_session *session,
 					struct sk_buff *skb)
 {
-	unsigned char hdr, type, param;
+	unsigned char *hdr;
+	unsigned char type, param;
 	int free_skb = 1;
 
 	BT_DBG("session %p skb %p len %u", session, skb, skb->len);
 
-	hdr = skb->data[0];
-	skb_pull(skb, 1);
+	hdr = skb_pull_data(skb, 1);
+	if (!hdr)
+		goto failed;
 
-	type = hdr & HIDP_HEADER_TRANS_MASK;
-	param = hdr & HIDP_HEADER_PARAM_MASK;
+	type = *hdr & HIDP_HEADER_TRANS_MASK;
+	param = *hdr & HIDP_HEADER_PARAM_MASK;
 
 	switch (type) {
 	case HIDP_TRANS_HANDSHAKE:
@@ -590,6 +592,7 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
 		break;
 	}
 
+failed:
 	if (free_skb)
 		kfree_skb(skb);
 }
@@ -597,14 +600,15 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
 static void hidp_recv_intr_frame(struct hidp_session *session,
 				struct sk_buff *skb)
 {
-	unsigned char hdr;
+	unsigned char *hdr;
 
 	BT_DBG("session %p skb %p len %u", session, skb, skb->len);
 
-	hdr = skb->data[0];
-	skb_pull(skb, 1);
+	hdr = skb_pull_data(skb, 1);
+	if (!hdr)
+		goto failed;
 
-	if (hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
+	if (*hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
 		hidp_set_timer(session);
 
 		if (session->input)
@@ -616,9 +620,10 @@ static void hidp_recv_intr_frame(struct hidp_session *session,
 			BT_DBG("report len %d", skb->len);
 		}
 	} else {
-		BT_DBG("Unsupported protocol header 0x%02x", hdr);
+		BT_DBG("Unsupported protocol header 0x%02x", *hdr);
 	}
 
+failed:
 	kfree_skb(skb);
 }
 
-- 
2.34.1







At 2026-07-21 03:47:21, "Luiz Augusto von Dentz" <luiz.dentz@gmail.com> wrote:
>Hi,
>
>On Sun, Jul 19, 2026 at 1:38 AM jiale yao <19888972804@163.com> wrote:
>>
>>
>>
>>
>>
>>
>>
>> In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
>> read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
>> delivered via the HIDP control or interrupt channel causes an
>> out-of-bounds read.
>>
>> Use skb_pull_data(skb, 1) which combines the length check and pull
>> in one operation, matching the existing pattern in hidp_input_report()
>> and the fix in commit 6770d3a8acdf ("Bluetooth: bnep: reject short
>> frames before parsing") which addressed the same class of bug in BNEP.
>>
>> Assisted-by: Claude:deepseek-v4-pro
>> Signed-off-by: Jiale Yao <yaojiale02@163.com>
>> ---
>>  net/bluetooth/hidp/core.c | 21 ++++++++++++---------
>>  1 file changed, 12 insertions(+), 9 deletions(-)
>>
>> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
>> index 0e24c5e2955e..bb61602b02e1 100644
>> --- a/net/bluetooth/hidp/core.c
>> +++ b/net/bluetooth/hidp/core.c
>> @@ -560,16 +560,18 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
>>  static void hidp_recv_ctrl_frame(struct hidp_session *session,
>>                                         struct sk_buff *skb)
>>  {
>> -       unsigned char hdr, type, param;
>> +       unsigned char *hdr;
>> +       unsigned char type, param;
>>         int free_skb = 1;
>>
>>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>>
>> -       hdr = skb->data[0];
>> -       skb_pull(skb, 1);
>> +       hdr = skb_pull_data(skb, 1);
>> +       if (!hdr)
>> +               return;
>
>https://sashiko.dev/#/patchset/20260717153238.2002330-1-yaojiale02%40163.com
>
>It seems that by early returning we are leaking the skb.
>
>>
>> -       type = hdr & HIDP_HEADER_TRANS_MASK;
>> -       param = hdr & HIDP_HEADER_PARAM_MASK;
>> +       type = *hdr & HIDP_HEADER_TRANS_MASK;
>> +       param = *hdr & HIDP_HEADER_PARAM_MASK;
>>
>>         switch (type) {
>>         case HIDP_TRANS_HANDSHAKE:
>> @@ -597,14 +599,15 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
>>  static void hidp_recv_intr_frame(struct hidp_session *session,
>>                                 struct sk_buff *skb)
>>  {
>> -       unsigned char hdr;
>> +       unsigned char *hdr;
>>
>>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>>
>> -       hdr = skb->data[0];
>> -       skb_pull(skb, 1);
>> +       hdr = skb_pull_data(skb, 1);
>> +       if (!hdr)
>> +               return;
>>
>> -       if (hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
>> +       if (*hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
>>                 hidp_set_timer(session);
>>
>>                 if (session->input)
>> --
>> 2.34.1
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> At 2026-07-17 23:40:11, "Luiz Augusto von Dentz" <luiz.dentz@gmail.com> wrote:
>> >Hi,
>> >
>> >On Fri, Jul 17, 2026 at 11:33 AM Jiale Yao <yaojiale02@163.com> wrote:
>> >>
>> >> In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
>> >> read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
>> >> delivered via the HIDP control or interrupt channel causes an
>> >> out-of-bounds read.
>> >>
>> >> Add pskb_may_pull(skb, 1) guards before both reads, matching the fix
>> >> in commit 6770d3a8acdf ("Bluetooth: bnep: reject short frames before
>> >> parsing") which addressed the same class of bug in BNEP.
>> >>
>> >> Assisted-by: Claude:deepseek-v4-pro
>> >> Signed-off-by: Jiale Yao <yaojiale02@163.com>
>> >> ---
>> >>  net/bluetooth/hidp/core.c | 4 ++++
>> >>  1 file changed, 4 insertions(+)
>> >>
>> >> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
>> >> index 0e24c5e2955e..6c7da8aca732 100644
>> >> --- a/net/bluetooth/hidp/core.c
>> >> +++ b/net/bluetooth/hidp/core.c
>> >> @@ -565,6 +565,8 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
>> >>
>> >>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>> >>
>> >> +       if (!pskb_may_pull(skb, 1))
>> >> +               return;
>> >
>> >We can probably replace this with skb_pull_data.
>> >
>> >>         hdr = skb->data[0];
>> >>         skb_pull(skb, 1);
>> >>
>> >> @@ -601,6 +603,8 @@ static void hidp_recv_intr_frame(struct hidp_session *session,
>> >>
>> >>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>> >>
>> >> +       if (!pskb_may_pull(skb, 1))
>> >> +               return;
>> >
>> >Ditto.
>> >
>> >>         hdr = skb->data[0];
>> >>         skb_pull(skb, 1);
>> >>
>> >> --
>> >> 2.34.1
>> >>
>> >
>> >
>> >--
>> >Luiz Augusto von Dentz
>
>
>
>-- 
>Luiz Augusto von Dentz

Re: Re: Re: [PATCH v3] Bluetooth: HIDP: add missing length check for incoming frames

Posted by Muhammad Bilal 3 days, 4 hours ago

Hi Jiale,

Looks correct. Both goto failed paths free the skb properly now, and
the intr_frame goto also avoids a NULL hdr deref.

Reviewed-by: Muhammad Bilal <meatuni001@gmail.com>

Thanks,
Muhammad Bilal

On Tue, Jul 21, 2026 at 10:55 AM jiale yao <19888972804@163.com> wrote:
>
> In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
> read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
> delivered via the HIDP control or interrupt channel causes an
> out-of-bounds read.
>
> Use skb_pull_data(skb, 1) which combines the length check and pull
> in one operation, matching the existing pattern in hidp_input_report()
> and the fix in commit 6770d3a8acdf ("Bluetooth: bnep: reject short
> frames before parsing") which addressed the same class of bug in BNEP.
>
> Assisted-by: Claude:deepseek-v4-pro
> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> ---
>  net/bluetooth/hidp/core.c | 25 +++++++++++++++----------
>  1 file changed, 15 insertions(+), 10 deletions(-)
>
> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> index 0e24c5e2955e..e29fe66f01d4 100644
> --- a/net/bluetooth/hidp/core.c
> +++ b/net/bluetooth/hidp/core.c
> @@ -560,16 +560,18 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
>  static void hidp_recv_ctrl_frame(struct hidp_session *session,
>                                         struct sk_buff *skb)
>  {
> -       unsigned char hdr, type, param;
> +       unsigned char *hdr;
> +       unsigned char type, param;
>         int free_skb = 1;
>
>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>
> -       hdr = skb->data[0];
> -       skb_pull(skb, 1);
> +       hdr = skb_pull_data(skb, 1);
> +       if (!hdr)
> +               goto failed;
>
> -       type = hdr & HIDP_HEADER_TRANS_MASK;
> -       param = hdr & HIDP_HEADER_PARAM_MASK;
> +       type = *hdr & HIDP_HEADER_TRANS_MASK;
> +       param = *hdr & HIDP_HEADER_PARAM_MASK;
>
>         switch (type) {
>         case HIDP_TRANS_HANDSHAKE:
> @@ -590,6 +592,7 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
>                 break;
>         }
>
> +failed:
>         if (free_skb)
>                 kfree_skb(skb);
>  }
> @@ -597,14 +600,15 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
>  static void hidp_recv_intr_frame(struct hidp_session *session,
>                                 struct sk_buff *skb)
>  {
> -       unsigned char hdr;
> +       unsigned char *hdr;
>
>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
>
> -       hdr = skb->data[0];
> -       skb_pull(skb, 1);
> +       hdr = skb_pull_data(skb, 1);
> +       if (!hdr)
> +               goto failed;
>
> -       if (hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
> +       if (*hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
>                 hidp_set_timer(session);
>
>                 if (session->input)
> @@ -616,9 +620,10 @@ static void hidp_recv_intr_frame(struct hidp_session *session,
>                         BT_DBG("report len %d", skb->len);
>                 }
>         } else {
> -               BT_DBG("Unsupported protocol header 0x%02x", hdr);
> +               BT_DBG("Unsupported protocol header 0x%02x", *hdr);
>         }
>
> +failed:
>         kfree_skb(skb);
>  }
>
> --
> 2.34.1
>
>
>
>
>
>
>
> At 2026-07-21 03:47:21, "Luiz Augusto von Dentz" <luiz.dentz@gmail.com> wrote:
> >Hi,
> >
> >On Sun, Jul 19, 2026 at 1:38 AM jiale yao <19888972804@163.com> wrote:
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >> In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
> >> read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
> >> delivered via the HIDP control or interrupt channel causes an
> >> out-of-bounds read.
> >>
> >> Use skb_pull_data(skb, 1) which combines the length check and pull
> >> in one operation, matching the existing pattern in hidp_input_report()
> >> and the fix in commit 6770d3a8acdf ("Bluetooth: bnep: reject short
> >> frames before parsing") which addressed the same class of bug in BNEP.
> >>
> >> Assisted-by: Claude:deepseek-v4-pro
> >> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> >> ---
> >>  net/bluetooth/hidp/core.c | 21 ++++++++++++---------
> >>  1 file changed, 12 insertions(+), 9 deletions(-)
> >>
> >> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> >> index 0e24c5e2955e..bb61602b02e1 100644
> >> --- a/net/bluetooth/hidp/core.c
> >> +++ b/net/bluetooth/hidp/core.c
> >> @@ -560,16 +560,18 @@ static int hidp_process_data(struct hidp_session *session, struct sk_buff *skb,
> >>  static void hidp_recv_ctrl_frame(struct hidp_session *session,
> >>                                         struct sk_buff *skb)
> >>  {
> >> -       unsigned char hdr, type, param;
> >> +       unsigned char *hdr;
> >> +       unsigned char type, param;
> >>         int free_skb = 1;
> >>
> >>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
> >>
> >> -       hdr = skb->data[0];
> >> -       skb_pull(skb, 1);
> >> +       hdr = skb_pull_data(skb, 1);
> >> +       if (!hdr)
> >> +               return;
> >
> >https://sashiko.dev/#/patchset/20260717153238.2002330-1-yaojiale02%40163.com
> >
> >It seems that by early returning we are leaking the skb.
> >
> >>
> >> -       type = hdr & HIDP_HEADER_TRANS_MASK;
> >> -       param = hdr & HIDP_HEADER_PARAM_MASK;
> >> +       type = *hdr & HIDP_HEADER_TRANS_MASK;
> >> +       param = *hdr & HIDP_HEADER_PARAM_MASK;
> >>
> >>         switch (type) {
> >>         case HIDP_TRANS_HANDSHAKE:
> >> @@ -597,14 +599,15 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
> >>  static void hidp_recv_intr_frame(struct hidp_session *session,
> >>                                 struct sk_buff *skb)
> >>  {
> >> -       unsigned char hdr;
> >> +       unsigned char *hdr;
> >>
> >>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
> >>
> >> -       hdr = skb->data[0];
> >> -       skb_pull(skb, 1);
> >> +       hdr = skb_pull_data(skb, 1);
> >> +       if (!hdr)
> >> +               return;
> >>
> >> -       if (hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
> >> +       if (*hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) {
> >>                 hidp_set_timer(session);
> >>
> >>                 if (session->input)
> >> --
> >> 2.34.1
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >>
> >> At 2026-07-17 23:40:11, "Luiz Augusto von Dentz" <luiz.dentz@gmail.com> wrote:
> >> >Hi,
> >> >
> >> >On Fri, Jul 17, 2026 at 11:33 AM Jiale Yao <yaojiale02@163.com> wrote:
> >> >>
> >> >> In hidp_recv_ctrl_frame() and hidp_recv_intr_frame(), skb->data[0] is
> >> >> read without verifying that skb->len >= 1.  A zero-length L2CAP PDU
> >> >> delivered via the HIDP control or interrupt channel causes an
> >> >> out-of-bounds read.
> >> >>
> >> >> Add pskb_may_pull(skb, 1) guards before both reads, matching the fix
> >> >> in commit 6770d3a8acdf ("Bluetooth: bnep: reject short frames before
> >> >> parsing") which addressed the same class of bug in BNEP.
> >> >>
> >> >> Assisted-by: Claude:deepseek-v4-pro
> >> >> Signed-off-by: Jiale Yao <yaojiale02@163.com>
> >> >> ---
> >> >>  net/bluetooth/hidp/core.c | 4 ++++
> >> >>  1 file changed, 4 insertions(+)
> >> >>
> >> >> diff --git a/net/bluetooth/hidp/core.c b/net/bluetooth/hidp/core.c
> >> >> index 0e24c5e2955e..6c7da8aca732 100644
> >> >> --- a/net/bluetooth/hidp/core.c
> >> >> +++ b/net/bluetooth/hidp/core.c
> >> >> @@ -565,6 +565,8 @@ static void hidp_recv_ctrl_frame(struct hidp_session *session,
> >> >>
> >> >>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
> >> >>
> >> >> +       if (!pskb_may_pull(skb, 1))
> >> >> +               return;
> >> >
> >> >We can probably replace this with skb_pull_data.
> >> >
> >> >>         hdr = skb->data[0];
> >> >>         skb_pull(skb, 1);
> >> >>
> >> >> @@ -601,6 +603,8 @@ static void hidp_recv_intr_frame(struct hidp_session *session,
> >> >>
> >> >>         BT_DBG("session %p skb %p len %u", session, skb, skb->len);
> >> >>
> >> >> +       if (!pskb_may_pull(skb, 1))
> >> >> +               return;
> >> >
> >> >Ditto.
> >> >
> >> >>         hdr = skb->data[0];
> >> >>         skb_pull(skb, 1);
> >> >>
> >> >> --
> >> >> 2.34.1
> >> >>
> >> >
> >> >
> >> >--
> >> >Luiz Augusto von Dentz
> >
> >
> >
> >--
> >Luiz Augusto von Dentz