[PATCH 0/3] Bluetooth: btmtk: Harden firmware parsing and improve logging

[PATCH 0/3] Bluetooth: btmtk: Harden firmware parsing and improve logging

[Chris Lu]

Three changes to the shared btmtk firmware download path, all in
preparation for MT7928 support but useful on their own.

Patch 1 bounds-checks the firmware image before the section map walk in
btmtk_setup_firmware_79xx(). Today the section count, the section map
array and each section's offset/length come straight out of the file and
are never compared against fw->size, so a truncated or corrupted file
makes the driver read past the end of request_firmware()'s buffer. The
section count is a __le32 from the file, so on 32-bit builds multiplying
it by the 64-byte map size wraps a size_t and a bound computed without an
overflow check would come out small enough to accept the file; both
helpers order their arithmetic so nothing can wrap. A section count of
zero is rejected too, since it passes every size check but would leave
the download loop with nothing to do and still report success.

The checks live in two helpers rather than inline because the MT7928
CBMCU download path added later needs exactly the same arithmetic and
should not carry a second copy of it. Bounding dlsize by fw->size also
removes an existing hazard in the download loop: dlen is computed as
min_t(int, 250, dl_size) from an otherwise unbounded __le32, so a large
enough value turned dlen negative and "dl_size -= dlen" then grew
dl_size instead of shrinking it.

Patch 2 makes the log line more useful: it never said which file was
requested, it reported the firmware's own hwver field as the HW version
rather than the device id the driver read from the chip, and it printed
the 16-byte datetime array with %s even though the array need not be
NUL-terminated. Both callers pass a real device id - btmtksdio reads it
from register 0x70010200 and btusb switches on it before getting here.

Patch 3 replaces the bare 1/2/3 sequence flags on BTMTK_WMT_PATCH_DWNLD
packets with a named enum. No functional change. The other bare flag
values in the driver belong to other WMT opcodes, where the field means
something different, and are left alone. Paul Menzel reviewed this change
in an earlier MT7928 series; the enum values are unchanged here, the only
difference being a comment added above it.

Testing
=======

Compile-tested with CONFIG_BT_MTK, CONFIG_BT_HCIBTUSB and
CONFIG_BT_MTKSDIO as modules, each patch applied individually, no new
warnings.

Runtime-tested on MT7922 (USB 0e8d:223c) over repeated unplug/replug and
Bluetooth on/off cycles; it comes up every time and the firmware download
is unchanged:

  [  365.233785] usb 1-2: New USB device found, idVendor=0e8d, idProduct=223c
  [  365.245951] Bluetooth: hci0: Loading BT firmware: mediatek/BT_RAM_CODE_MT7922_1_1_hdr.bin
  [  365.245956] Bluetooth: hci0: BT HW ver: 0x7922, SW ver: 0x008a, Build Time: 20260605203811
  [  367.542038] Bluetooth: hci0: Device setup in 2244536 usecs
  [  367.601570] Bluetooth: hci0: AOSP extensions version v1.00
  [  367.601581] Bluetooth: hci0: AOSP quality report is supported

Chris Lu (3):
  Bluetooth: btmtk: Validate the firmware layout before parsing it
  Bluetooth: btmtk: Improve BT firmware logging
  Bluetooth: btmtk: Replace magic numbers with WMT packet flag enum

 drivers/bluetooth/btmtk.c | 92 +++++++++++++++++++++++++++++++++++-----
 drivers/bluetooth/btmtk.h |  9 ++++
 2 files changed, 90 insertions(+), 11 deletions(-)


base-commit: 701ca71884b3d101fd25b7adbf972355056ef352
--
2.45.2

[PATCH 1/3] Bluetooth: btmtk: Validate the firmware layout before parsing it

[Chris Lu]

btmtk_setup_firmware_79xx() reads the section count out of the firmware's
global descriptor, then walks a 64-byte section map per section and hands
each section's file-supplied offset and length to the WMT download path.
None of that is checked against fw->size, so a truncated or corrupted
file makes the driver read past the end of request_firmware()'s buffer.
On 32-bit the section count can also wrap the section map size
calculation.

Add btmtk_fw_validate_layout() to check that the header, global
descriptor and section map array all fit, and btmtk_fw_validate_section()
to check one section's offset and length, both written so the size
arithmetic cannot wrap. A section count of zero is rejected as well: it
passes every size check but leaves the download loop with nothing to do,
so the function would report success on hardware it never programmed.
They are helpers rather than open-coded because the MT7928 CBMCU download
path added later needs the same arithmetic.

This is hardening against a corrupted /lib/firmware rather than a bug an
unprivileged user can trigger, hence bluetooth-next and no Fixes: tag.
Well-formed firmware files are unaffected.

Signed-off-by: Chris Lu <chris.lu@mediatek.com>
Assisted-by: Claude:claude-opus-5
---
 drivers/bluetooth/btmtk.c | 74 +++++++++++++++++++++++++++++++++++++--
 1 file changed, 71 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index 911aba5c134e..07a7a7a3656a 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -6,6 +6,7 @@
 #include <linux/firmware.h>
 #include <linux/usb.h>
 #include <linux/iopoll.h>
+#include <linux/overflow.h>
 #include <linux/unaligned.h>
 
 #include <net/bluetooth/bluetooth.h>
@@ -59,6 +60,67 @@ struct btmtk_section_map {
 	};
 } __packed;
 
+/* Reject a firmware image whose header, global descriptor or section map
+ * array does not fit in the file, and hand back the validated section
+ * count, so that the section map walks stay inside the firmware buffer.
+ */
+static int btmtk_fw_validate_layout(struct hci_dev *hdev,
+				    const struct firmware *fw,
+				    u32 *section_num)
+{
+	const struct btmtk_global_desc *globaldesc;
+	size_t need;
+
+	if (fw->size < MTK_FW_ROM_PATCH_HEADER_SIZE + MTK_FW_ROM_PATCH_GD_SIZE) {
+		bt_dev_err(hdev, "Firmware too small: %zu bytes, need at least %u",
+			   fw->size,
+			   MTK_FW_ROM_PATCH_HEADER_SIZE + MTK_FW_ROM_PATCH_GD_SIZE);
+		return -EINVAL;
+	}
+
+	globaldesc = (const struct btmtk_global_desc *)(fw->data +
+			MTK_FW_ROM_PATCH_HEADER_SIZE);
+	*section_num = le32_to_cpu(globaldesc->section_num);
+
+	if (!*section_num) {
+		bt_dev_err(hdev, "Firmware declares no sections");
+		return -EINVAL;
+	}
+
+	if (check_mul_overflow((size_t)MTK_FW_ROM_PATCH_SEC_MAP_SIZE,
+			       (size_t)*section_num, &need) ||
+	    check_add_overflow(need,
+			       (size_t)(MTK_FW_ROM_PATCH_HEADER_SIZE +
+					MTK_FW_ROM_PATCH_GD_SIZE),
+			       &need)) {
+		bt_dev_err(hdev, "Firmware section count too large: %u",
+			   *section_num);
+		return -EINVAL;
+	}
+
+	if (fw->size < need) {
+		bt_dev_err(hdev, "Firmware truncated: %zu bytes, need %zu for %u sections",
+			   fw->size, need, *section_num);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
+/* Reject a section whose payload does not lie within the firmware file. */
+static int btmtk_fw_validate_section(struct hci_dev *hdev,
+				     const struct firmware *fw, int index,
+				     u32 offset, u32 size)
+{
+	if (offset > fw->size || size > fw->size - offset) {
+		bt_dev_err(hdev, "Firmware section %d out of bounds: offset=%u, size=%u, fw_size=%zu",
+			   index, offset, size, fw->size);
+		return -EINVAL;
+	}
+
+	return 0;
+}
+
 static void btmtk_coredump(struct hci_dev *hdev)
 {
 	int err;
@@ -137,7 +199,6 @@ int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,
 {
 	struct btmtk_hci_wmt_params wmt_params;
 	struct btmtk_patch_header *hdr;
-	struct btmtk_global_desc *globaldesc = NULL;
 	struct btmtk_section_map *sectionmap;
 	const struct firmware *fw;
 	const u8 *fw_ptr;
@@ -153,11 +214,13 @@ int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,
 		return err;
 	}
 
+	err = btmtk_fw_validate_layout(hdev, fw, &section_num);
+	if (err < 0)
+		goto err_release_fw;
+
 	fw_ptr = fw->data;
 	fw_bin_ptr = fw_ptr;
 	hdr = (struct btmtk_patch_header *)fw_ptr;
-	globaldesc = (struct btmtk_global_desc *)(fw_ptr + MTK_FW_ROM_PATCH_HEADER_SIZE);
-	section_num = le32_to_cpu(globaldesc->section_num);
 
 	bt_dev_info(hdev, "HW/SW Version: 0x%04x%04x, Build Time: %s",
 		    le16_to_cpu(hdr->hwver), le16_to_cpu(hdr->swver), hdr->datetime);
@@ -180,6 +243,11 @@ int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,
 			continue;
 
 		if (dl_size > 0) {
+			err = btmtk_fw_validate_section(hdev, fw, i,
+							section_offset, dl_size);
+			if (err < 0)
+				goto err_release_fw;
+
 			retry = 20;
 			while (retry > 0) {
 				cmd[0] = 0; /* 0 means legacy dl mode. */
-- 
2.45.2

[PATCH 2/3] Bluetooth: btmtk: Improve BT firmware logging

[Chris Lu]

The firmware log line in btmtk_setup_firmware_79xx() never says which
file was requested, which matters because btmtk_fw_get_filename() derives
the name from the device id and firmware version at runtime. It reports
the firmware's own hwver field as the HW version, when the device id the
driver read from the chip is the more useful value and is already
available as dev_id. And it prints datetime, a u8[16] with no guaranteed
NUL terminator, with %s.

Log the filename before the image is parsed, so a file that fails
validation still says which file it was, report dev_id as the HW version,
and bound datetime with %.16s.

The datetime change is hardening rather than a fix, hence no Fixes: tag:
every file in linux-firmware terminates the field, and provoking the
over-read needs a malformed file in /lib/firmware.

Log output on MT7922 after the change:

  [  365.245951] Bluetooth: hci0: Loading BT firmware: mediatek/BT_RAM_CODE_MT7922_1_1_hdr.bin
  [  365.245956] Bluetooth: hci0: BT HW ver: 0x7922, SW ver: 0x008a, Build Time: 20260605203811

Signed-off-by: Chris Lu <chris.lu@mediatek.com>
Assisted-by: Claude:claude-opus-5
---
 drivers/bluetooth/btmtk.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index 07a7a7a3656a..cf4f40349afa 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -214,6 +214,8 @@ int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,
 		return err;
 	}
 
+	bt_dev_info(hdev, "Loading BT firmware: %s", fwname);
+
 	err = btmtk_fw_validate_layout(hdev, fw, &section_num);
 	if (err < 0)
 		goto err_release_fw;
@@ -222,8 +224,8 @@ int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,
 	fw_bin_ptr = fw_ptr;
 	hdr = (struct btmtk_patch_header *)fw_ptr;
 
-	bt_dev_info(hdev, "HW/SW Version: 0x%04x%04x, Build Time: %s",
-		    le16_to_cpu(hdr->hwver), le16_to_cpu(hdr->swver), hdr->datetime);
+	bt_dev_info(hdev, "BT HW ver: 0x%04x, SW ver: 0x%04x, Build Time: %.16s",
+		    dev_id & 0xffff, le16_to_cpu(hdr->swver), hdr->datetime);
 
 	for (i = 0; i < section_num; i++) {
 		first_block = 1;
-- 
2.45.2

[PATCH 3/3] Bluetooth: btmtk: Replace magic numbers with WMT packet flag enum

[Chris Lu]

The flag field of a BTMTK_WMT_PATCH_DWNLD packet tells the device where
the packet sits in the download sequence, but both download loops write
the bare values 1, 2 and 3, so the reader has to infer the meaning from
the surrounding conditionals.

Add enum btmtk_wmt_pkt_flag and use it in btmtk_setup_firmware_79xx()
and btmtk_setup_firmware(). No functional change.

The other bare flag values in this driver belong to different WMT
opcodes (BTMTK_WMT_FUNC_CTRL, BTMTK_WMT_RST, BTMTK_WMT_SEMAPHORE and so
on), where the field means something else entirely, so they are left
alone.

Signed-off-by: Chris Lu <chris.lu@mediatek.com>
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Assisted-by: Claude:claude-opus-5
---
 drivers/bluetooth/btmtk.c | 12 ++++++------
 drivers/bluetooth/btmtk.h |  9 +++++++++
 2 files changed, 15 insertions(+), 6 deletions(-)

diff --git a/drivers/bluetooth/btmtk.c b/drivers/bluetooth/btmtk.c
index cf4f40349afa..660ed5b02841 100644
--- a/drivers/bluetooth/btmtk.c
+++ b/drivers/bluetooth/btmtk.c
@@ -300,12 +300,12 @@ int btmtk_setup_firmware_79xx(struct hci_dev *hdev, const char *fwname,
 			while (dl_size > 0) {
 				dlen = min_t(int, 250, dl_size);
 				if (first_block == 1) {
-					flag = 1;
+					flag = BTMTK_WMT_PKT_START;
 					first_block = 0;
 				} else if (dl_size - dlen <= 0) {
-					flag = 3;
+					flag = BTMTK_WMT_PKT_END;
 				} else {
-					flag = 2;
+					flag = BTMTK_WMT_PKT_CONTINUE;
 				}
 
 				wmt_params.flag = flag;
@@ -384,7 +384,7 @@ int btmtk_setup_firmware(struct hci_dev *hdev, const char *fwname,
 
 	fw_size -= 30;
 	fw_ptr += 30;
-	flag = 1;
+	flag = BTMTK_WMT_PKT_START;
 
 	wmt_params.op = BTMTK_WMT_PATCH_DWNLD;
 	wmt_params.status = NULL;
@@ -394,9 +394,9 @@ int btmtk_setup_firmware(struct hci_dev *hdev, const char *fwname,
 
 		/* Tell device the position in sequence */
 		if (fw_size - dlen <= 0)
-			flag = 3;
+			flag = BTMTK_WMT_PKT_END;
 		else if (fw_size < fw->size - 30)
-			flag = 2;
+			flag = BTMTK_WMT_PKT_CONTINUE;
 
 		wmt_params.flag = flag;
 		wmt_params.dlen = dlen;
diff --git a/drivers/bluetooth/btmtk.h b/drivers/bluetooth/btmtk.h
index bc26148ec544..e8ad281a93a0 100644
--- a/drivers/bluetooth/btmtk.h
+++ b/drivers/bluetooth/btmtk.h
@@ -67,6 +67,15 @@ enum {
 	BTMTK_WMT_ON_PROGRESS,
 };
 
+/* Position of a BTMTK_WMT_PATCH_DWNLD packet within a download sequence,
+ * carried in struct btmtk_wmt_hdr's flag field.
+ */
+enum btmtk_wmt_pkt_flag {
+	BTMTK_WMT_PKT_START = 1,
+	BTMTK_WMT_PKT_CONTINUE = 2,
+	BTMTK_WMT_PKT_END = 3,
+};
+
 struct btmtk_wmt_hdr {
 	u8	dir;
 	u8	op;
-- 
2.45.2

Re: [PATCH 0/3] Bluetooth: btmtk: Harden firmware parsing and improve logging

[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, 9 Sep 2026 20:00:08 +0800 you wrote:
> Three changes to the shared btmtk firmware download path, all in
> preparation for MT7928 support but useful on their own.
> 
> Patch 1 bounds-checks the firmware image before the section map walk in
> btmtk_setup_firmware_79xx(). Today the section count, the section map
> array and each section's offset/length come straight out of the file and
> are never compared against fw->size, so a truncated or corrupted file
> makes the driver read past the end of request_firmware()'s buffer. The
> section count is a __le32 from the file, so on 32-bit builds multiplying
> it by the 64-byte map size wraps a size_t and a bound computed without an
> overflow check would come out small enough to accept the file; both
> helpers order their arithmetic so nothing can wrap. A section count of
> zero is rejected too, since it passes every size check but would leave
> the download loop with nothing to do and still report success.
> 
> [...]

Here is the summary with links:
  - [1/3] Bluetooth: btmtk: Validate the firmware layout before parsing it
    https://git.kernel.org/bluetooth/bluetooth-next/c/71147d59cb23
  - [2/3] Bluetooth: btmtk: Improve BT firmware logging
    https://git.kernel.org/bluetooth/bluetooth-next/c/4a7b93f08d13
  - [3/3] Bluetooth: btmtk: Replace magic numbers with WMT packet flag enum
    https://git.kernel.org/bluetooth/bluetooth-next/c/1b737f2ef168

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