RE: Bluetooth: Miscellaneous fixes and cleanups

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

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 v3 0/7] Bluetooth: Miscellaneous fixes and cleanups

[Zijun Hu]

This series has bug fixes, cleanups, a new helper, and a new hook for
transport drivers — all simple.

---
Changes in v3:
- Append 4 more simple commits.
- Link to v2: https://patch.msgid.link/20260727-generic_fix-v2-0-a57bdb81ac67@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 (7):
      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
      Bluetooth: hci_core: Introduce __hci_reset_dev() with a hardware error code
      Bluetooth: btnxpuart: Simplify nxp_set_ind_reset() by __hci_reset_dev()
      Bluetooth: hci_event: Introduce handle_ev_vendor() for HCI_EV_VENDOR
      Bluetooth: hci_event: Use 255 as max event payload length in hci_ev_table[]

 drivers/bluetooth/btintel.c      |  5 ++---
 drivers/bluetooth/btnxpuart.c    | 14 +-------------
 include/net/bluetooth/coredump.h |  7 +++++++
 include/net/bluetooth/hci.h      |  1 +
 include/net/bluetooth/hci_core.h | 10 +++++++++-
 net/bluetooth/coredump.c         |  7 ++-----
 net/bluetooth/hci_core.c         |  6 +++---
 net/bluetooth/hci_event.c        | 22 +++++++++++++++-------
 8 files changed, 40 insertions(+), 32 deletions(-)
---
base-commit: d264c90ca22e2034ef46b1831d38b1f47af44380
change-id: 20260724-generic_fix-6f13710b92ea

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

[PATCH v3 1/7] 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 770b1fb371c7..d06a335ff1db 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -4016,17 +4016,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 v3 2/7] 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 d06a335ff1db..bcb2514b7bc0 100644
--- a/drivers/bluetooth/btintel.c
+++ b/drivers/bluetooth/btintel.c
@@ -3986,18 +3986,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 v3 3/7] 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

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 v3 4/7] Bluetooth: hci_core: Introduce __hci_reset_dev() with a hardware error code

[Zijun Hu]

hci_reset_dev() injects a constant hardware error code 0x00 to restart
the device. But a transport driver may need a different error code.

Fix by introducing __hci_reset_dev(hdev, hw_err_code), which will be
used by a follow-up patch.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 include/net/bluetooth/hci_core.h | 8 +++++++-
 net/bluetooth/hci_core.c         | 6 +++---
 2 files changed, 10 insertions(+), 4 deletions(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index 01b938c4b24a..b7e71733f360 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -1780,17 +1780,23 @@ static inline struct hci_dev *hci_alloc_dev(void)
 void hci_free_dev(struct hci_dev *hdev);
 int hci_register_dev(struct hci_dev *hdev);
 void hci_unregister_dev(struct hci_dev *hdev);
 void hci_release_dev(struct hci_dev *hdev);
 int hci_register_suspend_notifier(struct hci_dev *hdev);
 int hci_unregister_suspend_notifier(struct hci_dev *hdev);
 int hci_suspend_dev(struct hci_dev *hdev);
 int hci_resume_dev(struct hci_dev *hdev);
-int hci_reset_dev(struct hci_dev *hdev);
+int __hci_reset_dev(struct hci_dev *hdev, u8 hw_err_code);
+
+static inline int hci_reset_dev(struct hci_dev *hdev)
+{
+	return __hci_reset_dev(hdev, 0);
+}
+
 int hci_recv_frame(struct hci_dev *hdev, struct sk_buff *skb);
 int hci_recv_diag(struct hci_dev *hdev, struct sk_buff *skb);
 __printf(2, 3) void hci_set_hw_info(struct hci_dev *hdev, const char *fmt, ...);
 __printf(2, 3) void hci_set_fw_info(struct hci_dev *hdev, const char *fmt, ...);
 
 static inline void hci_set_msft_opcode(struct hci_dev *hdev, __u16 opcode)
 {
 #if IS_ENABLED(CONFIG_BT_MSFTEXT)
diff --git a/net/bluetooth/hci_core.c b/net/bluetooth/hci_core.c
index 9d5adf882509..509c820a693d 100644
--- a/net/bluetooth/hci_core.c
+++ b/net/bluetooth/hci_core.c
@@ -2847,34 +2847,34 @@ int hci_resume_dev(struct hci_dev *hdev)
 		      hdev->wake_addr_type);
 
 	hci_sock_dev_event(hdev, HCI_DEV_RESUME);
 	return ret;
 }
 EXPORT_SYMBOL(hci_resume_dev);
 
 /* Reset HCI device */
-int hci_reset_dev(struct hci_dev *hdev)
+int __hci_reset_dev(struct hci_dev *hdev, u8 hw_err_code)
 {
-	static const u8 hw_err[] = { HCI_EV_HARDWARE_ERROR, 0x01, 0x00 };
+	const u8 hw_err[] = { HCI_EV_HARDWARE_ERROR, 0x01, hw_err_code };
 	struct sk_buff *skb;
 
 	skb = bt_skb_alloc(3, GFP_ATOMIC);
 	if (!skb)
 		return -ENOMEM;
 
 	hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
 	skb_put_data(skb, hw_err, 3);
 
 	bt_dev_err(hdev, "Injecting HCI hardware error event");
 
 	/* Send Hardware Error to upper stack */
 	return hci_recv_frame(hdev, skb);
 }
-EXPORT_SYMBOL(hci_reset_dev);
+EXPORT_SYMBOL(__hci_reset_dev);
 
 static u8 hci_dev_classify_pkt_type(struct hci_dev *hdev, struct sk_buff *skb)
 {
 	if (hdev->classify_pkt_type)
 		return hdev->classify_pkt_type(hdev, skb);
 
 	return hci_skb_pkt_type(skb);
 }

-- 
2.34.1

[PATCH v3 5/7] Bluetooth: btnxpuart: Simplify nxp_set_ind_reset() by __hci_reset_dev()

[Zijun Hu]

nxp_set_ind_reset() injects the non-zero hardware error code
BTNXPUART_IR_HW_ERR.

Simplify it by __hci_reset_dev(hdev, BTNXPUART_IR_HW_ERR).

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

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 0bb300eef157..993c48b5f593 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -1326,29 +1326,17 @@ static int nxp_check_boot_sign(struct btnxpuart_dev *nxpdev)
 	return wait_event_interruptible_timeout(nxpdev->check_boot_sign_wait_q,
 					       !test_bit(BTNXPUART_CHECK_BOOT_SIGNATURE,
 							 &nxpdev->tx_state),
 					       msecs_to_jiffies(1000));
 }
 
 static int nxp_set_ind_reset(struct hci_dev *hdev, void *data)
 {
-	static const u8 ir_hw_err[] = { HCI_EV_HARDWARE_ERROR,
-					0x01, BTNXPUART_IR_HW_ERR };
-	struct sk_buff *skb;
-
-	skb = bt_skb_alloc(3, GFP_ATOMIC);
-	if (!skb)
-		return -ENOMEM;
-
-	hci_skb_pkt_type(skb) = HCI_EVENT_PKT;
-	skb_put_data(skb, ir_hw_err, 3);
-
-	/* Inject Hardware Error to upper stack */
-	return hci_recv_frame(hdev, skb);
+	return __hci_reset_dev(hdev, BTNXPUART_IR_HW_ERR);
 }
 
 /* Firmware dump */
 static void nxp_coredump(struct hci_dev *hdev)
 {
 	struct sk_buff *skb;
 	u8 pcmd = 2;
 

-- 
2.34.1

[PATCH v3 6/7] Bluetooth: hci_event: Introduce handle_ev_vendor() for HCI_EV_VENDOR

[Zijun Hu]

Introduce the hook to solve issues below:

msft_vendor_evt(), the current handler for all VSEs, is unsuitable
since:
- many VSEs are not MSFT ones;
- it always corrupts the non-MSFT VSEs by calling skb_pull_data()
  once the MSFT extension is enabled.

Several issues are caused by many transport drivers pre-processing
VSEs in their RX path, often an IRQ-disabled atomic context. Take
the two typical cases below as examples:

Case 1:
  // no btmon log, no way to reach userspace
  Step 1: handle and free @original_skb directly

Case 2:
  // hurts performance and consumes GFP_ATOMIC memory
  Step 1: cloned_skb = skb_clone(original_skb, GFP_ATOMIC);
  // the VSE is handled here
  Step 2: handle and free @cloned_skb
  Step 3: hci_recv_frame(hdev, original_skb);
  // already handled, but re-enters the stack's event-handling path
  Step 4: hci_event_packet(hdev, original_skb);

Fix by introducing the hook with usage:
1) the transport driver registers the hook for VSEs of interest;
2) the stack calls it in process context, handling the VSE like any
   other event:
   - if interested, handle the VSE - no need to free it - and
     return true;
   - otherwise return false.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
Previous version:
https://lore.kernel.org/all/20260722-support_vendor_hci-v2-3-132b506460a4@oss.qualcomm.com

Changes since previous version:
1) Don't handle ACL frames with a vendor-reserved handle for special
   usage.
2) Rename recv_bt_vendor() to handle_ev_vendor(), since the event is
   only in the RX direction.
---
 include/net/bluetooth/hci_core.h |  2 ++
 net/bluetooth/hci_event.c        | 10 +++++++++-
 2 files changed, 11 insertions(+), 1 deletion(-)

diff --git a/include/net/bluetooth/hci_core.h b/include/net/bluetooth/hci_core.h
index b7e71733f360..87db877f8c5c 100644
--- a/include/net/bluetooth/hci_core.h
+++ b/include/net/bluetooth/hci_core.h
@@ -641,16 +641,18 @@ struct hci_dev {
 #endif
 
 	int (*open)(struct hci_dev *hdev);
 	int (*close)(struct hci_dev *hdev);
 	int (*flush)(struct hci_dev *hdev);
 	int (*setup)(struct hci_dev *hdev);
 	int (*shutdown)(struct hci_dev *hdev);
 	int (*send)(struct hci_dev *hdev, struct sk_buff *skb);
+	/* Handle HCI_EV_VENDOR; return true if handled, false otherwise */
+	bool (*handle_ev_vendor)(struct hci_dev *hdev, struct sk_buff *skb);
 	void (*notify)(struct hci_dev *hdev, unsigned int evt);
 	void (*hw_error)(struct hci_dev *hdev, u8 code);
 	int (*post_init)(struct hci_dev *hdev);
 	int (*set_diag)(struct hci_dev *hdev, bool enable);
 	int (*set_bdaddr)(struct hci_dev *hdev, const bdaddr_t *bdaddr);
 	void (*reset)(struct hci_dev *hdev);
 	bool (*wakeup)(struct hci_dev *hdev);
 	int (*set_quality_report)(struct hci_dev *hdev, bool enable);
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 9c8bf6708356..8452d372524f 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -7597,16 +7597,24 @@ static void hci_le_meta_evt(struct hci_dev *hdev, void *data,
 			    ev->subevent, skb->len, subev->max_len);
 	data = hci_le_ev_skb_pull(hdev, skb, ev->subevent, subev->min_len);
 	if (!data)
 		return;
 
 	subev->func(hdev, data, skb);
 }
 
+static void hci_vendor_evt(struct hci_dev *hdev, void *data, struct sk_buff *skb)
+{
+	if (hdev->handle_ev_vendor && hdev->handle_ev_vendor(hdev, skb))
+		return;
+
+	msft_vendor_evt(hdev, data, skb);
+}
+
 static bool hci_get_cmd_complete(struct hci_dev *hdev, u16 opcode,
 				 u8 event, struct sk_buff *skb)
 {
 	struct hci_ev_cmd_complete *ev;
 	struct hci_event_hdr *hdr;
 
 	if (!skb)
 		return false;
@@ -7824,17 +7832,17 @@ static const struct hci_ev {
 	       sizeof(struct hci_ev_keypress_notify)),
 	/* [0x3d = HCI_EV_REMOTE_HOST_FEATURES] */
 	HCI_EV(HCI_EV_REMOTE_HOST_FEATURES, hci_remote_host_features_evt,
 	       sizeof(struct hci_ev_remote_host_features)),
 	/* [0x3e = HCI_EV_LE_META] */
 	HCI_EV_REQ_VL(HCI_EV_LE_META, hci_le_meta_evt,
 		      sizeof(struct hci_ev_le_meta), HCI_MAX_EVENT_SIZE),
 	/* [0xff = HCI_EV_VENDOR] */
-	HCI_EV_VL(HCI_EV_VENDOR, msft_vendor_evt, 0, HCI_MAX_EVENT_SIZE),
+	HCI_EV_VL(HCI_EV_VENDOR, hci_vendor_evt, 0, HCI_MAX_EVENT_SIZE),
 };
 
 static void hci_event_func(struct hci_dev *hdev, u8 event, struct sk_buff *skb,
 			   u16 *opcode, u8 *status,
 			   hci_req_complete_t *req_complete,
 			   hci_req_complete_skb_t *req_complete_skb)
 {
 	const struct hci_ev *ev = &hci_ev_table[event];

-- 
2.34.1

[PATCH v3 7/7] Bluetooth: hci_event: Use 255 as max event payload length in hci_ev_table[]

[Zijun Hu]

hci_event_func() validates skb->len against ev->max_len from the
entry in hci_ev_table[]. By then, the header has already been
stripped by skb_pull(). So the max event payload is 255, but
hci_ev_table[] still uses HCI_MAX_EVENT_SIZE (260) for it, which is
imprecise.

Fix by introducing HCI_MAX_EVENT_PLEN (255) and using it instead.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
Both hci_cc_table[] and hci_le_ev_table[] have the same issue. I'll
send separate patches for them if this one is accepted.
---
 include/net/bluetooth/hci.h |  1 +
 net/bluetooth/hci_event.c   | 14 +++++++-------
 2 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/include/net/bluetooth/hci.h b/include/net/bluetooth/hci.h
index cd3520a29131..1641d879dbda 100644
--- a/include/net/bluetooth/hci.h
+++ b/include/net/bluetooth/hci.h
@@ -3377,16 +3377,17 @@ struct hci_ev_si_security {
 	__u16    proto;
 	__u16    subproto;
 	__u8     incoming;
 } __packed;
 
 /* ---- HCI Packet structures ---- */
 #define HCI_COMMAND_HDR_SIZE 3
 #define HCI_EVENT_HDR_SIZE   2
+#define HCI_MAX_EVENT_PLEN   255
 #define HCI_ACL_HDR_SIZE     4
 #define HCI_SCO_HDR_SIZE     3
 #define HCI_ISO_HDR_SIZE     4
 
 struct hci_command_hdr {
 	__le16	opcode;		/* OCF & OGF */
 	__u8	plen;
 } __packed;
diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 8452d372524f..3879ba302d4d 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -7721,17 +7721,17 @@ static const struct hci_ev {
 	};
 	u16  min_len;
 	u16  max_len;
 } hci_ev_table[U8_MAX + 1] = {
 	/* [0x01 = HCI_EV_INQUIRY_COMPLETE] */
 	HCI_EV_STATUS(HCI_EV_INQUIRY_COMPLETE, hci_inquiry_complete_evt),
 	/* [0x02 = HCI_EV_INQUIRY_RESULT] */
 	HCI_EV_VL(HCI_EV_INQUIRY_RESULT, hci_inquiry_result_evt,
-		  sizeof(struct hci_ev_inquiry_result), HCI_MAX_EVENT_SIZE),
+		  sizeof(struct hci_ev_inquiry_result), HCI_MAX_EVENT_PLEN),
 	/* [0x03 = HCI_EV_CONN_COMPLETE] */
 	HCI_EV(HCI_EV_CONN_COMPLETE, hci_conn_complete_evt,
 	       sizeof(struct hci_ev_conn_complete)),
 	/* [0x04 = HCI_EV_CONN_REQUEST] */
 	HCI_EV(HCI_EV_CONN_REQUEST, hci_conn_request_evt,
 	       sizeof(struct hci_ev_conn_request)),
 	/* [0x05 = HCI_EV_DISCONN_COMPLETE] */
 	HCI_EV(HCI_EV_DISCONN_COMPLETE, hci_disconn_complete_evt,
@@ -7749,29 +7749,29 @@ static const struct hci_ev {
 	HCI_EV(HCI_EV_CHANGE_LINK_KEY_COMPLETE,
 	       hci_change_link_key_complete_evt,
 	       sizeof(struct hci_ev_change_link_key_complete)),
 	/* [0x0b = HCI_EV_REMOTE_FEATURES] */
 	HCI_EV(HCI_EV_REMOTE_FEATURES, hci_remote_features_evt,
 	       sizeof(struct hci_ev_remote_features)),
 	/* [0x0e = HCI_EV_CMD_COMPLETE] */
 	HCI_EV_REQ_VL(HCI_EV_CMD_COMPLETE, hci_cmd_complete_evt,
-		      sizeof(struct hci_ev_cmd_complete), HCI_MAX_EVENT_SIZE),
+		      sizeof(struct hci_ev_cmd_complete), HCI_MAX_EVENT_PLEN),
 	/* [0x0f = HCI_EV_CMD_STATUS] */
 	HCI_EV_REQ(HCI_EV_CMD_STATUS, hci_cmd_status_evt,
 		   sizeof(struct hci_ev_cmd_status)),
 	/* [0x10 = HCI_EV_CMD_STATUS] */
 	HCI_EV(HCI_EV_HARDWARE_ERROR, hci_hardware_error_evt,
 	       sizeof(struct hci_ev_hardware_error)),
 	/* [0x12 = HCI_EV_ROLE_CHANGE] */
 	HCI_EV(HCI_EV_ROLE_CHANGE, hci_role_change_evt,
 	       sizeof(struct hci_ev_role_change)),
 	/* [0x13 = HCI_EV_NUM_COMP_PKTS] */
 	HCI_EV_VL(HCI_EV_NUM_COMP_PKTS, hci_num_comp_pkts_evt,
-		  sizeof(struct hci_ev_num_comp_pkts), HCI_MAX_EVENT_SIZE),
+		  sizeof(struct hci_ev_num_comp_pkts), HCI_MAX_EVENT_PLEN),
 	/* [0x14 = HCI_EV_MODE_CHANGE] */
 	HCI_EV(HCI_EV_MODE_CHANGE, hci_mode_change_evt,
 	       sizeof(struct hci_ev_mode_change)),
 	/* [0x16 = HCI_EV_PIN_CODE_REQ] */
 	HCI_EV(HCI_EV_PIN_CODE_REQ, hci_pin_code_request_evt,
 	       sizeof(struct hci_ev_pin_code_req)),
 	/* [0x17 = HCI_EV_LINK_KEY_REQ] */
 	HCI_EV(HCI_EV_LINK_KEY_REQ, hci_link_key_request_evt,
@@ -7787,27 +7787,27 @@ static const struct hci_ev {
 	       sizeof(struct hci_ev_pkt_type_change)),
 	/* [0x20 = HCI_EV_PSCAN_REP_MODE] */
 	HCI_EV(HCI_EV_PSCAN_REP_MODE, hci_pscan_rep_mode_evt,
 	       sizeof(struct hci_ev_pscan_rep_mode)),
 	/* [0x22 = HCI_EV_INQUIRY_RESULT_WITH_RSSI] */
 	HCI_EV_VL(HCI_EV_INQUIRY_RESULT_WITH_RSSI,
 		  hci_inquiry_result_with_rssi_evt,
 		  sizeof(struct hci_ev_inquiry_result_rssi),
-		  HCI_MAX_EVENT_SIZE),
+		  HCI_MAX_EVENT_PLEN),
 	/* [0x23 = HCI_EV_REMOTE_EXT_FEATURES] */
 	HCI_EV(HCI_EV_REMOTE_EXT_FEATURES, hci_remote_ext_features_evt,
 	       sizeof(struct hci_ev_remote_ext_features)),
 	/* [0x2c = HCI_EV_SYNC_CONN_COMPLETE] */
 	HCI_EV(HCI_EV_SYNC_CONN_COMPLETE, hci_sync_conn_complete_evt,
 	       sizeof(struct hci_ev_sync_conn_complete)),
 	/* [0x2f = HCI_EV_EXTENDED_INQUIRY_RESULT] */
 	HCI_EV_VL(HCI_EV_EXTENDED_INQUIRY_RESULT,
 		  hci_extended_inquiry_result_evt,
-		  sizeof(struct hci_ev_ext_inquiry_result), HCI_MAX_EVENT_SIZE),
+		  sizeof(struct hci_ev_ext_inquiry_result), HCI_MAX_EVENT_PLEN),
 	/* [0x30 = HCI_EV_KEY_REFRESH_COMPLETE] */
 	HCI_EV(HCI_EV_KEY_REFRESH_COMPLETE, hci_key_refresh_complete_evt,
 	       sizeof(struct hci_ev_key_refresh_complete)),
 	/* [0x31 = HCI_EV_IO_CAPA_REQUEST] */
 	HCI_EV(HCI_EV_IO_CAPA_REQUEST, hci_io_capa_request_evt,
 	       sizeof(struct hci_ev_io_capa_request)),
 	/* [0x32 = HCI_EV_IO_CAPA_REPLY] */
 	HCI_EV(HCI_EV_IO_CAPA_REPLY, hci_io_capa_reply_evt,
@@ -7830,19 +7830,19 @@ static const struct hci_ev {
 	/* [0x3c = HCI_EV_KEYPRESS_NOTIFY] */
 	HCI_EV(HCI_EV_KEYPRESS_NOTIFY, hci_keypress_notify_evt,
 	       sizeof(struct hci_ev_keypress_notify)),
 	/* [0x3d = HCI_EV_REMOTE_HOST_FEATURES] */
 	HCI_EV(HCI_EV_REMOTE_HOST_FEATURES, hci_remote_host_features_evt,
 	       sizeof(struct hci_ev_remote_host_features)),
 	/* [0x3e = HCI_EV_LE_META] */
 	HCI_EV_REQ_VL(HCI_EV_LE_META, hci_le_meta_evt,
-		      sizeof(struct hci_ev_le_meta), HCI_MAX_EVENT_SIZE),
+		      sizeof(struct hci_ev_le_meta), HCI_MAX_EVENT_PLEN),
 	/* [0xff = HCI_EV_VENDOR] */
-	HCI_EV_VL(HCI_EV_VENDOR, hci_vendor_evt, 0, HCI_MAX_EVENT_SIZE),
+	HCI_EV_VL(HCI_EV_VENDOR, hci_vendor_evt, 0, HCI_MAX_EVENT_PLEN),
 };
 
 static void hci_event_func(struct hci_dev *hdev, u8 event, struct sk_buff *skb,
 			   u16 *opcode, u8 *status,
 			   hci_req_complete_t *req_complete,
 			   hci_req_complete_skb_t *req_complete_skb)
 {
 	const struct hci_ev *ev = &hci_ev_table[event];

-- 
2.34.1

RE: Bluetooth: Miscellaneous fixes and cleanups

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      5.70 seconds
VerifyFixes                   PASS      0.13 seconds
VerifySignedoff               PASS      0.13 seconds
GitLint                       FAIL      2.23 seconds
SubjectPrefix                 PASS      0.85 seconds
BuildKernel                   PASS      27.00 seconds
CheckAllWarning               PASS      30.10 seconds
CheckSparse                   PASS      29.11 seconds
BuildKernel32                 PASS      26.19 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      496.41 seconds
TestRunner_l2cap-tester       PASS      63.07 seconds
TestRunner_iso-tester         PASS      87.34 seconds
TestRunner_bnep-tester        PASS      19.34 seconds
TestRunner_mgmt-tester        FAIL      220.71 seconds
TestRunner_rfcomm-tester      PASS      26.15 seconds
TestRunner_sco-tester         PASS      36.98 seconds
TestRunner_ioctl-tester       PASS      27.46 seconds
TestRunner_mesh-tester        FAIL      26.94 seconds
TestRunner_smp-tester         PASS      23.73 seconds
TestRunner_userchan-tester    PASS      20.53 seconds
TestRunner_6lowpan-tester     PASS      23.56 seconds
IncrementalBuild              PASS      101.77 seconds

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

1: T1 Title exceeds max length (85>80): "[v3,2/7] Bluetooth: btintel: Remove redundant (hdr->plen > 0) in btintel_recv_event()"
[v3,4/7] Bluetooth: hci_core: Introduce __hci_reset_dev() with a hardware error code

1: T1 Title exceeds max length (84>80): "[v3,4/7] Bluetooth: hci_core: Introduce __hci_reset_dev() with a hardware error code"
[v3,7/7] Bluetooth: hci_event: Use 255 as max event payload length in hci_ev_table[]

1: T1 Title exceeds max length (84>80): "[v3,7/7] Bluetooth: hci_event: Use 255 as max event payload length in hci_ev_table[]"
##############################
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: 501, Passed: 496 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.245 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.491 seconds
Mesh - Send cancel - 2                               Timed out    1.986 seconds


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

---
Regards,
Linux Bluetooth