[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <ec505433-3929-4a17-b875-198edab3066a@huawei.com>
Date: Fri, 25 Oct 2024 16:26:04 +0800
From: "zhangzekun (A)" <zhangzekun11@...wei.com>
To: Dan Carpenter <dan.carpenter@...aro.org>, <oe-kbuild@...ts.linux.dev>,
Ming Yen Hsieh <mingyen.hsieh@...iatek.com>
CC: <lkp@...el.com>, <oe-kbuild-all@...ts.linux.dev>,
<linux-kernel@...r.kernel.org>, Felix Fietkau <nbd@....name>
Subject: Re: drivers/net/wireless/mediatek/mt76/mt7925/mcu.c:645
mt7925_load_clc() error: buffer overflow 'phy->clc' 2 <= 2
在 2024/10/25 15:36, Dan Carpenter 写道:
> tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master
> head: 4e46774408d942efe4eb35dc62e5af3af71b9a30
> commit: 9679ca7326e52282cc923c4d71d81c999cb6cd55 wifi: mt76: mt7925: fix a potential array-index-out-of-bounds issue for clc
> config: parisc-randconfig-r071-20241024 (https://download.01.org/0day-ci/archive/20241025/202410250608.Ly4Aj2NI-lkp@intel.com/config)
> compiler: hppa-linux-gcc (GCC) 14.1.0
>
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@...el.com>
> | Reported-by: Dan Carpenter <dan.carpenter@...aro.org>
> | Closes: https://lore.kernel.org/r/202410250608.Ly4Aj2NI-lkp@intel.com/
>
> New smatch warnings:
> drivers/net/wireless/mediatek/mt76/mt7925/mcu.c:645 mt7925_load_clc() error: buffer overflow 'phy->clc' 2 <= 2
>
> Old smatch warnings:
> drivers/net/wireless/mediatek/mt76/mt7925/mcu.c:1158 mt7925_mcu_set_mlo_roc() warn: variable dereferenced before check 'mconf' (see line 1130)
>
> vim +645 drivers/net/wireless/mediatek/mt76/mt7925/mcu.c
>
> c948b5da6bbec7 Deren Wu 2023-09-18 589 static int mt7925_load_clc(struct mt792x_dev *dev, const char *fw_name)
> c948b5da6bbec7 Deren Wu 2023-09-18 590 {
> c948b5da6bbec7 Deren Wu 2023-09-18 591 const struct mt76_connac2_fw_trailer *hdr;
> c948b5da6bbec7 Deren Wu 2023-09-18 592 const struct mt76_connac2_fw_region *region;
> c948b5da6bbec7 Deren Wu 2023-09-18 593 const struct mt7925_clc *clc;
> c948b5da6bbec7 Deren Wu 2023-09-18 594 struct mt76_dev *mdev = &dev->mt76;
> c948b5da6bbec7 Deren Wu 2023-09-18 595 struct mt792x_phy *phy = &dev->phy;
> c948b5da6bbec7 Deren Wu 2023-09-18 596 const struct firmware *fw;
> c948b5da6bbec7 Deren Wu 2023-09-18 597 int ret, i, len, offset = 0;
> c948b5da6bbec7 Deren Wu 2023-09-18 598 u8 *clc_base = NULL;
> c948b5da6bbec7 Deren Wu 2023-09-18 599
> c948b5da6bbec7 Deren Wu 2023-09-18 600 if (mt7925_disable_clc ||
> c948b5da6bbec7 Deren Wu 2023-09-18 601 mt76_is_usb(&dev->mt76))
> c948b5da6bbec7 Deren Wu 2023-09-18 602 return 0;
> c948b5da6bbec7 Deren Wu 2023-09-18 603
> c948b5da6bbec7 Deren Wu 2023-09-18 604 ret = request_firmware(&fw, fw_name, mdev->dev);
> c948b5da6bbec7 Deren Wu 2023-09-18 605 if (ret)
> c948b5da6bbec7 Deren Wu 2023-09-18 606 return ret;
> c948b5da6bbec7 Deren Wu 2023-09-18 607
> c948b5da6bbec7 Deren Wu 2023-09-18 608 if (!fw || !fw->data || fw->size < sizeof(*hdr)) {
> c948b5da6bbec7 Deren Wu 2023-09-18 609 dev_err(mdev->dev, "Invalid firmware\n");
> c948b5da6bbec7 Deren Wu 2023-09-18 610 ret = -EINVAL;
> c948b5da6bbec7 Deren Wu 2023-09-18 611 goto out;
> c948b5da6bbec7 Deren Wu 2023-09-18 612 }
> c948b5da6bbec7 Deren Wu 2023-09-18 613
> c948b5da6bbec7 Deren Wu 2023-09-18 614 hdr = (const void *)(fw->data + fw->size - sizeof(*hdr));
> c948b5da6bbec7 Deren Wu 2023-09-18 615 for (i = 0; i < hdr->n_region; i++) {
> c948b5da6bbec7 Deren Wu 2023-09-18 616 region = (const void *)((const u8 *)hdr -
> c948b5da6bbec7 Deren Wu 2023-09-18 617 (hdr->n_region - i) * sizeof(*region));
> c948b5da6bbec7 Deren Wu 2023-09-18 618 len = le32_to_cpu(region->len);
> c948b5da6bbec7 Deren Wu 2023-09-18 619
> c948b5da6bbec7 Deren Wu 2023-09-18 620 /* check if we have valid buffer size */
> c948b5da6bbec7 Deren Wu 2023-09-18 621 if (offset + len > fw->size) {
> c948b5da6bbec7 Deren Wu 2023-09-18 622 dev_err(mdev->dev, "Invalid firmware region\n");
> c948b5da6bbec7 Deren Wu 2023-09-18 623 ret = -EINVAL;
> c948b5da6bbec7 Deren Wu 2023-09-18 624 goto out;
> c948b5da6bbec7 Deren Wu 2023-09-18 625 }
> c948b5da6bbec7 Deren Wu 2023-09-18 626
> c948b5da6bbec7 Deren Wu 2023-09-18 627 if ((region->feature_set & FW_FEATURE_NON_DL) &&
> c948b5da6bbec7 Deren Wu 2023-09-18 628 region->type == FW_TYPE_CLC) {
> c948b5da6bbec7 Deren Wu 2023-09-18 629 clc_base = (u8 *)(fw->data + offset);
> c948b5da6bbec7 Deren Wu 2023-09-18 630 break;
> c948b5da6bbec7 Deren Wu 2023-09-18 631 }
> c948b5da6bbec7 Deren Wu 2023-09-18 632 offset += len;
> c948b5da6bbec7 Deren Wu 2023-09-18 633 }
> c948b5da6bbec7 Deren Wu 2023-09-18 634
> c948b5da6bbec7 Deren Wu 2023-09-18 635 if (!clc_base)
> c948b5da6bbec7 Deren Wu 2023-09-18 636 goto out;
> c948b5da6bbec7 Deren Wu 2023-09-18 637
> c948b5da6bbec7 Deren Wu 2023-09-18 638 for (offset = 0; offset < len; offset += le32_to_cpu(clc->len)) {
> c948b5da6bbec7 Deren Wu 2023-09-18 639 clc = (const struct mt7925_clc *)(clc_base + offset);
> c948b5da6bbec7 Deren Wu 2023-09-18 640
> 9679ca7326e522 Ming Yen Hsieh 2024-08-19 641 if (clc->idx > ARRAY_SIZE(phy->clc))
>
> This should be >= instead of >.
>
> 9679ca7326e522 Ming Yen Hsieh 2024-08-19 642 break;
> 9679ca7326e522 Ming Yen Hsieh 2024-08-19 643
> c948b5da6bbec7 Deren Wu 2023-09-18 644 /* do not init buf again if chip reset triggered */
> c948b5da6bbec7 Deren Wu 2023-09-18 @645 if (phy->clc[clc->idx])
> c948b5da6bbec7 Deren Wu 2023-09-18 646 continue;
> c948b5da6bbec7 Deren Wu 2023-09-18 647
> c948b5da6bbec7 Deren Wu 2023-09-18 648 phy->clc[clc->idx] = devm_kmemdup(mdev->dev, clc,
> c948b5da6bbec7 Deren Wu 2023-09-18 649 le32_to_cpu(clc->len),
> c948b5da6bbec7 Deren Wu 2023-09-18 650 GFP_KERNEL);
> c948b5da6bbec7 Deren Wu 2023-09-18 651
> c948b5da6bbec7 Deren Wu 2023-09-18 652 if (!phy->clc[clc->idx]) {
> c948b5da6bbec7 Deren Wu 2023-09-18 653 ret = -ENOMEM;
> c948b5da6bbec7 Deren Wu 2023-09-18 654 goto out;
> c948b5da6bbec7 Deren Wu 2023-09-18 655 }
> c948b5da6bbec7 Deren Wu 2023-09-18 656 }
> c948b5da6bbec7 Deren Wu 2023-09-18 657
> c948b5da6bbec7 Deren Wu 2023-09-18 658 ret = mt7925_mcu_set_clc(dev, "00", ENVIRON_INDOOR);
> c948b5da6bbec7 Deren Wu 2023-09-18 659 out:
> c948b5da6bbec7 Deren Wu 2023-09-18 660 release_firmware(fw);
> c948b5da6bbec7 Deren Wu 2023-09-18 661
> c948b5da6bbec7 Deren Wu 2023-09-18 662 return ret;
> c948b5da6bbec7 Deren Wu 2023-09-18 663 }
>
Hi, Dan,
The issue is obvious here. Will send a fix soon.
Thanks,
Zekun
Powered by blists - more mailing lists