* [PATCH] Bluetooth: HIDP: add missing length check for incoming frames
@ 2026-07-17 15:32 Jiale Yao
2026-07-17 15:40 ` Luiz Augusto von Dentz
2026-07-17 16:23 ` bluez.test.bot
0 siblings, 2 replies; 8+ messages in thread
From: Jiale Yao @ 2026-07-17 15:32 UTC (permalink / raw)
To: Marcel Holtmann, Luiz Augusto von Dentz, Jiale Yao, Tim Bird,
Muhammad Bilal, Kees Cook, Michael Bommarito, linux-bluetooth,
linux-kernel
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
^ permalink raw reply related [flat|nested] 8+ messages in thread* Re: [PATCH] Bluetooth: HIDP: add missing length check for incoming frames 2026-07-17 15:32 [PATCH] Bluetooth: HIDP: add missing length check for incoming frames Jiale Yao @ 2026-07-17 15:40 ` Luiz Augusto von Dentz 2026-07-19 5:37 ` Re:Re: [PATCH v2] " jiale yao 2026-07-17 16:23 ` bluez.test.bot 1 sibling, 1 reply; 8+ messages in thread From: Luiz Augusto von Dentz @ 2026-07-17 15:40 UTC (permalink / raw) To: Jiale Yao Cc: Marcel Holtmann, Tim Bird, Muhammad Bilal, Kees Cook, Michael Bommarito, linux-bluetooth, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re:Re: [PATCH v2] Bluetooth: HIDP: add missing length check for incoming frames 2026-07-17 15:40 ` Luiz Augusto von Dentz @ 2026-07-19 5:37 ` jiale yao 2026-07-20 19:47 ` Luiz Augusto von Dentz 2026-08-14 4:27 ` Re:Re: [PATCH v2] " kernel test robot 0 siblings, 2 replies; 8+ messages in thread From: jiale yao @ 2026-07-19 5:37 UTC (permalink / raw) To: Luiz Augusto von Dentz Cc: Marcel Holtmann, Tim Bird, Muhammad Bilal, Kees Cook, Michael Bommarito, linux-bluetooth, linux-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Re: [PATCH v2] Bluetooth: HIDP: add missing length check for incoming frames 2026-07-19 5:37 ` Re:Re: [PATCH v2] " jiale yao @ 2026-07-20 19:47 ` Luiz Augusto von Dentz 2026-07-21 5:54 ` Re:Re: Re: [PATCH v3] " jiale yao 2026-08-14 4:27 ` Re:Re: [PATCH v2] " kernel test robot 1 sibling, 1 reply; 8+ messages in thread From: Luiz Augusto von Dentz @ 2026-07-20 19:47 UTC (permalink / raw) To: jiale yao Cc: Marcel Holtmann, Tim Bird, Muhammad Bilal, Kees Cook, Michael Bommarito, linux-bluetooth, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re:Re: Re: [PATCH v3] Bluetooth: HIDP: add missing length check for incoming frames 2026-07-20 19:47 ` Luiz Augusto von Dentz @ 2026-07-21 5:54 ` jiale yao 2026-07-22 0:06 ` Muhammad Bilal 0 siblings, 1 reply; 8+ messages in thread From: jiale yao @ 2026-07-21 5:54 UTC (permalink / raw) To: Luiz Augusto von Dentz Cc: Marcel Holtmann, Tim Bird, Muhammad Bilal, Kees Cook, Michael Bommarito, linux-bluetooth, linux-kernel 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 ^ permalink raw reply related [flat|nested] 8+ messages in thread
* Re: Re: Re: [PATCH v3] Bluetooth: HIDP: add missing length check for incoming frames 2026-07-21 5:54 ` Re:Re: Re: [PATCH v3] " jiale yao @ 2026-07-22 0:06 ` Muhammad Bilal 0 siblings, 0 replies; 8+ messages in thread From: Muhammad Bilal @ 2026-07-22 0:06 UTC (permalink / raw) To: jiale yao Cc: Luiz Augusto von Dentz, Marcel Holtmann, Tim Bird, Kees Cook, Michael Bommarito, linux-bluetooth, linux-kernel 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 ^ permalink raw reply [flat|nested] 8+ messages in thread
* Re: Re:Re: [PATCH v2] Bluetooth: HIDP: add missing length check for incoming frames 2026-07-19 5:37 ` Re:Re: [PATCH v2] " jiale yao 2026-07-20 19:47 ` Luiz Augusto von Dentz @ 2026-08-14 4:27 ` kernel test robot 1 sibling, 0 replies; 8+ messages in thread From: kernel test robot @ 2026-08-14 4:27 UTC (permalink / raw) To: jiale yao, Luiz Augusto von Dentz Cc: oe-kbuild-all, Marcel Holtmann, Tim Bird, Muhammad Bilal, Kees Cook, Michael Bommarito, linux-bluetooth, linux-kernel Hi jiale, kernel test robot noticed the following build warnings: [auto build test WARNING on kees/for-next/pstore] [also build test WARNING on kees/for-next/kspp] [cannot apply to bluetooth-next/master bluetooth/master linus/master v7.2-rc7 next-20260812] [If your patch is applied to the wrong git tree, kindly drop us a note. And when submitting patch, we suggest to use '--base' as documented in https://git-scm.com/docs/git-format-patch#_base_tree_information] url: https://github.com/intel-lab-lkp/linux/commits/jiale-yao/Re-Re-PATCH-v2-Bluetooth-HIDP-add-missing-length-check-for-incoming-frames/20260814-100503 base: https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git for-next/pstore patch link: https://lore.kernel.org/r/220b6be5.d3f.19f78e165aa.Coremail.19888972804%40163.com patch subject: Re:Re: [PATCH v2] Bluetooth: HIDP: add missing length check for incoming frames config: powerpc-allmodconfig (https://download.01.org/0day-ci/archive/20260814/202608141113.WMJnrfnI-lkp@intel.com/config) compiler: powerpc64-linux-gcc (GCC) 16.1.0 reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20260814/202608141113.WMJnrfnI-lkp@intel.com/reproduce) If you fix the issue in a separate patch/commit (i.e. not just a new version of the same patch/commit), kindly add following tags | Reported-by: kernel test robot <lkp@intel.com> | Closes: https://lore.kernel.org/oe-kbuild-all/202608141113.WMJnrfnI-lkp@intel.com/ All warnings (new ones prefixed by >>): In file included from include/asm-generic/bug.h:31, from arch/powerpc/include/asm/bug.h:116, from include/linux/bug.h:5, from include/linux/thread_info.h:13, from include/asm-generic/preempt.h:5, from arch/powerpc/include/asm/preempt.h:5, from include/linux/preempt.h:79, from include/linux/spinlock.h:56, from include/linux/kref.h:16, from net/bluetooth/hidp/core.c:24: net/bluetooth/hidp/core.c: In function 'hidp_recv_intr_frame': >> net/bluetooth/hidp/core.c:612:24: warning: format '%x' expects argument of type 'unsigned int', but argument 3 has type 'unsigned char *' [-Wformat=] 612 | BT_DBG("Unsupported protocol header 0x%02x", hdr); | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ include/linux/printk.h:401:21: note: in definition of macro 'pr_fmt' 401 | #define pr_fmt(fmt) fmt | ^~~ include/linux/dynamic_debug.h:259:9: note: in expansion of macro '__dynamic_func_call_cls' 259 | __dynamic_func_call_cls(__UNIQUE_ID(ddebug), cls, fmt, func, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~~~~~~~~ include/linux/dynamic_debug.h:261:9: note: in expansion of macro '_dynamic_func_call_cls' 261 | _dynamic_func_call_cls(_DPRINTK_CLASS_DFLT, fmt, func, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~~~~~~~ include/linux/dynamic_debug.h:280:9: note: in expansion of macro '_dynamic_func_call' 280 | _dynamic_func_call(fmt, __dynamic_pr_debug, \ | ^~~~~~~~~~~~~~~~~~ include/linux/printk.h:635:9: note: in expansion of macro 'dynamic_pr_debug' 635 | dynamic_pr_debug(fmt, ##__VA_ARGS__) | ^~~~~~~~~~~~~~~~ include/net/bluetooth/bluetooth.h:287:33: note: in expansion of macro 'pr_debug' 287 | #define BT_DBG(fmt, ...) pr_debug(fmt "\n", ##__VA_ARGS__) | ^~~~~~~~ net/bluetooth/hidp/core.c:612:17: note: in expansion of macro 'BT_DBG' 612 | BT_DBG("Unsupported protocol header 0x%02x", hdr); | ^~~~~~ net/bluetooth/hidp/core.c:612:58: note: format string is defined here 612 | BT_DBG("Unsupported protocol header 0x%02x", hdr); | ~~~^ | | | unsigned int | %02hhn vim +612 net/bluetooth/hidp/core.c ^1da177e4c3f415 Linus Torvalds 2005-04-16 588 91f5cca3d1b4341 Andrew Morton 2008-02-05 589 static void hidp_recv_intr_frame(struct hidp_session *session, 91f5cca3d1b4341 Andrew Morton 2008-02-05 590 struct sk_buff *skb) ^1da177e4c3f415 Linus Torvalds 2005-04-16 591 { afd2c613d3369fb jiale yao 2026-07-19 592 unsigned char *hdr; ^1da177e4c3f415 Linus Torvalds 2005-04-16 593 093dabb4f1aff98 Kai Ye 2021-06-03 594 BT_DBG("session %p skb %p len %u", session, skb, skb->len); ^1da177e4c3f415 Linus Torvalds 2005-04-16 595 afd2c613d3369fb jiale yao 2026-07-19 596 hdr = skb_pull_data(skb, 1); afd2c613d3369fb jiale yao 2026-07-19 597 if (!hdr) afd2c613d3369fb jiale yao 2026-07-19 598 return; ^1da177e4c3f415 Linus Torvalds 2005-04-16 599 afd2c613d3369fb jiale yao 2026-07-19 600 if (*hdr == (HIDP_TRANS_DATA | HIDP_DATA_RTYPE_INPUT)) { ^1da177e4c3f415 Linus Torvalds 2005-04-16 601 hidp_set_timer(session); e1aaadd4d8162a2 Marcel Holtmann 2007-02-17 602 ^1da177e4c3f415 Linus Torvalds 2005-04-16 603 if (session->input) ^1da177e4c3f415 Linus Torvalds 2005-04-16 604 hidp_input_report(session, skb); e1aaadd4d8162a2 Marcel Holtmann 2007-02-17 605 e1aaadd4d8162a2 Marcel Holtmann 2007-02-17 606 if (session->hid) { a4b1b5877b514b2 David Rheinsberg 2013-12-19 607 hidp_process_report(session, HID_INPUT_REPORT, a4b1b5877b514b2 David Rheinsberg 2013-12-19 608 skb->data, skb->len, 1); e1aaadd4d8162a2 Marcel Holtmann 2007-02-17 609 BT_DBG("report len %d", skb->len); e1aaadd4d8162a2 Marcel Holtmann 2007-02-17 610 } ^1da177e4c3f415 Linus Torvalds 2005-04-16 611 } else { ^1da177e4c3f415 Linus Torvalds 2005-04-16 @612 BT_DBG("Unsupported protocol header 0x%02x", hdr); ^1da177e4c3f415 Linus Torvalds 2005-04-16 613 } ^1da177e4c3f415 Linus Torvalds 2005-04-16 614 ^1da177e4c3f415 Linus Torvalds 2005-04-16 615 kfree_skb(skb); ^1da177e4c3f415 Linus Torvalds 2005-04-16 616 } ^1da177e4c3f415 Linus Torvalds 2005-04-16 617 -- 0-DAY CI Kernel Test Service https://github.com/intel/lkp-tests/wiki ^ permalink raw reply [flat|nested] 8+ messages in thread
* RE: Bluetooth: HIDP: add missing length check for incoming frames 2026-07-17 15:32 [PATCH] Bluetooth: HIDP: add missing length check for incoming frames Jiale Yao 2026-07-17 15:40 ` Luiz Augusto von Dentz @ 2026-07-17 16:23 ` bluez.test.bot 1 sibling, 0 replies; 8+ messages in thread From: bluez.test.bot @ 2026-07-17 16:23 UTC (permalink / raw) To: linux-bluetooth, yaojiale02 [-- Attachment #1: Type: text/plain, Size: 2389 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=1129697 ---Test result--- Test Summary: CheckPatch PASS 0.58 seconds VerifyFixes PASS 0.10 seconds VerifySignedoff PASS 0.11 seconds GitLint PASS 0.27 seconds SubjectPrefix PASS 0.16 seconds BuildKernel PASS 22.57 seconds CheckAllWarning PASS 25.42 seconds CheckSparse PASS 25.04 seconds BuildKernel32 PASS 22.34 seconds CheckKernelLLVM SKIP 0.00 seconds TestRunnerSetup PASS 408.54 seconds TestRunner_l2cap-tester PASS 56.29 seconds TestRunner_iso-tester PASS 78.41 seconds TestRunner_bnep-tester PASS 17.10 seconds TestRunner_mgmt-tester FAIL 204.30 seconds TestRunner_rfcomm-tester PASS 23.90 seconds TestRunner_sco-tester PASS 30.72 seconds TestRunner_ioctl-tester PASS 23.78 seconds TestRunner_mesh-tester FAIL 23.76 seconds TestRunner_smp-tester PASS 21.47 seconds TestRunner_userchan-tester PASS 18.22 seconds TestRunner_6lowpan-tester PASS 20.95 seconds IncrementalBuild PASS 21.80 seconds Details ############################## Test: CheckKernelLLVM - SKIP Desc: Build kernel with LLVM + context analysis Output: Clang not found ############################## Test: TestRunner_mgmt-tester - FAIL Desc: Run mgmt-tester with test-runner Output: Total: 494, Passed: 489 (99.0%), Failed: 1, Not Run: 4 Failed Test Cases Read Exp Feature - Success Failed 0.232 seconds ############################## Test: TestRunner_mesh-tester - FAIL Desc: Run mesh-tester with test-runner Output: Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0 Failed Test Cases Mesh - Send cancel - 1 Timed out 1.977 seconds Mesh - Send cancel - 2 Timed out 1.988 seconds https://github.com/bluez/bluetooth-next/pull/450 --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2026-08-14 4:27 UTC | newest] Thread overview: 8+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-17 15:32 [PATCH] Bluetooth: HIDP: add missing length check for incoming frames Jiale Yao 2026-07-17 15:40 ` Luiz Augusto von Dentz 2026-07-19 5:37 ` Re:Re: [PATCH v2] " jiale yao 2026-07-20 19:47 ` Luiz Augusto von Dentz 2026-07-21 5:54 ` Re:Re: Re: [PATCH v3] " jiale yao 2026-07-22 0:06 ` Muhammad Bilal 2026-08-14 4:27 ` Re:Re: [PATCH v2] " kernel test robot 2026-07-17 16:23 ` bluez.test.bot
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.