Bluetooth: bpa10x: check response length in bpa10x_setup()

PATCH In review Weiming Shi 4 participants 6 messages Jun 30 – Jul 1 Bluetooth drivers

Share & tools

Weiming Shi

author

2026-06-30 06:28 UTC #0 lore raw

PATCH 1/1 Bluetooth: bpa10x: check response length in bpa10x_setup()

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 <[email protected]>

Assisted-by Claude:claude-opus-4-8

Signed-off-by Weiming Shi <[email protected]>

---
 drivers/bluetooth/bpa10x.c | 6 ++++++
 1 file changed, 6 insertions(+)

drivers/bluetooth/bpa10x.c +6 -0

index 2ae38a321c4b..07c6ed6f18ac 100644
@@ -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);

Signature · 1 line

2.43.0

[email protected]

2026-06-30 09:18 UTC #1 lore raw

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 <[email protected]>

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

Luiz Augusto von Dentz

maintainer

2026-06-30 18:24 UTC #2 lore raw

In reply to Weiming Shi

Hi Weiming,

Quoted 29 lines · 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 <[email protected]>
Assisted-by Claude:claude-opus-4-8
Signed-off-by Weiming Shi <[email protected]>
---
 drivers/bluetooth/bpa10x.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/bluetooth/bpa10x.c +0 -0
index 2ae38a321c4b..07c6ed6f18ac 100644

drivers/bluetooth/bpa10x.c static int bpa10x_setup(struct hci_dev *hdev) · line 263

Quoted 9 lines

drivers/bluetooth/bpa10x.c +6 -0

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

Quoted 6 lines

drivers/bluetooth/bpa10x.c +0 -1

        bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
 
        hci_set_fw_info(hdev, "%s", skb->data + 1);
--

2.43.0

Signature · 1 line

Luiz Augusto von Dentz

Pauli Virtanen

2026-06-30 19:04 UTC #3 lore raw

In reply to Weiming Shi

Hi,

ma, 2026-06-29 kello 23:28 -0700, Weiming Shi kirjoitti:

Quoted 28 lines

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 <[email protected]>
Assisted-by Claude:claude-opus-4-8
Signed-off-by Weiming Shi <[email protected]>
---
 drivers/bluetooth/bpa10x.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/bluetooth/bpa10x.c +0 -0
index 2ae38a321c4b..07c6ed6f18ac 100644

drivers/bluetooth/bpa10x.c static int bpa10x_setup(struct hci_dev *hdev) · line 262

Quoted 8 lines

drivers/bluetooth/bpa10x.c +5 -0

@@ -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.

Quoted 4 lines

drivers/bluetooth/bpa10x.c +1 -0

+
 	bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
 
 	hci_set_fw_info(hdev, "%s", skb->data + 1);

Signature · 1 line

Pauli Virtanen

Weiming Shi

author

2026-07-01 15:46 UTC #4 lore raw

Quoted 48 lines · Pauli Virtanen <[email protected]> 于2026年7月1日周三 03:04写道：

Hi,
ma, 2026-06-29 kello 23:28 -0700, Weiming Shi kirjoitti:
Quoted 28 lines
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 <[email protected]>
Assisted-by Claude:claude-opus-4-8
Signed-off-by Weiming Shi <[email protected]>
---
 drivers/bluetooth/bpa10x.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/bluetooth/bpa10x.c +0 -0
index 2ae38a321c4b..07c6ed6f18ac 100644
drivers/bluetooth/bpa10x.c static int bpa10x_setup(struct hci_dev *hdev) · line 262
Quoted 8 lines
drivers/bluetooth/bpa10x.c +5 -0
@@ -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.

Quoted 7 lines

drivers/bluetooth/bpa10x.c static int bpa10x_setup(struct hci_dev *hdev) · line 266
Quoted 4 lines
drivers/bluetooth/bpa10x.c +1 -0
+
      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.

Pauli Virtanen

2026-06-30 19:10 UTC #5 lore raw

In reply to Weiming Shi

Hi,

ma, 2026-06-29 kello 23:28 -0700, Weiming Shi kirjoitti:

Quoted 28 lines

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 <[email protected]>
Assisted-by Claude:claude-opus-4-8
Signed-off-by Weiming Shi <[email protected]>
---
 drivers/bluetooth/bpa10x.c | 6 ++++++
 1 file changed, 6 insertions(+)
drivers/bluetooth/bpa10x.c +0 -0
index 2ae38a321c4b..07c6ed6f18ac 100644

drivers/bluetooth/bpa10x.c static int bpa10x_setup(struct hci_dev *hdev) · line 262

Quoted 8 lines

drivers/bluetooth/bpa10x.c +5 -0

@@ -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?

Quoted 4 lines

drivers/bluetooth/bpa10x.c +1 -0

+
 	bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
 
 	hci_set_fw_info(hdev, "%s", skb->data + 1);

Signature · 1 line

Pauli Virtanen