* RE: Bluetooth: bpa10x: check response length in bpa10x_setup()
2026-06-30 6:28 [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup() Weiming Shi
@ 2026-06-30 9:18 ` bluez.test.bot
2026-06-30 18:24 ` [PATCH] " Luiz Augusto von Dentz
` (2 subsequent siblings)
3 siblings, 0 replies; 6+ messages in thread
From: bluez.test.bot @ 2026-06-30 9:18 UTC (permalink / raw)
To: linux-bluetooth, bestswngs
[-- Attachment #1: Type: text/plain, Size: 2280 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=1118670
---Test result---
Test Summary:
CheckPatch FAIL 1.51 seconds
VerifyFixes PASS 0.23 seconds
VerifySignedoff PASS 0.24 seconds
GitLint FAIL 0.69 seconds
SubjectPrefix PASS 0.31 seconds
BuildKernel PASS 26.76 seconds
CheckAllWarning PASS 29.40 seconds
CheckSparse PASS 28.49 seconds
BuildKernel32 PASS 25.75 seconds
CheckKernelLLVM SKIP 0.00 seconds
TestRunnerSetup PASS 490.41 seconds
IncrementalBuild PASS 25.57 seconds
Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: bpa10x: check response length in bpa10x_setup()
WARNING: Reported-by: should be immediately followed by Closes: with a URL to the report
#117:
Reported-by: Xiang Mei <xmei5@asu.edu>
Assisted-by: Claude:claude-opus-4-8
total: 0 errors, 1 warnings, 12 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
/github/workspace/src/patch/14652779.patch has style problems, please review.
NOTE: Ignored message types: UNKNOWN_COMMIT_ID
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: bpa10x: check response length in bpa10x_setup()
7: B3 Line contains hard tab characters (\t): " bt_dev_info(hdev, "%s", (char *)(skb->data + 1));"
8: B3 Line contains hard tab characters (\t): " hci_set_fw_info(hdev, "%s", skb->data + 1);"
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found
https://github.com/bluez/bluetooth-next/pull/375
---
Regards,
Linux Bluetooth
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup()
2026-06-30 6:28 [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup() Weiming Shi
2026-06-30 9:18 ` bluez.test.bot
@ 2026-06-30 18:24 ` Luiz Augusto von Dentz
2026-06-30 19:04 ` Pauli Virtanen
2026-06-30 19:04 ` Pauli Virtanen
3 siblings, 0 replies; 6+ messages in thread
From: Luiz Augusto von Dentz @ 2026-06-30 18:24 UTC (permalink / raw)
To: Weiming Shi
Cc: Marcel Holtmann, Johan Hedberg, linux-bluetooth, linux-kernel,
Xiang Mei
Hi Weiming,
On Tue, Jun 30, 2026 at 2:28 AM Weiming Shi <bestswngs@gmail.com> wrote:
>
> bpa10x_setup() sends the vendor command 0xfc0e and passes the response
> to bt_dev_info() and hci_set_fw_info() as a "%s" string starting at
> skb->data + 1, without checking the length:
>
> bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
> hci_set_fw_info(hdev, "%s", skb->data + 1);
>
> A device that returns a one-byte response (status only) leaves
> skb->data + 1 past the end of the data, and the %s walk reads adjacent
> slab memory until it meets a NUL. The same happens when the payload is
> not NUL-terminated within skb->len. The out-of-bounds bytes end up in
> the kernel log and the firmware-info debugfs file.
>
> Bail out if the response is too short or not NUL-terminated.
>
> Fixes: ddd68ec8f484 ("Bluetooth: bpa10x: Read revision information in setup stage")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
> drivers/bluetooth/bpa10x.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
> index 2ae38a321c4b..07c6ed6f18ac 100644
> --- a/drivers/bluetooth/bpa10x.c
> +++ b/drivers/bluetooth/bpa10x.c
> @@ -255,6 +255,12 @@ static int bpa10x_setup(struct hci_dev *hdev)
> if (IS_ERR(skb))
> return PTR_ERR(skb);
>
> + /* Reject a short or non-terminated response before the %s reads. */
> + if (skb->len < 2 || skb->data[skb->len - 1] != '\0') {
> + kfree_skb(skb);
> + return -EILSEQ;
> + }
> +
Looks valid to me. We shouldn't really depend on the string being
NULL-terminated, as it could be truncated for some reason. Need to
check if %.*s is safe though and if that would work with the likes of
hci_set_fw_info:
https://sashiko.dev/#/patchset/20260630062759.2769059-3-bestswngs%40gmail.com
> bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
>
> hci_set_fw_info(hdev, "%s", skb->data + 1);
> --
> 2.43.0
>
--
Luiz Augusto von Dentz
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup()
2026-06-30 6:28 [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup() Weiming Shi
2026-06-30 9:18 ` bluez.test.bot
2026-06-30 18:24 ` [PATCH] " Luiz Augusto von Dentz
@ 2026-06-30 19:04 ` Pauli Virtanen
2026-06-30 19:04 ` Pauli Virtanen
3 siblings, 0 replies; 6+ messages in thread
From: Pauli Virtanen @ 2026-06-30 19:04 UTC (permalink / raw)
To: Weiming Shi, Marcel Holtmann, Luiz Augusto von Dentz
Cc: Johan Hedberg, linux-bluetooth, linux-kernel, Xiang Mei
Hi,
ma, 2026-06-29 kello 23:28 -0700, Weiming Shi kirjoitti:
> bpa10x_setup() sends the vendor command 0xfc0e and passes the response
> to bt_dev_info() and hci_set_fw_info() as a "%s" string starting at
> skb->data + 1, without checking the length:
>
> bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
> hci_set_fw_info(hdev, "%s", skb->data + 1);
>
> A device that returns a one-byte response (status only) leaves
> skb->data + 1 past the end of the data, and the %s walk reads adjacent
> slab memory until it meets a NUL. The same happens when the payload is
> not NUL-terminated within skb->len. The out-of-bounds bytes end up in
> the kernel log and the firmware-info debugfs file.
>
> Bail out if the response is too short or not NUL-terminated.
>
> Fixes: ddd68ec8f484 ("Bluetooth: bpa10x: Read revision information in setup stage")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
> drivers/bluetooth/bpa10x.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
> index 2ae38a321c4b..07c6ed6f18ac 100644
> --- a/drivers/bluetooth/bpa10x.c
> +++ b/drivers/bluetooth/bpa10x.c
> @@ -255,6 +255,12 @@ static int bpa10x_setup(struct hci_dev *hdev)
> if (IS_ERR(skb))
> return PTR_ERR(skb);
>
> + /* Reject a short or non-terminated response before the %s reads. */
> + if (skb->len < 2 || skb->data[skb->len - 1] != '\0') {
> + kfree_skb(skb);
> + return -EILSEQ;
> + }
Was this tested on real hardware?
With an out-of-bounds access fix like the above in btmtk some weeks
ago, it turned out HW actually was sending truncated data, and it was
found only later in production use that the driver was broken by the
fix.
> +
> bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
>
> hci_set_fw_info(hdev, "%s", skb->data + 1);
--
Pauli Virtanen
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup()
2026-06-30 6:28 [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup() Weiming Shi
` (2 preceding siblings ...)
2026-06-30 19:04 ` Pauli Virtanen
@ 2026-06-30 19:04 ` Pauli Virtanen
2026-07-01 15:45 ` Weiming Shi
3 siblings, 1 reply; 6+ messages in thread
From: Pauli Virtanen @ 2026-06-30 19:04 UTC (permalink / raw)
To: Weiming Shi, Marcel Holtmann, Luiz Augusto von Dentz
Cc: Johan Hedberg, linux-bluetooth, linux-kernel, Xiang Mei
Hi,
ma, 2026-06-29 kello 23:28 -0700, Weiming Shi kirjoitti:
> bpa10x_setup() sends the vendor command 0xfc0e and passes the response
> to bt_dev_info() and hci_set_fw_info() as a "%s" string starting at
> skb->data + 1, without checking the length:
>
> bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
> hci_set_fw_info(hdev, "%s", skb->data + 1);
>
> A device that returns a one-byte response (status only) leaves
> skb->data + 1 past the end of the data, and the %s walk reads adjacent
> slab memory until it meets a NUL. The same happens when the payload is
> not NUL-terminated within skb->len. The out-of-bounds bytes end up in
> the kernel log and the firmware-info debugfs file.
>
> Bail out if the response is too short or not NUL-terminated.
>
> Fixes: ddd68ec8f484 ("Bluetooth: bpa10x: Read revision information in setup stage")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> ---
> drivers/bluetooth/bpa10x.c | 6 ++++++
> 1 file changed, 6 insertions(+)
>
> diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
> index 2ae38a321c4b..07c6ed6f18ac 100644
> --- a/drivers/bluetooth/bpa10x.c
> +++ b/drivers/bluetooth/bpa10x.c
> @@ -255,6 +255,12 @@ static int bpa10x_setup(struct hci_dev *hdev)
> if (IS_ERR(skb))
> return PTR_ERR(skb);
>
> + /* Reject a short or non-terminated response before the %s reads. */
> + if (skb->len < 2 || skb->data[skb->len - 1] != '\0') {
> + kfree_skb(skb);
> + return -EILSEQ;
> + }
Was this tested on real hardware?
With an out-of-bounds access fix like the above in btmtk some weeks
ago, it turned out HW actually was sending truncated data, and it was
found only later in production use that the driver was broken by the
fix.
> +
> bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
>
> hci_set_fw_info(hdev, "%s", skb->data + 1);
--
Pauli Virtanen
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup()
2026-06-30 19:04 ` Pauli Virtanen
@ 2026-07-01 15:45 ` Weiming Shi
0 siblings, 0 replies; 6+ messages in thread
From: Weiming Shi @ 2026-07-01 15:45 UTC (permalink / raw)
To: Pauli Virtanen
Cc: Marcel Holtmann, Luiz Augusto von Dentz, Johan Hedberg,
linux-bluetooth, linux-kernel, Xiang Mei
Pauli Virtanen <pav@iki.fi> 于2026年7月1日周三 03:04写道：
>
> Hi,
>
> ma, 2026-06-29 kello 23:28 -0700, Weiming Shi kirjoitti:
> > bpa10x_setup() sends the vendor command 0xfc0e and passes the response
> > to bt_dev_info() and hci_set_fw_info() as a "%s" string starting at
> > skb->data + 1, without checking the length:
> >
> > bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
> > hci_set_fw_info(hdev, "%s", skb->data + 1);
> >
> > A device that returns a one-byte response (status only) leaves
> > skb->data + 1 past the end of the data, and the %s walk reads adjacent
> > slab memory until it meets a NUL. The same happens when the payload is
> > not NUL-terminated within skb->len. The out-of-bounds bytes end up in
> > the kernel log and the firmware-info debugfs file.
> >
> > Bail out if the response is too short or not NUL-terminated.
> >
> > Fixes: ddd68ec8f484 ("Bluetooth: bpa10x: Read revision information in setup stage")
> > Reported-by: Xiang Mei <xmei5@asu.edu>
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Weiming Shi <bestswngs@gmail.com>
> > ---
> > drivers/bluetooth/bpa10x.c | 6 ++++++
> > 1 file changed, 6 insertions(+)
> >
> > diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
> > index 2ae38a321c4b..07c6ed6f18ac 100644
> > --- a/drivers/bluetooth/bpa10x.c
> > +++ b/drivers/bluetooth/bpa10x.c
> > @@ -255,6 +255,12 @@ static int bpa10x_setup(struct hci_dev *hdev)
> > if (IS_ERR(skb))
> > return PTR_ERR(skb);
> >
> > + /* Reject a short or non-terminated response before the %s reads. */
> > + if (skb->len < 2 || skb->data[skb->len - 1] != '\0') {
> > + kfree_skb(skb);
> > + return -EILSEQ;
> > + }
>
> Was this tested on real hardware?
>
> With an out-of-bounds access fix like the above in btmtk some weeks
> ago, it turned out HW actually was sending truncated data, and it was
> found only later in production use that the driver was broken by the
> fix.
>
> > +
> > bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
> >
> > hci_set_fw_info(hdev, "%s", skb->data + 1);
>
> --
> Pauli Virtanen
Right, -EILSEQ is too aggressive. It would break a device that sends a
short or unterminated response. v2 uses "%.*s" bounded to skb->len - 1
and drops the error return.
No real hardware here. Tested with dummy_hcd + raw-gadget .
Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread