[PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups

[PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups

[Johan Hovold]

Here are two fixes for potential info leaks in the QCA driver and a few
cleanups.

All of these can go into 6.10.

Johan


Johan Hovold (5):
  Bluetooth: qca: fix info leak when fetching fw build id
  Bluetooth: qca: fix info leak when fetching board id
  Bluetooth: qca: drop bogus edl header checks
  Bluetooth: qca: drop bogus module version
  Bluetooth: qca: clean up defines

 drivers/bluetooth/btqca.c | 51 ++++++++++++++++----------------
 drivers/bluetooth/btqca.h | 61 +++++++++++++++++++--------------------
 2 files changed, 55 insertions(+), 57 deletions(-)

-- 
2.43.2

[PATCH 1/5] Bluetooth: qca: fix info leak when fetching fw build id

[Johan Hovold]

Add the missing sanity checks and move the 255-byte build-id buffer off
the stack to avoid leaking stack data through debugfs in case the
build-info reply is malformed.

Fixes: c0187b0bd3e9 ("Bluetooth: btqca: Add support to read FW build version for WCN3991 BTSoC")
Cc: stable@vger.kernel.org	# 5.12
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/bluetooth/btqca.c | 25 +++++++++++++++++++++----
 drivers/bluetooth/btqca.h |  1 -
 2 files changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 38a770278103..a508d79d9aaa 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -99,7 +99,8 @@ static int qca_read_fw_build_info(struct hci_dev *hdev)
 {
 	struct sk_buff *skb;
 	struct edl_event_hdr *edl;
-	char cmd, build_label[QCA_FW_BUILD_VER_LEN];
+	char *build_label;
+	char cmd;
 	int build_lbl_len, err = 0;
 
 	bt_dev_dbg(hdev, "QCA read fw build info");
@@ -114,6 +115,11 @@ static int qca_read_fw_build_info(struct hci_dev *hdev)
 		return err;
 	}
 
+	if (skb->len < sizeof(*edl)) {
+		err = -EILSEQ;
+		goto out;
+	}
+
 	edl = (struct edl_event_hdr *)(skb->data);
 	if (!edl) {
 		bt_dev_err(hdev, "QCA read fw build info with no header");
@@ -129,14 +135,25 @@ static int qca_read_fw_build_info(struct hci_dev *hdev)
 		goto out;
 	}
 
+	if (skb->len < sizeof(*edl) + 1) {
+		err = -EILSEQ;
+		goto out;
+	}
+
 	build_lbl_len = edl->data[0];
-	if (build_lbl_len <= QCA_FW_BUILD_VER_LEN - 1) {
-		memcpy(build_label, edl->data + 1, build_lbl_len);
-		*(build_label + build_lbl_len) = '\0';
+
+	if (skb->len < sizeof(*edl) + 1 + build_lbl_len) {
+		err = -EILSEQ;
+		goto out;
 	}
 
+	build_label = kstrndup(&edl->data[1], build_lbl_len, GFP_KERNEL);
+	if (!build_label)
+		goto out;
+
 	hci_set_fw_info(hdev, "%s", build_label);
 
+	kfree(build_label);
 out:
 	kfree_skb(skb);
 	return err;
diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index 49ad668d0d0b..215433fd76a1 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -48,7 +48,6 @@
 #define get_soc_ver(soc_id, rom_ver)	\
 	((le32_to_cpu(soc_id) << 16) | (le16_to_cpu(rom_ver)))
 
-#define QCA_FW_BUILD_VER_LEN		255
 #define QCA_HSP_GF_SOC_ID			0x1200
 #define QCA_HSP_GF_SOC_MASK			0x0000ff00
 
-- 
2.43.2

RE: Bluetooth: qca: info leak fixes and cleanups

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PASS      2.80 seconds
GitLint                       FAIL      1.45 seconds
SubjectPrefix                 PASS      0.40 seconds
BuildKernel                   PASS      30.14 seconds
CheckAllWarning               PASS      32.95 seconds
CheckSparse                   PASS      43.36 seconds
CheckSmatch                   FAIL      36.62 seconds
BuildKernel32                 PASS      29.42 seconds
TestRunnerSetup               PASS      526.69 seconds
TestRunner_l2cap-tester       PASS      20.37 seconds
TestRunner_iso-tester         FAIL      31.41 seconds
TestRunner_bnep-tester        PASS      4.68 seconds
TestRunner_mgmt-tester        PASS      107.70 seconds
TestRunner_rfcomm-tester      PASS      7.21 seconds
TestRunner_sco-tester         PASS      15.02 seconds
TestRunner_ioctl-tester       PASS      7.77 seconds
TestRunner_mesh-tester        FAIL      6.02 seconds
TestRunner_smp-tester         PASS      8.34 seconds
TestRunner_userchan-tester    PASS      5.18 seconds
IncrementalBuild              PASS      47.33 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[1/5] Bluetooth: qca: fix info leak when fetching fw build id

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
7: B3 Line contains hard tab characters (\t): "Cc: stable@vger.kernel.org	# 5.12"
[2/5] Bluetooth: qca: fix info leak when fetching board id

WARNING: I3 - ignore-body-lines: gitlint will be switching from using Python regex 'match' (match beginning) to 'search' (match anywhere) semantics. Please review your ignore-body-lines.regex option accordingly. To remove this warning, set general.regex-style-search=True. More details: https://jorisroovers.github.io/gitlint/configuration/#regex-style-search
6: B3 Line contains hard tab characters (\t): "Cc: stable@vger.kernel.org	# 6.7"
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139
make[4]: *** Deleting file 'net/bluetooth/hci_core.o'
make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: net] Error 2
make[2]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o'
make[4]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bpa10x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bpa10x.o'
make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: TestRunner_iso-tester - FAIL
Desc: Run iso-tester with test-runner
Output:
Total: 122, Passed: 121 (99.2%), Failed: 1, Not Run: 0

Failed Test Cases
ISO Connect2 Suspend - Success                       Failed       4.226 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 9 (90.0%), Failed: 1, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Failed       0.094 seconds


---
Regards,
Linux Bluetooth

[PATCH 2/5] Bluetooth: qca: fix info leak when fetching board id

[Johan Hovold]

Add the missing sanity check when fetching the board id to avoid leaking
slab data when later requesting the firmware.

Fixes: a7f8dedb4be2 ("Bluetooth: qca: add support for QCA2066")
Cc: stable@vger.kernel.org	# 6.7
Cc: Tim Jiang <quic_tjiang@quicinc.com>
Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/bluetooth/btqca.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index a508d79d9aaa..638074992c82 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -252,6 +252,11 @@ static int qca_read_fw_board_id(struct hci_dev *hdev, u16 *bid)
 		goto out;
 	}
 
+	if (skb->len < 3) {
+		err = -EILSEQ;
+		goto out;
+	}
+
 	*bid = (edl->data[1] << 8) + edl->data[2];
 	bt_dev_dbg(hdev, "%s: bid = %x", __func__, *bid);
 
-- 
2.43.2

[PATCH 3/5] Bluetooth: qca: drop bogus edl header checks

[Johan Hovold]

The skb->data pointer is never NULL so drop the bogus sanity checks when
initialising the EDL header pointer.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/bluetooth/btqca.c | 20 --------------------
 1 file changed, 20 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 638074992c82..2ba1f21f0320 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -55,11 +55,6 @@ int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
 	}
 
 	edl = (struct edl_event_hdr *)(skb->data);
-	if (!edl) {
-		bt_dev_err(hdev, "QCA TLV with no header");
-		err = -EILSEQ;
-		goto out;
-	}
 
 	if (edl->cresp != EDL_CMD_REQ_RES_EVT ||
 	    edl->rtype != rtype) {
@@ -121,11 +116,6 @@ static int qca_read_fw_build_info(struct hci_dev *hdev)
 	}
 
 	edl = (struct edl_event_hdr *)(skb->data);
-	if (!edl) {
-		bt_dev_err(hdev, "QCA read fw build info with no header");
-		err = -EILSEQ;
-		goto out;
-	}
 
 	if (edl->cresp != EDL_CMD_REQ_RES_EVT ||
 	    edl->rtype != EDL_GET_BUILD_INFO_CMD) {
@@ -183,11 +173,6 @@ static int qca_send_patch_config_cmd(struct hci_dev *hdev)
 	}
 
 	edl = (struct edl_event_hdr *)(skb->data);
-	if (!edl) {
-		bt_dev_err(hdev, "QCA Patch config with no header");
-		err = -EILSEQ;
-		goto out;
-	}
 
 	if (edl->cresp != EDL_PATCH_CONFIG_RES_EVT || edl->rtype != EDL_PATCH_CONFIG_CMD) {
 		bt_dev_err(hdev, "QCA Wrong packet received %d %d", edl->cresp,
@@ -502,11 +487,6 @@ static int qca_tlv_send_segment(struct hci_dev *hdev, int seg_size,
 	}
 
 	edl = (struct edl_event_hdr *)(skb->data);
-	if (!edl) {
-		bt_dev_err(hdev, "TLV with no header");
-		err = -EILSEQ;
-		goto out;
-	}
 
 	if (edl->cresp != EDL_CMD_REQ_RES_EVT || edl->rtype != rtype) {
 		bt_dev_err(hdev, "QCA TLV with error stat 0x%x rtype 0x%x",
-- 
2.43.2

[PATCH 4/5] Bluetooth: qca: drop bogus module version

[Johan Hovold]

Random module versions serves no purpose, what matters is the kernel
version.

Drop the bogus module version which has never been updated.

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/bluetooth/btqca.c | 5 +----
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 2ba1f21f0320..8980e31e5094 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -13,8 +13,6 @@
 
 #include "btqca.h"
 
-#define VERSION "0.1"
-
 int qca_read_soc_version(struct hci_dev *hdev, struct qca_btsoc_version *ver,
 			 enum qca_btsoc_type soc_type)
 {
@@ -941,6 +939,5 @@ EXPORT_SYMBOL_GPL(qca_set_bdaddr);
 
 
 MODULE_AUTHOR("Ben Young Tae Kim <ytkim@qca.qualcomm.com>");
-MODULE_DESCRIPTION("Bluetooth support for Qualcomm Atheros family ver " VERSION);
-MODULE_VERSION(VERSION);
+MODULE_DESCRIPTION("Bluetooth support for Qualcomm Atheros family");
 MODULE_LICENSE("GPL");
-- 
2.43.2

[PATCH 5/5] Bluetooth: qca: clean up defines

[Johan Hovold]

Clean up the QCA driver defines by dropping redundant parentheses around
values and making sure they are aligned (using tabs only).

Signed-off-by: Johan Hovold <johan+linaro@kernel.org>
---
 drivers/bluetooth/btqca.h | 60 +++++++++++++++++++--------------------
 1 file changed, 30 insertions(+), 30 deletions(-)

diff --git a/drivers/bluetooth/btqca.h b/drivers/bluetooth/btqca.h
index 215433fd76a1..bb5207d7a8c7 100644
--- a/drivers/bluetooth/btqca.h
+++ b/drivers/bluetooth/btqca.h
@@ -5,33 +5,33 @@
  *  Copyright (c) 2015 The Linux Foundation. All rights reserved.
  */
 
-#define EDL_PATCH_CMD_OPCODE		(0xFC00)
-#define EDL_NVM_ACCESS_OPCODE		(0xFC0B)
-#define EDL_WRITE_BD_ADDR_OPCODE	(0xFC14)
-#define EDL_PATCH_CMD_LEN		(1)
-#define EDL_PATCH_VER_REQ_CMD		(0x19)
-#define EDL_PATCH_TLV_REQ_CMD		(0x1E)
-#define EDL_GET_BUILD_INFO_CMD		(0x20)
-#define EDL_GET_BID_REQ_CMD			(0x23)
-#define EDL_NVM_ACCESS_SET_REQ_CMD	(0x01)
-#define EDL_PATCH_CONFIG_CMD		(0x28)
-#define MAX_SIZE_PER_TLV_SEGMENT	(243)
-#define QCA_PRE_SHUTDOWN_CMD		(0xFC08)
-#define QCA_DISABLE_LOGGING		(0xFC17)
-
-#define EDL_CMD_REQ_RES_EVT		(0x00)
-#define EDL_PATCH_VER_RES_EVT		(0x19)
-#define EDL_APP_VER_RES_EVT		(0x02)
-#define EDL_TVL_DNLD_RES_EVT		(0x04)
-#define EDL_CMD_EXE_STATUS_EVT		(0x00)
-#define EDL_SET_BAUDRATE_RSP_EVT	(0x92)
-#define EDL_NVM_ACCESS_CODE_EVT		(0x0B)
-#define EDL_PATCH_CONFIG_RES_EVT	(0x00)
-#define QCA_DISABLE_LOGGING_SUB_OP	(0x14)
+#define EDL_PATCH_CMD_OPCODE		0xFC00
+#define EDL_NVM_ACCESS_OPCODE		0xFC0B
+#define EDL_WRITE_BD_ADDR_OPCODE	0xFC14
+#define EDL_PATCH_CMD_LEN		1
+#define EDL_PATCH_VER_REQ_CMD		0x19
+#define EDL_PATCH_TLV_REQ_CMD		0x1E
+#define EDL_GET_BUILD_INFO_CMD		0x20
+#define EDL_GET_BID_REQ_CMD		0x23
+#define EDL_NVM_ACCESS_SET_REQ_CMD	0x01
+#define EDL_PATCH_CONFIG_CMD		0x28
+#define MAX_SIZE_PER_TLV_SEGMENT	243
+#define QCA_PRE_SHUTDOWN_CMD		0xFC08
+#define QCA_DISABLE_LOGGING		0xFC17
+
+#define EDL_CMD_REQ_RES_EVT		0x00
+#define EDL_PATCH_VER_RES_EVT		0x19
+#define EDL_APP_VER_RES_EVT		0x02
+#define EDL_TVL_DNLD_RES_EVT		0x04
+#define EDL_CMD_EXE_STATUS_EVT		0x00
+#define EDL_SET_BAUDRATE_RSP_EVT	0x92
+#define EDL_NVM_ACCESS_CODE_EVT		0x0B
+#define EDL_PATCH_CONFIG_RES_EVT	0x00
+#define QCA_DISABLE_LOGGING_SUB_OP	0x14
 
 #define EDL_TAG_ID_BD_ADDR		2
-#define EDL_TAG_ID_HCI			(17)
-#define EDL_TAG_ID_DEEP_SLEEP		(27)
+#define EDL_TAG_ID_HCI			17
+#define EDL_TAG_ID_DEEP_SLEEP		27
 
 #define QCA_WCN3990_POWERON_PULSE	0xFC
 #define QCA_WCN3990_POWEROFF_PULSE	0xC0
@@ -39,7 +39,7 @@
 #define QCA_HCI_CC_OPCODE		0xFC00
 #define QCA_HCI_CC_SUCCESS		0x00
 
-#define QCA_WCN3991_SOC_ID		(0x40014320)
+#define QCA_WCN3991_SOC_ID		0x40014320
 
 /* QCA chipset version can be decided by patch and SoC
  * version, combination with upper 2 bytes from SoC
@@ -48,11 +48,11 @@
 #define get_soc_ver(soc_id, rom_ver)	\
 	((le32_to_cpu(soc_id) << 16) | (le16_to_cpu(rom_ver)))
 
-#define QCA_HSP_GF_SOC_ID			0x1200
-#define QCA_HSP_GF_SOC_MASK			0x0000ff00
+#define QCA_HSP_GF_SOC_ID		0x1200
+#define QCA_HSP_GF_SOC_MASK		0x0000ff00
 
 enum qca_baudrate {
-	QCA_BAUDRATE_115200 	= 0,
+	QCA_BAUDRATE_115200	= 0,
 	QCA_BAUDRATE_57600,
 	QCA_BAUDRATE_38400,
 	QCA_BAUDRATE_19200,
@@ -71,7 +71,7 @@ enum qca_baudrate {
 	QCA_BAUDRATE_1600000,
 	QCA_BAUDRATE_3200000,
 	QCA_BAUDRATE_3500000,
-	QCA_BAUDRATE_AUTO 	= 0xFE,
+	QCA_BAUDRATE_AUTO	= 0xFE,
 	QCA_BAUDRATE_RESERVED
 };
 
-- 
2.43.2

Re: [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups

[patchwork-bot+bluetooth]

Hello:

This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Wed,  1 May 2024 14:34:51 +0200 you wrote:
> Here are two fixes for potential info leaks in the QCA driver and a few
> cleanups.
> 
> All of these can go into 6.10.
> 
> Johan
> 
> [...]

Here is the summary with links:
  - [1/5] Bluetooth: qca: fix info leak when fetching fw build id
    https://git.kernel.org/bluetooth/bluetooth-next/c/cfc2a7747108
  - [2/5] Bluetooth: qca: fix info leak when fetching board id
    https://git.kernel.org/bluetooth/bluetooth-next/c/3e2faecb09fb
  - [3/5] Bluetooth: qca: drop bogus edl header checks
    https://git.kernel.org/bluetooth/bluetooth-next/c/ca8934466039
  - [4/5] Bluetooth: qca: drop bogus module version
    https://git.kernel.org/bluetooth/bluetooth-next/c/2684457bf2dd
  - [5/5] Bluetooth: qca: clean up defines
    https://git.kernel.org/bluetooth/bluetooth-next/c/f50efbe27afd

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html