[PATCH v2 0/3] Bluetooth: Miscellaneous fixes and cleanups

[PATCH v2 0/3] Bluetooth: Miscellaneous fixes and cleanups

[Zijun Hu]

This series has one bug fix and two cleanups, all very simple.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
Changes in v2:
- Rework patch 1: guard with (@len + 1) against skb->len, per Luiz.
- Add two more btintel/coredump cleanups.
- Drop "hci_core: Don't treat HCI_DRV_PKT/HCI_DIAG_PKT as unknown type".
- Link to v1: https://patch.msgid.link/20260725-generic_fix-v1-0-305aec261a19@oss.qualcomm.com

---
Zijun Hu (3):
      Bluetooth: btintel: Fix diagnostics event detection
      Bluetooth: btintel: Remove redundant (hdr->plen > 0) in btintel_recv_event()
      Bluetooth: coredump: Expose header size and end marker to drivers

 drivers/bluetooth/btintel.c      | 5 ++---
 include/net/bluetooth/coredump.h | 7 +++++++
 net/bluetooth/coredump.c         | 7 ++-----
 3 files changed, 11 insertions(+), 8 deletions(-)
---
base-commit: a7ef68421906a8b1c4204dda9b2e52b90f6fd2ba
change-id: 20260724-generic_fix-6f13710b92ea

Best regards,
--  
Zijun Hu <zijun.hu@oss.qualcomm.com>

[PATCH v2 1/3] Bluetooth: btintel: Fix diagnostics event detection

[Zijun Hu]

For a diagnostics VSE, diagnostics_hdr[] sits at the start of the event
payload, skb->data[2], but btintel_recv_event() wrongly guards its
memcmp with @len, which is measured from skb->data[3] for the earlier
INTEL_BOOTLOADER check.

Fix by using (@len + 1) instead, which ==
(skb->len - HCI_EVENT_HDR_SIZE) exactly.

Fixes: af395330abed ("Bluetooth: btintel: Add Intel devcoredump support")
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 drivers/bluetooth/btintel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 680f96c188d4..b800379f91cf 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -3827,17 +3827,17 @@ int btintel_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
 				kfree_skb(skb);
 				return 0;
 			}
 		}
 
 		/* Handle all diagnostics events separately. May still call
 		 * hci_recv_frame.
 		 */
-		if (len >= sizeof(diagnostics_hdr) &&
+		if (len + 1 >= sizeof(diagnostics_hdr) &&
 		    memcmp(&skb->data[2], diagnostics_hdr,
 			   sizeof(diagnostics_hdr)) == 0) {
 			return btintel_diagnostics(hdev, skb);
 		}
 	}
 
 	return hci_recv_frame(hdev, skb);
 }

-- 
2.34.1

RE: Bluetooth: Miscellaneous fixes and cleanups

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2682 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=1135668

---Test result---

Test Summary:
CheckPatch                    PASS      2.08 seconds
VerifyFixes                   PASS      0.11 seconds
VerifySignedoff               PASS      0.10 seconds
GitLint                       FAIL      0.82 seconds
SubjectPrefix                 PASS      0.29 seconds
BuildKernel                   PASS      25.79 seconds
CheckAllWarning               PASS      28.60 seconds
CheckSparse                   PASS      27.02 seconds
BuildKernel32                 PASS      25.56 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      468.71 seconds
TestRunner_l2cap-tester       PASS      63.45 seconds
TestRunner_iso-tester         PASS      88.12 seconds
TestRunner_bnep-tester        PASS      19.64 seconds
TestRunner_mgmt-tester        FAIL      222.34 seconds
TestRunner_rfcomm-tester      PASS      25.87 seconds
TestRunner_sco-tester         PASS      31.54 seconds
TestRunner_ioctl-tester       PASS      26.60 seconds
TestRunner_mesh-tester        FAIL      25.96 seconds
TestRunner_smp-tester         PASS      23.33 seconds
TestRunner_userchan-tester    PASS      20.18 seconds
TestRunner_6lowpan-tester     PASS      23.27 seconds
IncrementalBuild              PASS      43.25 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[v2,2/3] Bluetooth: btintel: Remove redundant (hdr->plen > 0) in btintel_recv_event()

1: T1 Title exceeds max length (85>80): "[v2,2/3] Bluetooth: btintel: Remove redundant (hdr->plen > 0) in btintel_recv_event()"
##############################
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.260 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    2.362 seconds
Mesh - Send cancel - 2                               Timed out    1.988 seconds


https://github.com/bluez/bluetooth-next/pull/504

---
Regards,
Linux Bluetooth

[PATCH v2 2/3] Bluetooth: btintel: Remove redundant (hdr->plen > 0) in btintel_recv_event()

[Zijun Hu]

Drop the check since:
- it is already implied by the existing (skb->len > HCI_EVENT_HDR_SIZE)
- hdr->plen is then not used by the function at all

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 drivers/bluetooth/btintel.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index b800379f91cf..6e61f1bb1871 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -3797,18 +3797,17 @@ static int btintel_diagnostics(struct hci_dev *hdev, struct sk_buff *skb)
 	return hci_recv_frame(hdev, skb);
 }
 
 int btintel_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	struct hci_event_hdr *hdr = (void *)skb->data;
 	const char diagnostics_hdr[] = { 0x87, 0x80, 0x03 };
 
-	if (skb->len > HCI_EVENT_HDR_SIZE && hdr->evt == 0xff &&
-	    hdr->plen > 0) {
+	if (skb->len > HCI_EVENT_HDR_SIZE && hdr->evt == 0xff) {
 		const void *ptr = skb->data + HCI_EVENT_HDR_SIZE + 1;
 		unsigned int len = skb->len - HCI_EVENT_HDR_SIZE - 1;
 
 		if (btintel_test_flag(hdev, INTEL_BOOTLOADER)) {
 			switch (skb->data[2]) {
 			case 0x02:
 				/* When switching to the operational firmware
 				 * the device sends a vendor specific event

-- 
2.34.1

[PATCH v2 3/3] Bluetooth: coredump: Expose header size and end marker to drivers

[Zijun Hu]

To separate the coredump header and data far more easily, give a
vendor driver the option to pad its header to a fixed size, by
moving the header size limit and ending marker to coredump.h:

 - HCI_DEVCD_HDR_SIZE_MAX: the max header size
 - HCI_DEVCD_HDR_END_MARKER: the header-ending marker

No functional change intended.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
Previous version:
https://lore.kernel.org/all/20260713-btusb_prep_qcc2072-v2-2-bbcb651285f2@oss.qualcomm.com

Changes since previous version:
- Rename MAX_HCI_DEVCD_HDR_SIZE to HCI_DEVCD_HDR_SIZE_MAX
- Rename HCI_DEVCD_HDR_END to HCI_DEVCD_HDR_END_MARKER
- Improve the commit title and message
---
 include/net/bluetooth/coredump.h | 7 +++++++
 net/bluetooth/coredump.c         | 7 ++-----
 2 files changed, 9 insertions(+), 5 deletions(-)

diff --git a/include/net/bluetooth/coredump.h b/include/net/bluetooth/coredump.h
index ab85a6adfffd..1f071ab55416 100644
--- a/include/net/bluetooth/coredump.h
+++ b/include/net/bluetooth/coredump.h
@@ -3,16 +3,23 @@
  * Copyright (C) 2022 Google Corporation
  */
 
 #ifndef __COREDUMP_H
 #define __COREDUMP_H
 
 #define DEVCOREDUMP_TIMEOUT	msecs_to_jiffies(10000)	/* 10 sec */
 
+/*
+ * Max header size, shared by both the devcoredump core and
+ * the dmp_hdr() registered by driver via hci_devcd_register()
+ */
+#define HCI_DEVCD_HDR_SIZE_MAX	512
+#define HCI_DEVCD_HDR_END_MARKER	"--- Start dump ---\n"
+
 typedef void (*coredump_t)(struct hci_dev *hdev);
 typedef void (*dmp_hdr_t)(struct hci_dev *hdev, struct sk_buff *skb);
 typedef void (*notify_change_t)(struct hci_dev *hdev, int state);
 
 /* struct hci_devcoredump - Devcoredump state
  *
  * @supported: Indicates if FW dump collection is supported by driver
  * @state: Current state of dump collection
diff --git a/net/bluetooth/coredump.c b/net/bluetooth/coredump.c
index 913bbba559f8..5bee863bd6d2 100644
--- a/net/bluetooth/coredump.c
+++ b/net/bluetooth/coredump.c
@@ -29,18 +29,16 @@ struct hci_devcoredump_skb_pattern {
 #define hci_dmp_cb(skb)	((struct hci_devcoredump_skb_cb *)((skb)->cb))
 
 #define DBG_UNEXPECTED_STATE() \
 	bt_dev_dbg(hdev, \
 		   "Unexpected packet (%d) for state %s.", \
 		   hci_dmp_cb(skb)->pkt_type, \
 		   hci_devcd_state_name(hdev->dump.state))
 
-#define MAX_DEVCOREDUMP_HDR_SIZE	512	/* bytes */
-
 static int hci_devcd_update_hdr_state(char *buf, size_t size, int state)
 {
 	int len = 0;
 
 	if (!buf)
 		return 0;
 
 	len = scnprintf(buf, size, "Bluetooth devcoredump\nState: %d\n", state);
@@ -58,28 +56,27 @@ static int hci_devcd_update_state(struct hci_dev *hdev, int state)
 	hdev->dump.state = state;
 
 	return hci_devcd_update_hdr_state(hdev->dump.head,
 					  hdev->dump.alloc_size, state);
 }
 
 static int hci_devcd_mkheader(struct hci_dev *hdev, struct sk_buff *skb)
 {
-	char dump_start[] = "--- Start dump ---\n";
 	char hdr[80];
 	int hdr_len;
 
 	hdr_len = hci_devcd_update_hdr_state(hdr, sizeof(hdr),
 					     HCI_DEVCOREDUMP_IDLE);
 	skb_put_data(skb, hdr, hdr_len);
 
 	if (hdev->dump.dmp_hdr)
 		hdev->dump.dmp_hdr(hdev, skb);
 
-	skb_put_data(skb, dump_start, strlen(dump_start));
+	skb_put_data(skb, HCI_DEVCD_HDR_END_MARKER, strlen(HCI_DEVCD_HDR_END_MARKER));
 
 	return skb->len;
 }
 
 /* Do not call with hci_dev_lock since this calls driver code. */
 static void hci_devcd_notify(struct hci_dev *hdev, int state)
 {
 	if (hdev->dump.notify_change)
@@ -149,17 +146,17 @@ static bool hci_devcd_memset(struct hci_dev *hdev, u8 pattern, u32 len)
 
 /* Call with hci_dev_lock only. */
 static int hci_devcd_prepare(struct hci_dev *hdev, u32 dump_size)
 {
 	struct sk_buff *skb;
 	int dump_hdr_size;
 	int err = 0;
 
-	skb = alloc_skb(MAX_DEVCOREDUMP_HDR_SIZE, GFP_ATOMIC);
+	skb = alloc_skb(HCI_DEVCD_HDR_SIZE_MAX, GFP_ATOMIC);
 	if (!skb)
 		return -ENOMEM;
 
 	dump_hdr_size = hci_devcd_mkheader(hdev, skb);
 
 	if (hci_devcd_alloc(hdev, dump_hdr_size + dump_size)) {
 		err = -ENOMEM;
 		goto hdr_free;

-- 
2.34.1

[PATCH 01/11] Bluetooth: btintel: Fix diagnostics event detection

[Zijun Hu]

For a diagnostics VSE, diagnostics_hdr[] sits at the start of the event
payload, skb->data[2], but btintel_recv_event() wrongly guards its
memcmp with @len, which is measured from skb->data[3] for the earlier
INTEL_BOOTLOADER check.

Fix by using @hdr->plen, which is the event payload length, instead.

Fixes: af395330abed ("Bluetooth: btintel: Add Intel devcoredump support")
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 drivers/bluetooth/btintel.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btintel.c b/drivers/bluetooth/btintel.c
index 5e9cac090bd8..d7baf1b9d852 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -3826,17 +3826,17 @@ int btintel_recv_event(struct hci_dev *hdev, struct sk_buff *skb)
 				kfree_skb(skb);
 				return 0;
 			}
 		}
 
 		/* Handle all diagnostics events separately. May still call
 		 * hci_recv_frame.
 		 */
-		if (len >= sizeof(diagnostics_hdr) &&
+		if (hdr->plen >= sizeof(diagnostics_hdr) &&
 		    memcmp(&skb->data[2], diagnostics_hdr,
 			   sizeof(diagnostics_hdr)) == 0) {
 			return btintel_diagnostics(hdev, skb);
 		}
 	}
 
 	return hci_recv_frame(hdev, skb);
 }

-- 
2.34.1

RE: Bluetooth: Miscellaneous fixes and cleanups

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 3846 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=1134336

---Test result---

Test Summary:
CheckPatch                    PASS      6.36 seconds
VerifyFixes                   PASS      0.08 seconds
VerifySignedoff               PASS      0.08 seconds
GitLint                       FAIL      2.36 seconds
SubjectPrefix                 PASS      0.79 seconds
BuildKernel                   PASS      27.42 seconds
CheckAllWarning               PASS      29.82 seconds
CheckSparse                   PASS      28.70 seconds
BuildKernel32                 PASS      30.25 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      500.70 seconds
TestRunner_l2cap-tester       PASS      60.58 seconds
TestRunner_iso-tester         FAIL      92.90 seconds
TestRunner_bnep-tester        PASS      19.37 seconds
TestRunner_mgmt-tester        FAIL      227.45 seconds
TestRunner_rfcomm-tester      PASS      26.72 seconds
TestRunner_sco-tester         PASS      33.09 seconds
TestRunner_ioctl-tester       PASS      27.38 seconds
TestRunner_mesh-tester        FAIL      26.98 seconds
TestRunner_smp-tester         PASS      24.37 seconds
TestRunner_userchan-tester    PASS      20.77 seconds
TestRunner_6lowpan-tester     PASS      24.06 seconds
IncrementalBuild              PASS      80.01 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[09/11] Bluetooth: btintel: Simplify btintel_classify_pkt_type() by hci_acl_handle()

1: T1 Title exceeds max length (84>80): "[09/11] Bluetooth: btintel: Simplify btintel_classify_pkt_type() by hci_acl_handle()"
[10/11] Bluetooth: btmrvl_sdio: Do not free HCI_VENDOR_PKT frame by hci_recv_frame()

1: T1 Title exceeds max length (84>80): "[10/11] Bluetooth: btmrvl_sdio: Do not free HCI_VENDOR_PKT frame by hci_recv_frame()"
[11/11] Bluetooth: hci_core: Don't treat HCI_DRV_PKT/HCI_DIAG_PKT as unknown type

1: T1 Title exceeds max length (81>80): "[11/11] Bluetooth: hci_core: Don't treat HCI_DRV_PKT/HCI_DIAG_PKT as unknown type"
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
Total: 141, Passed: 134 (95.0%), Failed: 7, Not Run: 0

Failed Test Cases
ISO Disconnect - Success                             Timed out    2.152 seconds
ISO Reconnect - Success                              Timed out    1.996 seconds
ISO Reconnect Send and Receive #16 - Success         Timed out    1.993 seconds
ISO Reconnect AC 6(i) - Success                      Timed out    2.544 seconds
ISO Reconnect AC 6(ii) - Success                     Timed out    2.006 seconds
ISO Broadcaster Reconnect - Success                  Timed out    2.714 seconds
ISO Broadcaster Receiver Defer Reconnect - Success   Timed out    2.347 seconds
##############################
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.265 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    2.781 seconds
Mesh - Send cancel - 2                               Timed out    1.984 seconds


https://github.com/bluez/bluetooth-next/pull/492

---
Regards,
Linux Bluetooth