[PATCH] wifi: mt76: mt7921: validate CLC firmware records

[PATCH] wifi: mt76: mt7921: validate CLC firmware records

[Laxman Acharya Padhya]

The CLC region is supplied by firmware, but the loader trusts the
region count and each record length. A malformed image can make the
region table pointer precede the firmware buffer, make the record loop
fail to advance, or index phy->clc past its end. Validate the table and
record bounds before dereferencing or copying.

Fixes: 23bdc5d8cadf ("wifi: mt76: mt7921: introduce Country Location
Control support")
Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>

---
 drivers/net/wireless/mediatek/mt76/mt7921/mcu.c | 28 +++++++++++++++++++++----
 1 file changed, 24 insertions(+), 4 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
index 25b9437250f7..564dd836e0b3 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
@@ -415,7 +415,8 @@ static int mt7921_load_clc(struct mt792x_dev *dev,
const char *fw_name)
 	struct mt76_dev *mdev = &dev->mt76;
 	struct mt792x_phy *phy = &dev->phy;
 	const struct firmware *fw;
-	int ret, i, len, offset = 0;
+	size_t clc_len, fw_data_len, len, offset = 0;
+	int ret, i;
 	u8 *clc_base = NULL, hw_encap = 0;

 	dev->phy.clc_chan_conf = 0xff;
@@ -441,13 +442,21 @@ static int mt7921_load_clc(struct mt792x_dev
*dev, const char *fw_name)
 	}

 	hdr = (const void *)(fw->data + fw->size - sizeof(*hdr));
+	if (hdr->n_region > (fw->size - sizeof(*hdr)) / sizeof(*region)) {
+		dev_err(mdev->dev, "Invalid firmware region table\n");
+		ret = -EINVAL;
+		goto out;
+	}
+	fw_data_len = fw->size - sizeof(*hdr) -
+		      hdr->n_region * sizeof(*region);
+
 	for (i = 0; i < hdr->n_region; i++) {
 		region = (const void *)((const u8 *)hdr -
 					(hdr->n_region - i) * sizeof(*region));
 		len = le32_to_cpu(region->len);

 		/* check if we have valid buffer size */
-		if (offset + len > fw->size) {
+		if (len > fw_data_len - offset) {
 			dev_err(mdev->dev, "Invalid firmware region\n");
 			ret = -EINVAL;
 			goto out;
@@ -464,8 +473,19 @@ static int mt7921_load_clc(struct mt792x_dev
*dev, const char *fw_name)
 	if (!clc_base)
 		goto out;

-	for (offset = 0; offset < len; offset += le32_to_cpu(clc->len)) {
+	for (offset = 0; offset < len; offset += clc_len) {
+		if (len - offset < sizeof(*clc)) {
+			ret = -EINVAL;
+			goto out;
+		}
+
 		clc = (const struct mt7921_clc *)(clc_base + offset);
+		clc_len = le32_to_cpu(clc->len);
+		if (clc_len < sizeof(*clc) || clc_len > len - offset ||
+		    clc->idx >= ARRAY_SIZE(phy->clc)) {
+			ret = -EINVAL;
+			goto out;
+		}

 		/* do not init buf again if chip reset triggered */
 		if (phy->clc[clc->idx])
@@ -477,7 +497,7 @@ static int mt7921_load_clc(struct mt792x_dev *dev,
const char *fw_name)
 			continue;

 		phy->clc[clc->idx] = devm_kmemdup(mdev->dev, clc,
-						  le32_to_cpu(clc->len),
+						  clc_len,
 						  GFP_KERNEL);

 		if (!phy->clc[clc->idx]) {

Re: [PATCH] wifi: mt76: mt7921: validate CLC firmware records

[Mikhail Gavrilov]

This patch is in linux-next as 9417c5818a01, and on current shipping
firmware it stops mt7921e from probing at all.

An unknown clc->idx is treated as fatal:

	if (clc_len < sizeof(*clc) || clc_len > len - offset ||
	    clc->idx >= ARRAY_SIZE(phy->clc)) {
		ret = -EINVAL;
		goto out;
	}

but linux-firmware 20260810 ships an MT7922 image that legitimately
carries records with idx 3. Dumping the CLC region of
WIFI_RAM_CODE_MT7922_1.bin before and after that firmware update:

  20260622, region len 366448:
    idx=0 ver=1 nr_country=255 type=0 len=179384
    idx=0 ver=1 nr_country=255 type=1 len=187064

  20260810, region len 475488:
    idx=0 ver=1 nr_country=255 type=0 len=179384
    idx=0 ver=1 nr_country=255 type=1 len=187054
    idx=3 ver=1 nr_country=0   type=0 len=54520
    idx=3 ver=1 nr_country=0   type=1 len=54530

So mt7921_load_clc() returns -EINVAL, mt7921_run_firmware() fails, and
the probe retries the firmware ten times before giving up:

  mt7921e 0000:0b:00.0: ASIC revision: 79220010
  mt7921e 0000:0b:00.0: HW/SW Version: 0x8a108a10, Build Time: 20260724143257a
  mt7921e 0000:0b:00.0: WM Firmware Version: ____000000, Build Time: 20260724143402
  [ ... nine more identical pairs ... ]
  mt7921e 0000:0b:00.0: hardware init failed

No interface is created. There is no dev_err on this path, so the only
clue is the retry loop. Loading mt7921_common with disable_clc=1, which
makes mt7921_load_clc() return before any of this, brings the device up
normally and the firmware is loaded once rather than ten times.

The rest of the patch is a real improvement - bounds-checking the region
table and each record length also closes an infinite loop on a
zero-length record. Only the handling of an unknown index needs to
change: an index the driver does not know about is not a malformed
image, it is a newer firmware, and the record should be skipped:

	if (clc_len < sizeof(*clc) || clc_len > len - offset) {
		ret = -EINVAL;
		goto out;
	}

	if (clc->idx >= ARRAY_SIZE(phy->clc))
		continue;

I sent an overlapping patch yesterday, "wifi: mt76: mt7921: fix
array-index-out-of-bounds in mt7921_load_clc()", before I found this one
in linux-next; I will mark mine superseded, since this patch is the
broader fix. Happy to send the one-line change above as a follow-up
patch and test it here, or leave it to you to fold in before this
reaches mainline.

Separately, it would be useful to know what CLC index 3 actually
contains - roughly 109 KB of payload in two hw_encap variants that the
driver currently discards either way.

[PATCH] wifi: mt76: mt7921: skip unknown CLC firmware records

[Laxman Acharya Padhya]

Treat an out-of-range CLC index as newer firmware rather than a
malformed image. linux-firmware 20260810 ships MT7922 records with
idx 3, and rejecting them made mt7921e fail to probe.

Keep the record-length checks, and report those as errors so a
truncated table is visible instead of a silent retry loop.

Fixes: 9417c5818a01 ("wifi: mt76: mt7921: validate CLC firmware records")
Reported-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>
Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>
---
 drivers/net/wireless/mediatek/mt76/mt7921/mcu.c | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
index a118a301564c..40546005c743 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/mcu.c
@@ -477,18 +477,23 @@ static int mt7921_load_clc(struct mt792x_dev *dev, const char *fw_name)
 
 	for (offset = 0; offset < len; offset += clc_len) {
 		if (len - offset < sizeof(*clc)) {
+			dev_err(mdev->dev, "Invalid CLC record\n");
 			ret = -EINVAL;
 			goto out;
 		}
 
 		clc = (const struct mt7921_clc *)(clc_base + offset);
 		clc_len = le32_to_cpu(clc->len);
-		if (clc_len < sizeof(*clc) || clc_len > len - offset ||
-		    clc->idx >= ARRAY_SIZE(phy->clc)) {
+		if (clc_len < sizeof(*clc) || clc_len > len - offset) {
+			dev_err(mdev->dev, "Invalid CLC record\n");
 			ret = -EINVAL;
 			goto out;
 		}
 
+		/* Newer firmware may add records this driver does not use yet */
+		if (clc->idx >= ARRAY_SIZE(phy->clc))
+			continue;
+
 		/* do not init buf again if chip reset triggered */
 		if (phy->clc[clc->idx])
 			continue;
-- 
2.51.2

Re: [PATCH] wifi: mt76: mt7921: skip unknown CLC firmware records

[Mikhail Gavrilov]

Thanks for the quick turnaround.

Tested on an MT7922 (mt7921e) with linux-firmware 20260810 - the image
whose CLC region carries the idx 3 records - on a lockdep and UBSAN
build of mainline at fd923b32d761, with 9417c5818a01 applied underneath.

The device probes normally: the RAM firmware is loaded once rather than
ten times, wlp11s0 appears, association with a 6 GHz AP works (channel
37, 6135 MHz, 160 MHz), and switching between a 5 GHz and a 6 GHz BSS
and back is clean. No "hardware init failed", no "Invalid CLC record",
and no UBSAN report - so the bounds checks are still doing their job on
the very firmware that used to trip them.

Tested-by: Mikhail Gavrilov <mikhail.v.gavrilov@gmail.com>

One nit, take it or leave it: both length failures print the same
"Invalid CLC record", so the log does not distinguish a table that ends
mid-header from a record whose length is out of range. Two distinct
strings would make a future report easier to read.