[PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup()

[PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup()

[Weiming Shi]

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;
+	}
+
 	bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
 
 	hci_set_fw_info(hdev, "%s", skb->data + 1);
-- 
2.43.0

RE: Bluetooth: bpa10x: check response length in bpa10x_setup()

[bluez.test.bot]

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

Re: [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup()

[Luiz Augusto von Dentz]

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

Re: [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup()

[Pauli Virtanen]

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

Re: [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup()

[Pauli Virtanen]

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

Re: [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup()

[Weiming Shi]

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.