[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

[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