All of lore.kernel.org
 help / color / mirror / Atom feed
From: Qianqiang Liu <qianqiang.liu@163.com>
To: mingyen.hsieh@mediatek.com
Cc: nbd@nbd.name, lorenzo@kernel.org, deren.wu@mediatek.com,
	linux-mediatek@lists.infradead.org
Subject: Is this a out-of-bounds issue?
Date: Thu, 12 Sep 2024 21:26:45 +0800	[thread overview]
Message-ID: <ZuLsFWd6yg07B20y@thinkpad.> (raw)

Hi,

The code in drivers/net/wireless/mediatek/mt76/mt7925/mcu.c may have a
out-of-bounds issue:

638         for (offset = 0; offset < len; offset += le32_to_cpu(clc->len)) {
639                 clc = (const struct mt7925_clc *)(clc_base + offset);
640
641                 if (clc->idx > ARRAY_SIZE(phy->clc)) <-
642                         break;
643
644                 /* do not init buf again if chip reset triggered */
645                 if (phy->clc[clc->idx])
646                         continue;
647
648                 phy->clc[clc->idx] = devm_kmemdup(mdev->dev, clc,
649                                                   le32_to_cpu(clc->len),
650                                                   GFP_KERNEL);
651
652                 if (!phy->clc[clc->idx]) {
653                         ret = -ENOMEM;
654                         goto out;
655                 }
656         }

Let's say the array size of "phy->clc" is 2, then the valid index is 0 and 1.
If "clc->idx" is 2, "clc->idx > ARRAY_SIZE(phy->clc)" must be false, the "break"
statement won't be executed, and "phy->clc[2]" may access illegal memory address.

So, should we modify the code like this?

diff --git a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
index 748ea6adbc6b..0c2a2337c313 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
@@ -638,7 +638,7 @@ static int mt7925_load_clc(struct mt792x_dev *dev, const char *fw_name)
 	for (offset = 0; offset < len; offset += le32_to_cpu(clc->len)) {
 		clc = (const struct mt7925_clc *)(clc_base + offset);
 
-		if (clc->idx > ARRAY_SIZE(phy->clc))
+		if (clc->idx >= ARRAY_SIZE(phy->clc))
 			break;
 
 		/* do not init buf again if chip reset triggered */

-- 
Best,
Qianqiang Li 



             reply	other threads:[~2024-09-12 13:27 UTC|newest]

Thread overview: 3+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-09-12 13:26 Qianqiang Liu [this message]
     [not found] <66e2ec22.050a0220.37047b.174cSMTPIN_ADDED_BROKEN@mx.google.com>
2024-09-12 13:34 ` Is this a out-of-bounds issue? Lorenzo Bianconi
2024-09-12 14:20   ` Qianqiang Liu

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=ZuLsFWd6yg07B20y@thinkpad. \
    --to=qianqiang.liu@163.com \
    --cc=deren.wu@mediatek.com \
    --cc=linux-mediatek@lists.infradead.org \
    --cc=lorenzo@kernel.org \
    --cc=mingyen.hsieh@mediatek.com \
    --cc=nbd@nbd.name \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.