Re:Re: Re: [PATCH v3] Bluetooth: HIDP: add missing length check for incoming frames
From: jiale yao
Date: Tue Jul 21 2026 - 01:55:38 EST
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@xxxxxxx>
---
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@xxxxxxxxx> wrote:
>Hi,
>
>On Sun, Jul 19, 2026 at 1:38 AM jiale yao <19888972804@xxxxxxx> 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@xxxxxxx>
>> ---
>> 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@xxxxxxxxx> wrote:
>> >Hi,
>> >
>> >On Fri, Jul 17, 2026 at 11:33 AM Jiale Yao <yaojiale02@xxxxxxx> 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@xxxxxxx>
>> >> ---
>> >> 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