LKML Archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups
@ 2024-05-01 12:34 Johan Hovold
  2024-05-01 12:34 ` [PATCH 1/5] Bluetooth: qca: fix info leak when fetching fw build id Johan Hovold
                   ` (5 more replies)
  0 siblings, 6 replies; 7+ messages in thread
From: Johan Hovold @ 2024-05-01 12:34 UTC (permalink / raw
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: Janaki Ramaiah Thota, linux-bluetooth, linux-kernel, 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


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH 1/5] Bluetooth: qca: fix info leak when fetching fw build id
  2024-05-01 12:34 [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups Johan Hovold
@ 2024-05-01 12:34 ` Johan Hovold
  2024-05-01 12:34 ` [PATCH 2/5] Bluetooth: qca: fix info leak when fetching board id Johan Hovold
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2024-05-01 12:34 UTC (permalink / raw
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: Janaki Ramaiah Thota, linux-bluetooth, linux-kernel, Johan Hovold,
	stable

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 2/5] Bluetooth: qca: fix info leak when fetching board id
  2024-05-01 12:34 [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups Johan Hovold
  2024-05-01 12:34 ` [PATCH 1/5] Bluetooth: qca: fix info leak when fetching fw build id Johan Hovold
@ 2024-05-01 12:34 ` Johan Hovold
  2024-05-01 12:34 ` [PATCH 3/5] Bluetooth: qca: drop bogus edl header checks Johan Hovold
                   ` (3 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2024-05-01 12:34 UTC (permalink / raw
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: Janaki Ramaiah Thota, linux-bluetooth, linux-kernel, Johan Hovold,
	stable, Tim Jiang

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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 3/5] Bluetooth: qca: drop bogus edl header checks
  2024-05-01 12:34 [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups Johan Hovold
  2024-05-01 12:34 ` [PATCH 1/5] Bluetooth: qca: fix info leak when fetching fw build id Johan Hovold
  2024-05-01 12:34 ` [PATCH 2/5] Bluetooth: qca: fix info leak when fetching board id Johan Hovold
@ 2024-05-01 12:34 ` Johan Hovold
  2024-05-01 12:34 ` [PATCH 4/5] Bluetooth: qca: drop bogus module version Johan Hovold
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2024-05-01 12:34 UTC (permalink / raw
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: Janaki Ramaiah Thota, linux-bluetooth, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 4/5] Bluetooth: qca: drop bogus module version
  2024-05-01 12:34 [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups Johan Hovold
                   ` (2 preceding siblings ...)
  2024-05-01 12:34 ` [PATCH 3/5] Bluetooth: qca: drop bogus edl header checks Johan Hovold
@ 2024-05-01 12:34 ` Johan Hovold
  2024-05-01 12:34 ` [PATCH 5/5] Bluetooth: qca: clean up defines Johan Hovold
  2024-05-01 20:00 ` [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups patchwork-bot+bluetooth
  5 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2024-05-01 12:34 UTC (permalink / raw
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: Janaki Ramaiah Thota, linux-bluetooth, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH 5/5] Bluetooth: qca: clean up defines
  2024-05-01 12:34 [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups Johan Hovold
                   ` (3 preceding siblings ...)
  2024-05-01 12:34 ` [PATCH 4/5] Bluetooth: qca: drop bogus module version Johan Hovold
@ 2024-05-01 12:34 ` Johan Hovold
  2024-05-01 20:00 ` [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups patchwork-bot+bluetooth
  5 siblings, 0 replies; 7+ messages in thread
From: Johan Hovold @ 2024-05-01 12:34 UTC (permalink / raw
  To: Marcel Holtmann, Luiz Augusto von Dentz
  Cc: Janaki Ramaiah Thota, linux-bluetooth, linux-kernel, 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


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups
  2024-05-01 12:34 [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups Johan Hovold
                   ` (4 preceding siblings ...)
  2024-05-01 12:34 ` [PATCH 5/5] Bluetooth: qca: clean up defines Johan Hovold
@ 2024-05-01 20:00 ` patchwork-bot+bluetooth
  5 siblings, 0 replies; 7+ messages in thread
From: patchwork-bot+bluetooth @ 2024-05-01 20:00 UTC (permalink / raw
  To: Johan Hovold
  Cc: marcel, luiz.dentz, quic_janathot, linux-bluetooth, linux-kernel

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



^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2024-05-01 20:00 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-05-01 12:34 [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups Johan Hovold
2024-05-01 12:34 ` [PATCH 1/5] Bluetooth: qca: fix info leak when fetching fw build id Johan Hovold
2024-05-01 12:34 ` [PATCH 2/5] Bluetooth: qca: fix info leak when fetching board id Johan Hovold
2024-05-01 12:34 ` [PATCH 3/5] Bluetooth: qca: drop bogus edl header checks Johan Hovold
2024-05-01 12:34 ` [PATCH 4/5] Bluetooth: qca: drop bogus module version Johan Hovold
2024-05-01 12:34 ` [PATCH 5/5] Bluetooth: qca: clean up defines Johan Hovold
2024-05-01 20:00 ` [PATCH 0/5] Bluetooth: qca: info leak fixes and cleanups patchwork-bot+bluetooth

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).