[PATCH 0/4] Bluetooth: Fix 2 devcoredump bugs and improve event length checks

[PATCH 0/4] Bluetooth: Fix 2 devcoredump bugs and improve event length checks

[Zijun Hu]

This series fixes two devcoredump issues and improves the payload
length limits used to validate HCI events, — all simple.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
Zijun Hu (4):
      Bluetooth: coredump: Fix skb leak in hci_devcd_append() stub
      Bluetooth: coredump: Fix btmon heap-buffer-overflow
      Bluetooth: hci_event: Use 254 as max LE Meta subevent payload length in hci_le_ev_table[]
      Bluetooth: hci_event: Use 252 as max CC event payload length in hci_cc_table[]

 include/net/bluetooth/coredump.h |  1 +
 net/bluetooth/coredump.c         | 17 ++++++++++++-----
 net/bluetooth/hci_event.c        | 28 +++++++++++++++++-----------
 3 files changed, 30 insertions(+), 16 deletions(-)
---
base-commit: e40edfa049b967f0b9379bf549c59431302ad181
change-id: 20260913-misc_fix-6f6f518fe6b5

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

[PATCH 1/4] Bluetooth: coredump: Fix skb leak in hci_devcd_append() stub

[Zijun Hu]

For hci_devcd_append(hdev, skb):
it consumes the skb when CONFIG_DEV_COREDUMP=y, but its stub does not free
the skb. this inconsistency causes skb leak when CONFIG_DEV_COREDUMP=n.

NXP btuart device driver encounters this leak:

btnxpuart.c:
nxp_process_fw_dump(hdev, skb)
{
	...
	// the skb_clone() is leaked.
	err = hci_devcd_append(hdev, skb_clone(skb, GFP_ATOMIC));
	if (err < 0)
		goto free_skb;
	...
free_skb:
	kfree_skb(skb);		/* frees the original, not the clone */
	return 0;
}

Fix by freeing the skb in the stub to keep consistent behavior.

Fixes: 9695ef876fd1 ("Bluetooth: Add support for hci devcoredump")
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 include/net/bluetooth/coredump.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/include/net/bluetooth/coredump.h b/include/net/bluetooth/coredump.h
index acc1849f66c0..07940a1d5dd4 100644
--- a/include/net/bluetooth/coredump.h
+++ b/include/net/bluetooth/coredump.h
@@ -103,16 +103,17 @@ static inline int hci_devcd_register(struct hci_dev *hdev, coredump_t coredump,
 
 static inline int hci_devcd_init(struct hci_dev *hdev, u32 dump_size)
 {
 	return -EOPNOTSUPP;
 }
 
 static inline int hci_devcd_append(struct hci_dev *hdev, struct sk_buff *skb)
 {
+	kfree_skb(skb);
 	return -EOPNOTSUPP;
 }
 
 static inline int hci_devcd_append_pattern(struct hci_dev *hdev,
 					   u8 pattern, u32 len)
 {
 	return -EOPNOTSUPP;
 }

-- 
2.34.1

[PATCH 2/4] Bluetooth: coredump: Fix btmon heap-buffer-overflow

[Zijun Hu]

The collected coredump data can exceed btmon's maximum packet size of
1490 bytes, But hci_devcd_dump() forwards the entire coredump data as an
HCI_DIAG_PKT without checking its size, causing below issue in btmon.

Why ?

For a monitor packet, its payload length @len in header may exceed
BTSNOOP_MAX_PACKET_SIZE, btmon uses it to access payload in @buf.
so cause heap-buffer-overflow.

Kernel:
include/net/bluetooth/hci_mon.h
struct hci_mon_hdr {
	__le16  opcode;
	__le16  index;
	__le16  len;
} __packed;

BlueZ:
src/shared/btsnoop.h
monitor/control.c
struct control_data {
	uint16_t channel;
	int fd;
	unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];
	uint16_t offset;
};

Issue:
ERROR: AddressSanitizer: heap-buffer-overflow on address 0x51b00000065c at pc 0x7f50b987a029 bp 0x7ffde88dc8d0 sp 0x7ffde88dc088
READ of size 17916 at 0x51b00000065c thread T0
    #0 0x7f50b987a028 in write ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1096
    #1 0x5b4f3443ae3a in btsnoop_write ../src/shared/btsnoop.c:289
    #2 0x5b4f3429cbf0 in data_callback ../monitor/control.c:969
    #3 0x5b4f3444fa9d in mainloop_run ../src/shared/mainloop.c:104
    #4 0x5b4f34451da6 in mainloop_run_with_signal ../src/shared/mainloop-notify.c:196
    #5 0x5b4f342953fc in main ../monitor/main.c:303
    #6 0x7f50b8c2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
    #7 0x7f50b8c2a28a in __libc_start_main_impl ../csu/libc-start.c:360
    #8 0x5b4f34295ed4 in _start (/usr/bin/btmon+0x29fed4) (BuildId: b41caafb24db693946c283eaea48112186863d2d)

Fix by forwarding the collected dump data as an HCI_DIAG_PKT only when
its size <= 1490.

Fixes: b257e02ecc46 ("HCI: coredump: Log devcd dumps into the monitor")
Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 net/bluetooth/coredump.c | 17 ++++++++++++-----
 1 file changed, 12 insertions(+), 5 deletions(-)

diff --git a/net/bluetooth/coredump.c b/net/bluetooth/coredump.c
index 71fc8dab4004..61b45ba58a65 100644
--- a/net/bluetooth/coredump.c
+++ b/net/bluetooth/coredump.c
@@ -250,36 +250,43 @@ static void hci_devcd_handle_pkt_pattern(struct hci_dev *hdev,
 	}
 
 	pattern = skb_pull_data(skb, sizeof(*pattern));
 
 	if (!hci_devcd_memset(hdev, pattern->pattern, pattern->len))
 		bt_dev_dbg(hdev, "Failed to set pattern");
 }
 
+/* Align with BlueZ's BTSNOOP_MAX_PACKET_SIZE. */
+#define HCI_DEVCD_DIAG_MAX_SIZE (1486 + 4)
+
 static void hci_devcd_dump(struct hci_dev *hdev)
 {
 	struct sk_buff *skb;
 	u32 size;
 
 	bt_dev_dbg(hdev, "state %s", hci_devcd_state_name(hdev->dump.state));
 
 	size = hdev->dump.tail - hdev->dump.head;
 
-	/* Send a copy to monitor as a diagnostic packet */
-	skb = bt_skb_alloc(size, GFP_ATOMIC);
-	if (skb) {
-		skb_put_data(skb, hdev->dump.head, size);
-		hci_recv_diag(hdev, skb);
+	if (size <= HCI_DEVCD_DIAG_MAX_SIZE) {
+		/* Send a copy to monitor as a diagnostic packet */
+		skb = bt_skb_alloc(size, GFP_ATOMIC);
+		if (skb) {
+			skb_put_data(skb, hdev->dump.head, size);
+			hci_recv_diag(hdev, skb);
+		}
 	}
 
 	/* Emit a devcoredump with the available data */
 	dev_coredumpv(&hdev->dev, hdev->dump.head, size, GFP_KERNEL);
 }
 
+#undef HCI_DEVCD_DIAG_MAX_SIZE
+
 static void hci_devcd_handle_pkt_complete(struct hci_dev *hdev,
 					  struct sk_buff *skb)
 {
 	u32 dump_size;
 
 	if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) {
 		DBG_UNEXPECTED_STATE();
 		return;

-- 
2.34.1

[PATCH 3/4] Bluetooth: hci_event: Use 254 as max LE Meta subevent payload length in hci_le_ev_table[]

[Zijun Hu]

hci_le_meta_evt() validates skb->len against subev->max_len from the entry
in hci_le_ev_table[], By then, the HCI event header and LE Meta subevent
header have already been stripped by skb_pull(), So the max LE Meta
subevent payload is 254, but hci_le_ev_table[] still uses
HCI_MAX_EVENT_SIZE (260) for it, which is imprecise.

Fix by defining HCI_MAX_LE_SUBEVT_PLEN (254) and using it instead.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 net/bluetooth/hci_event.c | 21 ++++++++++++---------
 1 file changed, 12 insertions(+), 9 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 03b805207ab0..1eacdd93c5b5 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -7469,33 +7469,36 @@ static void hci_le_conn_rate_change_evt(struct hci_dev *hdev, void *data,
 }
 
 #define HCI_LE_EV(_op, _func, _len) \
 	HCI_LE_EV_VL(_op, _func, _len, _len)
 
 #define HCI_LE_EV_STATUS(_op, _func) \
 	HCI_LE_EV(_op, _func, sizeof(struct hci_ev_status))
 
+#define HCI_MAX_LE_SUBEVT_PLEN \
+	(HCI_MAX_EVENT_PLEN - sizeof(struct hci_ev_le_meta))
+
 /* Entries in this table shall have their position according to the subevent
  * opcode they handle so the use of the macros above is recommend since it does
  * attempt to initialize at its proper index using Designated Initializers that
  * way events without a callback function can be omitted.
  */
 static const struct hci_le_ev {
 	void (*func)(struct hci_dev *hdev, void *data, struct sk_buff *skb);
 	u16  min_len;
 	u16  max_len;
 } hci_le_ev_table[U8_MAX + 1] = {
 	/* [0x01 = HCI_EV_LE_CONN_COMPLETE] */
 	HCI_LE_EV(HCI_EV_LE_CONN_COMPLETE, hci_le_conn_complete_evt,
 		  sizeof(struct hci_ev_le_conn_complete)),
 	/* [0x02 = HCI_EV_LE_ADVERTISING_REPORT] */
 	HCI_LE_EV_VL(HCI_EV_LE_ADVERTISING_REPORT, hci_le_adv_report_evt,
 		     sizeof(struct hci_ev_le_advertising_report),
-		     HCI_MAX_EVENT_SIZE),
+		     HCI_MAX_LE_SUBEVT_PLEN),
 	/* [0x03 = HCI_EV_LE_CONN_UPDATE_COMPLETE] */
 	HCI_LE_EV(HCI_EV_LE_CONN_UPDATE_COMPLETE,
 		  hci_le_conn_update_complete_evt,
 		  sizeof(struct hci_ev_le_conn_update_complete)),
 	/* [0x04 = HCI_EV_LE_REMOTE_FEAT_COMPLETE] */
 	HCI_LE_EV(HCI_EV_LE_REMOTE_FEAT_COMPLETE,
 		  hci_le_remote_feat_complete_evt,
 		  sizeof(struct hci_ev_le_remote_feat_complete)),
@@ -7508,33 +7511,33 @@ static const struct hci_le_ev {
 		  sizeof(struct hci_ev_le_remote_conn_param_req)),
 	/* [0x0a = HCI_EV_LE_ENHANCED_CONN_COMPLETE] */
 	HCI_LE_EV(HCI_EV_LE_ENHANCED_CONN_COMPLETE,
 		  hci_le_enh_conn_complete_evt,
 		  sizeof(struct hci_ev_le_enh_conn_complete)),
 	/* [0x0b = HCI_EV_LE_DIRECT_ADV_REPORT] */
 	HCI_LE_EV_VL(HCI_EV_LE_DIRECT_ADV_REPORT, hci_le_direct_adv_report_evt,
 		     sizeof(struct hci_ev_le_direct_adv_report),
-		     HCI_MAX_EVENT_SIZE),
+		     HCI_MAX_LE_SUBEVT_PLEN),
 	/* [0x0c = HCI_EV_LE_PHY_UPDATE_COMPLETE] */
 	HCI_LE_EV(HCI_EV_LE_PHY_UPDATE_COMPLETE, hci_le_phy_update_evt,
 		  sizeof(struct hci_ev_le_phy_update_complete)),
 	/* [0x0d = HCI_EV_LE_EXT_ADV_REPORT] */
 	HCI_LE_EV_VL(HCI_EV_LE_EXT_ADV_REPORT, hci_le_ext_adv_report_evt,
 		     sizeof(struct hci_ev_le_ext_adv_report),
-		     HCI_MAX_EVENT_SIZE),
+		     HCI_MAX_LE_SUBEVT_PLEN),
 	/* [0x0e = HCI_EV_LE_PA_SYNC_ESTABLISHED] */
 	HCI_LE_EV(HCI_EV_LE_PA_SYNC_ESTABLISHED,
 		  hci_le_pa_sync_established_evt,
 		  sizeof(struct hci_ev_le_pa_sync_established)),
 	/* [0x0f = HCI_EV_LE_PER_ADV_REPORT] */
 	HCI_LE_EV_VL(HCI_EV_LE_PER_ADV_REPORT,
 				 hci_le_per_adv_report_evt,
 				 sizeof(struct hci_ev_le_per_adv_report),
-				 HCI_MAX_EVENT_SIZE),
+				 HCI_MAX_LE_SUBEVT_PLEN),
 	/* [0x10 = HCI_EV_LE_PA_SYNC_LOST] */
 	HCI_LE_EV(HCI_EV_LE_PA_SYNC_LOST, hci_le_pa_sync_lost_evt,
 		  sizeof(struct hci_ev_le_pa_sync_lost)),
 	/* [0x12 = HCI_EV_LE_EXT_ADV_SET_TERM] */
 	HCI_LE_EV(HCI_EV_LE_EXT_ADV_SET_TERM, hci_le_ext_adv_term_evt,
 		  sizeof(struct hci_evt_le_ext_adv_set_term)),
 	/* [0x18 = HCI_EVT_LE_PAST_RECEIVED] */
 	HCI_LE_EV(HCI_EV_LE_PAST_RECEIVED,
@@ -7545,38 +7548,38 @@ static const struct hci_le_ev {
 		  sizeof(struct hci_evt_le_cis_established)),
 	/* [0x1a = HCI_EVT_LE_CIS_REQ] */
 	HCI_LE_EV(HCI_EVT_LE_CIS_REQ, hci_le_cis_req_evt,
 		  sizeof(struct hci_evt_le_cis_req)),
 	/* [0x1b = HCI_EVT_LE_CREATE_BIG_COMPLETE] */
 	HCI_LE_EV_VL(HCI_EVT_LE_CREATE_BIG_COMPLETE,
 		     hci_le_create_big_complete_evt,
 		     sizeof(struct hci_evt_le_create_big_complete),
-		     HCI_MAX_EVENT_SIZE),
+		     HCI_MAX_LE_SUBEVT_PLEN),
 	/* [0x1d = HCI_EV_LE_BIG_SYNC_ESTABLISHED] */
 	HCI_LE_EV_VL(HCI_EVT_LE_BIG_SYNC_ESTABLISHED,
 		     hci_le_big_sync_established_evt,
 		     sizeof(struct hci_evt_le_big_sync_established),
-		     HCI_MAX_EVENT_SIZE),
+		     HCI_MAX_LE_SUBEVT_PLEN),
 	/* [0x1e = HCI_EVT_LE_BIG_SYNC_LOST] */
 	HCI_LE_EV_VL(HCI_EVT_LE_BIG_SYNC_LOST,
 		     hci_le_big_sync_lost_evt,
 		     sizeof(struct hci_evt_le_big_sync_lost),
-		     HCI_MAX_EVENT_SIZE),
+		     HCI_MAX_LE_SUBEVT_PLEN),
 	/* [0x22 = HCI_EVT_LE_BIG_INFO_ADV_REPORT] */
 	HCI_LE_EV_VL(HCI_EVT_LE_BIG_INFO_ADV_REPORT,
 		     hci_le_big_info_adv_report_evt,
 		     sizeof(struct hci_evt_le_big_info_adv_report),
-		     HCI_MAX_EVENT_SIZE),
+		     HCI_MAX_LE_SUBEVT_PLEN),
 	/* [0x2b = HCI_EVT_LE_ALL_REMOTE_FEATURES_COMPLETE] */
 	HCI_LE_EV_VL(HCI_EVT_LE_ALL_REMOTE_FEATURES_COMPLETE,
 		     hci_le_read_all_remote_features_evt,
 		     sizeof(struct
 			    hci_evt_le_read_all_remote_features_complete),
-		     HCI_MAX_EVENT_SIZE),
+		     HCI_MAX_LE_SUBEVT_PLEN),
 	/* [0x37 = HCI_EVT_LE_CONN_RATE_CHANGE] */
 	HCI_LE_EV(HCI_EVT_LE_CONN_RATE_CHANGE, hci_le_conn_rate_change_evt,
 		  sizeof(struct hci_evt_le_conn_rate_change)),
 };
 
 static void hci_le_meta_evt(struct hci_dev *hdev, void *data,
 			    struct sk_buff *skb, u16 *opcode, u8 *status,
 			    hci_req_complete_t *req_complete,

-- 
2.34.1

[PATCH 4/4] Bluetooth: hci_event: Use 252 as max CC event payload length in hci_cc_table[]

[Zijun Hu]

hci_cc_func() validates skb->len against cc->max_len from the entry in
hci_cc_table[], But By then, the HCI event and CC headers have already been
stripped by skb_pull(). So the max CC payload is 252, but hci_cc_table[]
still uses HCI_MAX_EVENT_SIZE (260) for it, which is imprecise.

Fix by defining HCI_MAX_CC_PLEN (252) and using it instead.

Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
---
 net/bluetooth/hci_event.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/net/bluetooth/hci_event.c b/net/bluetooth/hci_event.c
index 1eacdd93c5b5..25ddba2f603b 100644
--- a/net/bluetooth/hci_event.c
+++ b/net/bluetooth/hci_event.c
@@ -4094,16 +4094,19 @@ static u8 hci_cc_le_set_per_adv_enable(struct hci_dev *hdev, void *data,
 }
 
 #define HCI_CC(_op, _func, _len) \
 	HCI_CC_VL(_op, _func, _len, _len)
 
 #define HCI_CC_STATUS(_op, _func) \
 	HCI_CC(_op, _func, sizeof(struct hci_ev_status))
 
+#define HCI_MAX_CC_PLEN \
+	(HCI_MAX_EVENT_PLEN - sizeof(struct hci_ev_cmd_complete))
+
 static const struct hci_cc {
 	u16  op;
 	u8 (*func)(struct hci_dev *hdev, void *data, struct sk_buff *skb);
 	u16  min_len;
 	u16  max_len;
 } hci_cc_table[] = {
 	HCI_CC_STATUS(HCI_OP_INQUIRY_CANCEL, hci_cc_inquiry_cancel),
 	HCI_CC_STATUS(HCI_OP_PERIODIC_INQ, hci_cc_periodic_inq),
@@ -4187,17 +4190,17 @@ static const struct hci_cc {
 	HCI_CC(HCI_OP_READ_LOCAL_OOB_EXT_DATA, hci_cc_read_local_oob_ext_data,
 	       sizeof(struct hci_rp_read_local_oob_ext_data)),
 	HCI_CC(HCI_OP_LE_READ_BUFFER_SIZE, hci_cc_le_read_buffer_size,
 	       sizeof(struct hci_rp_le_read_buffer_size)),
 	HCI_CC(HCI_OP_LE_READ_LOCAL_FEATURES, hci_cc_le_read_local_features,
 	       sizeof(struct hci_rp_le_read_local_features)),
 	HCI_CC_VL(HCI_OP_LE_READ_CONN_INTERVAL, hci_cc_le_read_conn_interval,
 		  sizeof(struct hci_rp_le_read_conn_interval),
-		  HCI_MAX_EVENT_SIZE),
+		  HCI_MAX_CC_PLEN),
 	HCI_CC(HCI_OP_LE_READ_ADV_TX_POWER, hci_cc_le_read_adv_tx_power,
 	       sizeof(struct hci_rp_le_read_adv_tx_power)),
 	HCI_CC(HCI_OP_USER_CONFIRM_REPLY, hci_cc_user_confirm_reply,
 	       sizeof(struct hci_rp_user_confirm_reply)),
 	HCI_CC(HCI_OP_USER_CONFIRM_NEG_REPLY, hci_cc_user_confirm_neg_reply,
 	       sizeof(struct hci_rp_user_confirm_reply)),
 	HCI_CC(HCI_OP_USER_PASSKEY_REPLY, hci_cc_user_passkey_reply,
 	       sizeof(struct hci_rp_user_confirm_reply)),
@@ -4259,17 +4262,17 @@ static const struct hci_cc {
 	HCI_CC_STATUS(HCI_OP_LE_SET_PER_ADV_ENABLE,
 		      hci_cc_le_set_per_adv_enable),
 	HCI_CC(HCI_OP_LE_READ_TRANSMIT_POWER, hci_cc_le_read_transmit_power,
 	       sizeof(struct hci_rp_le_read_transmit_power)),
 	HCI_CC_STATUS(HCI_OP_LE_SET_PRIVACY_MODE, hci_cc_le_set_privacy_mode),
 	HCI_CC(HCI_OP_LE_READ_BUFFER_SIZE_V2, hci_cc_le_read_buffer_size_v2,
 	       sizeof(struct hci_rp_le_read_buffer_size_v2)),
 	HCI_CC_VL(HCI_OP_LE_SET_CIG_PARAMS, hci_cc_le_set_cig_params,
-		  sizeof(struct hci_rp_le_set_cig_params), HCI_MAX_EVENT_SIZE),
+		  sizeof(struct hci_rp_le_set_cig_params), HCI_MAX_CC_PLEN),
 	HCI_CC(HCI_OP_LE_SETUP_ISO_PATH, hci_cc_le_setup_iso_path,
 	       sizeof(struct hci_rp_le_setup_iso_path)),
 	HCI_CC(HCI_OP_LE_READ_ALL_LOCAL_FEATURES,
 	       hci_cc_le_read_all_local_features,
 	       sizeof(struct hci_rp_le_read_all_local_features)),
 };
 
 static u8 hci_cc_func(struct hci_dev *hdev, const struct hci_cc *cc,

-- 
2.34.1

RE: Bluetooth: Fix 2 devcoredump bugs and improve event length checks

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 5827 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/series/1164165/

---Test result---

Test Summary:
CheckPatch                    FAIL      2.37 seconds
VerifyFixes                   PASS      0.08 seconds
VerifySignedoff               PASS      7.79 seconds
GitLint                       FAIL      1.00 seconds
SubjectPrefix                 PASS      0.29 seconds
BuildKernel                   PASS      30.04 seconds
CheckAllWarning               PASS      34.56 seconds
CheckSparse                   WARNING   37.80 seconds
BuildKernel32                 PASS      29.91 seconds
CheckKernelLLVM               PASS      42.93 seconds
TestRunnerSetup               PASS      799.32 seconds
TestRunner_l2cap-tester       PASS      43.32 seconds
TestRunner_iso-tester         PASS      95.81 seconds
TestRunner_bnep-tester        PASS      13.60 seconds
TestRunner_mgmt-tester        FAIL      143.96 seconds
TestRunner_rfcomm-tester      PASS      16.92 seconds
TestRunner_sco-tester         PASS      22.48 seconds
TestRunner_ioctl-tester       PASS      18.47 seconds
TestRunner_mesh-tester        FAIL      19.15 seconds
TestRunner_smp-tester         PASS      15.66 seconds
TestRunner_userchan-tester    PASS      17.16 seconds
TestRunner_6lowpan-tester     PASS      16.51 seconds
IncrementalBuild              PASS      35.05 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
[2/4] Bluetooth: coredump: Fix btmon heap-buffer-overflow
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#191: 
ERROR: AddressSanitizer: heap-buffer-overflow on address 0x51b00000065c at pc 0x7f50b987a029 bp 0x7ffde88dc8d0 sp 0x7ffde88dc088

total: 0 errors, 1 warnings, 0 checks, 48 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/14812259.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:
[1/4] Bluetooth: coredump: Fix skb leak in hci_devcd_append() stub

12: B3 Line contains hard tab characters (\t): "	..."
13: B3 Line contains hard tab characters (\t): "	// the skb_clone() is leaked."
14: B3 Line contains hard tab characters (\t): "	err = hci_devcd_append(hdev, skb_clone(skb, GFP_ATOMIC));"
15: B3 Line contains hard tab characters (\t): "	if (err < 0)"
16: B3 Line contains hard tab characters (\t): "		goto free_skb;"
17: B3 Line contains hard tab characters (\t): "	..."
19: B3 Line contains hard tab characters (\t): "	kfree_skb(skb);		/* frees the original, not the clone */"
20: B3 Line contains hard tab characters (\t): "	return 0;"
[2/4] Bluetooth: coredump: Fix btmon heap-buffer-overflow

16: B3 Line contains hard tab characters (\t): "	__le16  opcode;"
17: B3 Line contains hard tab characters (\t): "	__le16  index;"
18: B3 Line contains hard tab characters (\t): "	__le16  len;"
25: B3 Line contains hard tab characters (\t): "	uint16_t channel;"
26: B3 Line contains hard tab characters (\t): "	int fd;"
27: B3 Line contains hard tab characters (\t): "	unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];"
28: B3 Line contains hard tab characters (\t): "	uint16_t offset;"
32: B1 Line exceeds max length (128>80): "ERROR: AddressSanitizer: heap-buffer-overflow on address 0x51b00000065c at pc 0x7f50b987a029 bp 0x7ffde88dc8d0 sp 0x7ffde88dc088"
34: B1 Line exceeds max length (115>80): "    #0 0x7f50b987a028 in write ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1096"
38: B1 Line exceeds max length (85>80): "    #4 0x5b4f34451da6 in mainloop_run_with_signal ../src/shared/mainloop-notify.c:196"
40: B1 Line exceeds max length (89>80): "    #6 0x7f50b8c2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58"
42: B1 Line exceeds max length (109>80): "    #8 0x5b4f34295ed4 in _start (/usr/bin/btmon+0x29fed4) (BuildId: b41caafb24db693946c283eaea48112186863d2d)"
[3/4] Bluetooth: hci_event: Use 254 as max LE Meta subevent payload length in hci_le_ev_table[]

1: T1 Title exceeds max length (95>80): "[3/4] Bluetooth: hci_event: Use 254 as max LE Meta subevent payload length in hci_le_ev_table[]"
[4/4] Bluetooth: hci_event: Use 252 as max CC event payload length in hci_cc_table[]

1: T1 Title exceeds max length (84>80): "[4/4] Bluetooth: hci_event: Use 252 as max CC event payload length in hci_cc_table[]"
##############################
Test: CheckSparse - WARNING
Desc: Run sparse tool with linux kernel
Output:
net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):net/bluetooth/hci_event.c: note: in included file (through include/net/bluetooth/hci_core.h):
##############################
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.140 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.568 seconds
Mesh - Send cancel - 2                               Timed out    1.997 seconds


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

---
Regards,
Linux Bluetooth

Re: [PATCH 1/4] Bluetooth: coredump: Fix skb leak in hci_devcd_append() stub

[Luiz Augusto von Dentz]

Hi Zijun,

On Sun, Sep 13, 2026 at 11:43 PM Zijun Hu <zijun.hu@oss.qualcomm.com> wrote:
>
> For hci_devcd_append(hdev, skb):
> it consumes the skb when CONFIG_DEV_COREDUMP=y, but its stub does not free
> the skb. this inconsistency causes skb leak when CONFIG_DEV_COREDUMP=n.
>
> NXP btuart device driver encounters this leak:
>
> btnxpuart.c:
> nxp_process_fw_dump(hdev, skb)
> {
>         ...
>         // the skb_clone() is leaked.
>         err = hci_devcd_append(hdev, skb_clone(skb, GFP_ATOMIC));
>         if (err < 0)
>                 goto free_skb;
>         ...
> free_skb:
>         kfree_skb(skb);         /* frees the original, not the clone */
>         return 0;
> }
>
> Fix by freeing the skb in the stub to keep consistent behavior.
>
> Fixes: 9695ef876fd1 ("Bluetooth: Add support for hci devcoredump")
> Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
> ---
>  include/net/bluetooth/coredump.h | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/include/net/bluetooth/coredump.h b/include/net/bluetooth/coredump.h
> index acc1849f66c0..07940a1d5dd4 100644
> --- a/include/net/bluetooth/coredump.h
> +++ b/include/net/bluetooth/coredump.h
> @@ -103,16 +103,17 @@ static inline int hci_devcd_register(struct hci_dev *hdev, coredump_t coredump,
>
>  static inline int hci_devcd_init(struct hci_dev *hdev, u32 dump_size)
>  {
>         return -EOPNOTSUPP;
>  }
>
>  static inline int hci_devcd_append(struct hci_dev *hdev, struct sk_buff *skb)
>  {
> +       kfree_skb(skb);

I don't think freeing it here is a good idea; in fact I think the bug
must be fixed in the caller so it properly frees all clones, etc,
actually it may need to check if hci_devcd_init fails and stops
cloning and calling hci_devcd_append as a result.

>         return -EOPNOTSUPP;
>  }
>
>  static inline int hci_devcd_append_pattern(struct hci_dev *hdev,
>                                            u8 pattern, u32 len)
>  {
>         return -EOPNOTSUPP;
>  }
>
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz

Re: [PATCH 2/4] Bluetooth: coredump: Fix btmon heap-buffer-overflow

[Luiz Augusto von Dentz]

Hi Zijun,

On Sun, Sep 13, 2026 at 11:43 PM Zijun Hu <zijun.hu@oss.qualcomm.com> wrote:
>
> The collected coredump data can exceed btmon's maximum packet size of
> 1490 bytes, But hci_devcd_dump() forwards the entire coredump data as an
> HCI_DIAG_PKT without checking its size, causing below issue in btmon.
>
> Why ?
>
> For a monitor packet, its payload length @len in header may exceed
> BTSNOOP_MAX_PACKET_SIZE, btmon uses it to access payload in @buf.
> so cause heap-buffer-overflow.
>
> Kernel:
> include/net/bluetooth/hci_mon.h
> struct hci_mon_hdr {
>         __le16  opcode;
>         __le16  index;
>         __le16  len;
> } __packed;
>
> BlueZ:
> src/shared/btsnoop.h
> monitor/control.c
> struct control_data {
>         uint16_t channel;
>         int fd;
>         unsigned char buf[BTSNOOP_MAX_PACKET_SIZE];
>         uint16_t offset;
> };
>
> Issue:
> ERROR: AddressSanitizer: heap-buffer-overflow on address 0x51b00000065c at pc 0x7f50b987a029 bp 0x7ffde88dc8d0 sp 0x7ffde88dc088
> READ of size 17916 at 0x51b00000065c thread T0
>     #0 0x7f50b987a028 in write ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1096
>     #1 0x5b4f3443ae3a in btsnoop_write ../src/shared/btsnoop.c:289
>     #2 0x5b4f3429cbf0 in data_callback ../monitor/control.c:969
>     #3 0x5b4f3444fa9d in mainloop_run ../src/shared/mainloop.c:104
>     #4 0x5b4f34451da6 in mainloop_run_with_signal ../src/shared/mainloop-notify.c:196
>     #5 0x5b4f342953fc in main ../monitor/main.c:303
>     #6 0x7f50b8c2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>     #7 0x7f50b8c2a28a in __libc_start_main_impl ../csu/libc-start.c:360
>     #8 0x5b4f34295ed4 in _start (/usr/bin/btmon+0x29fed4) (BuildId: b41caafb24db693946c283eaea48112186863d2d)

This probably needs to be fixed in btmon if it attempts to read past
the buffer size. Also, since it now supports vendor packets that may
not be limited by HCI packet size, perhaps we should allocate based on
the channel MTU which probably needs per-vendor configuration to allow
collecting its vendor packets.

> Fix by forwarding the collected dump data as an HCI_DIAG_PKT only when
> its size <= 1490.
>
> Fixes: b257e02ecc46 ("HCI: coredump: Log devcd dumps into the monitor")
> Signed-off-by: Zijun Hu <zijun.hu@oss.qualcomm.com>
> ---
>  net/bluetooth/coredump.c | 17 ++++++++++++-----
>  1 file changed, 12 insertions(+), 5 deletions(-)
>
> diff --git a/net/bluetooth/coredump.c b/net/bluetooth/coredump.c
> index 71fc8dab4004..61b45ba58a65 100644
> --- a/net/bluetooth/coredump.c
> +++ b/net/bluetooth/coredump.c
> @@ -250,36 +250,43 @@ static void hci_devcd_handle_pkt_pattern(struct hci_dev *hdev,
>         }
>
>         pattern = skb_pull_data(skb, sizeof(*pattern));
>
>         if (!hci_devcd_memset(hdev, pattern->pattern, pattern->len))
>                 bt_dev_dbg(hdev, "Failed to set pattern");
>  }
>
> +/* Align with BlueZ's BTSNOOP_MAX_PACKET_SIZE. */
> +#define HCI_DEVCD_DIAG_MAX_SIZE (1486 + 4)
> +
>  static void hci_devcd_dump(struct hci_dev *hdev)
>  {
>         struct sk_buff *skb;
>         u32 size;
>
>         bt_dev_dbg(hdev, "state %s", hci_devcd_state_name(hdev->dump.state));
>
>         size = hdev->dump.tail - hdev->dump.head;
>
> -       /* Send a copy to monitor as a diagnostic packet */
> -       skb = bt_skb_alloc(size, GFP_ATOMIC);
> -       if (skb) {
> -               skb_put_data(skb, hdev->dump.head, size);
> -               hci_recv_diag(hdev, skb);
> +       if (size <= HCI_DEVCD_DIAG_MAX_SIZE) {
> +               /* Send a copy to monitor as a diagnostic packet */
> +               skb = bt_skb_alloc(size, GFP_ATOMIC);
> +               if (skb) {
> +                       skb_put_data(skb, hdev->dump.head, size);
> +                       hci_recv_diag(hdev, skb);
> +               }
>         }
>
>         /* Emit a devcoredump with the available data */
>         dev_coredumpv(&hdev->dev, hdev->dump.head, size, GFP_KERNEL);
>  }
>
> +#undef HCI_DEVCD_DIAG_MAX_SIZE
> +
>  static void hci_devcd_handle_pkt_complete(struct hci_dev *hdev,
>                                           struct sk_buff *skb)
>  {
>         u32 dump_size;
>
>         if (hdev->dump.state != HCI_DEVCOREDUMP_ACTIVE) {
>                 DBG_UNEXPECTED_STATE();
>                 return;
>
> --
> 2.34.1
>


-- 
Luiz Augusto von Dentz

Re: [PATCH 2/4] Bluetooth: coredump: Fix btmon heap-buffer-overflow

[Zijun Hu]

On 9/14/2026 10:05 PM, Luiz Augusto von Dentz wrote:
>> Issue:
>> ERROR: AddressSanitizer: heap-buffer-overflow on address 0x51b00000065c at pc 0x7f50b987a029 bp 0x7ffde88dc8d0 sp 0x7ffde88dc088
>> READ of size 17916 at 0x51b00000065c thread T0
>>     #0 0x7f50b987a028 in write ../../../../src/libsanitizer/sanitizer_common/sanitizer_common_interceptors.inc:1096
>>     #1 0x5b4f3443ae3a in btsnoop_write ../src/shared/btsnoop.c:289
>>     #2 0x5b4f3429cbf0 in data_callback ../monitor/control.c:969
>>     #3 0x5b4f3444fa9d in mainloop_run ../src/shared/mainloop.c:104
>>     #4 0x5b4f34451da6 in mainloop_run_with_signal ../src/shared/mainloop-notify.c:196
>>     #5 0x5b4f342953fc in main ../monitor/main.c:303
>>     #6 0x7f50b8c2a1c9 in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58
>>     #7 0x7f50b8c2a28a in __libc_start_main_impl ../csu/libc-start.c:360
>>     #8 0x5b4f34295ed4 in _start (/usr/bin/btmon+0x29fed4) (BuildId: b41caafb24db693946c283eaea48112186863d2d)
> This probably needs to be fixed in btmon if it attempts to read past
> the buffer size. Also, since it now supports vendor packets that may

Agree, will fix on the btmon side.

> not be limited by HCI packet size, perhaps we should allocate based on
> the channel MTU which probably needs per-vendor configuration to allow
> collecting its vendor packets.

This issue has nothing to do with HCI_VENDOR_PKT (0xff).
This problem has likely existed for a long time across almost every vendor's devcoredump since controller's RAM size almost >= 1490
The root cause is that devcoredump data accumulated from MANY MANY packets is being sent as a SINGLE DIAG packet to monitor channel.

Re: [PATCH 1/4] Bluetooth: coredump: Fix skb leak in hci_devcd_append() stub

[Zijun Hu]

On 9/14/2026 9:57 PM, Luiz Augusto von Dentz wrote:
>>  static inline int hci_devcd_append(struct hci_dev *hdev, struct sk_buff *skb)
>>  {
>> +       kfree_skb(skb);
> I don't think freeing it here is a good idea; in fact I think the bug

Agree, but freeing it here is the simplest fix.

> must be fixed in the caller so it properly frees all clones, etc,
> actually it may need to check if hci_devcd_init fails and stops
> cloning and calling hci_devcd_append as a result.

It would require a persistent variable to track whether a devcoredump collection failure has ever occurred,
for every devcoredump packet reported by the controller.