2024-05-01 12:35:34

by Johan Hovold

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

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



2024-05-01 12:35:40

by Johan Hovold

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

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

Signed-off-by: Johan Hovold <[email protected]>
---
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


2024-05-01 12:35:41

by Johan Hovold

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

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: [email protected] # 6.7
Cc: Tim Jiang <[email protected]>
Signed-off-by: Johan Hovold <[email protected]>
---
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


2024-05-01 12:35:41

by Johan Hovold

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

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: [email protected] # 5.12
Signed-off-by: Johan Hovold <[email protected]>
---
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


2024-05-01 12:35:44

by Johan Hovold

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

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 <[email protected]>
---
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 <[email protected]>");
-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


2024-05-01 12:35:56

by Johan Hovold

[permalink] [raw]
Subject: [PATCH 5/5] Bluetooth: qca: clean up defines

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 <[email protected]>
---
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


2024-05-01 13:10:58

by bluez.test.bot

[permalink] [raw]
Subject: RE: Bluetooth: qca: info leak fixes and cleanups

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: [email protected] # 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: [email protected] # 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

2024-05-01 20:00:53

by patchwork-bot+bluetooth

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

Hello:

This series was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[email protected]>:

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