[PATCH v2] wifi: mt76: mt7921: fix resource leak in probe error path

[PATCH v2] wifi: mt76: mt7921: fix resource leak in probe error path

[Hongling Zeng]

When pcim_iomap_region() or devm_kmemdup() fail, the code returns
directly without cleaning up previously allocated resources:
  - mt76_device allocated by mt76_alloc_device()
  - pci irq vectors allocated by pci_alloc_irq_vectors()
Fix this by jumping to the existing error cleanup path instead of
returning directly.

To avoid using an uninitialized variable in the error path, move the
dev initialization before the error checks.

Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>

---
 Change in v1
   --fix uninitialized variable warning
---
 drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 15 ++++++++++-----
 1 file changed, 10 insertions(+), 5 deletions(-)

diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
index 7a790ddf43bb..49a37185f056 100644
--- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
+++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
@@ -343,11 +343,14 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
 
 	pci_set_drvdata(pdev, mdev);
 
+	dev = container_of(mdev, struct mt792x_dev, mt76);
+
 	regs =  pcim_iomap_region(pdev, 0, pci_name(pdev));
-	if (IS_ERR(regs))
-		return PTR_ERR(regs);
+	if (IS_ERR(regs)) {
+		ret = PTR_ERR(regs);
+		goto err_free_dev;
+	}
 
-	dev = container_of(mdev, struct mt792x_dev, mt76);
 	dev->fw_features = features;
 	dev->hif_ops = &mt7921_pcie_ops;
 	dev->irq_map = &irq_map;
@@ -359,8 +362,10 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
 		/* MT7902 needs a mutable copy because wm2_complete_mask differs */
 		map = devm_kmemdup(&pdev->dev, &irq_map,
 				   sizeof(irq_map), GFP_KERNEL);
-		if (!map)
-			return -ENOMEM;
+		if (!map) {
+			ret = -ENOMEM;
+			goto err_free_dev;
+		}
 
 		map->rx.wm2_complete_mask = 0;
 		dev->irq_map = map;
-- 
2.25.1

Re: [PATCH v2] wifi: mt76: mt7921: fix resource leak in probe error path

[Sean Wang]

Hi,

On Tue, May 26, 2026 at 10:44 PM Hongling Zeng <zenghongling@kylinos.cn> wrote:
>
> When pcim_iomap_region() or devm_kmemdup() fail, the code returns
> directly without cleaning up previously allocated resources:
>   - mt76_device allocated by mt76_alloc_device()
>   - pci irq vectors allocated by pci_alloc_irq_vectors()
> Fix this by jumping to the existing error cleanup path instead of
> returning directly.
>
> To avoid using an uninitialized variable in the error path, move the
> dev initialization before the error checks.
>
> Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>

The code change itself looks correct, but commit message Fixes: tag is wrong.

It points to 234738ea3390 ("phy: ti-pipe3: move clk initialization to
a separate function"), which is unrelated to mt76/mt7921.

The two direct-return leaks appear to come from:
      - ee5bb35d2b83 ("wifi: mt76: mt7921: Replace deprecated PCI
function") for the pcim_iomap_region() path.
      - 222606f43b58 ("wifi: mt76: mt7921: handle MT7902 irq_map quirk
with mutable copy") for the MT7902 devm_kmemdup() path.

> ---
>  Change in v1
>    --fix uninitialized variable warning
> ---
>  drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> index 7a790ddf43bb..49a37185f056 100644
> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
> @@ -343,11 +343,14 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
>
>         pci_set_drvdata(pdev, mdev);
>
> +       dev = container_of(mdev, struct mt792x_dev, mt76);
> +
>         regs =  pcim_iomap_region(pdev, 0, pci_name(pdev));
> -       if (IS_ERR(regs))
> -               return PTR_ERR(regs);
> +       if (IS_ERR(regs)) {
> +               ret = PTR_ERR(regs);
> +               goto err_free_dev;
> +       }
>
> -       dev = container_of(mdev, struct mt792x_dev, mt76);
>         dev->fw_features = features;
>         dev->hif_ops = &mt7921_pcie_ops;
>         dev->irq_map = &irq_map;
> @@ -359,8 +362,10 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
>                 /* MT7902 needs a mutable copy because wm2_complete_mask differs */
>                 map = devm_kmemdup(&pdev->dev, &irq_map,
>                                    sizeof(irq_map), GFP_KERNEL);
> -               if (!map)
> -                       return -ENOMEM;
> +               if (!map) {
> +                       ret = -ENOMEM;
> +                       goto err_free_dev;
> +               }
>
>                 map->rx.wm2_complete_mask = 0;
>                 dev->irq_map = map;
> --
> 2.25.1
>
>

Re: [PATCH v2] wifi: mt76: mt7921: fix resource leak in probe error path

[Hongling Zeng]

   Hi Sean,

   Thanks for the review! You're absolutely right about the incorrect 
Fixes: tag.

   Here's v3 with the correct Fixes tags pointing to the actual commits that
   introduced the resource leaks:

   - ee5bb35d2b83 introduced the pcim_iomap_region() direct return
   - 222606f43b58 introduced the devm_kmemdup() direct return

   Thanks again for catching that.

   Regards,
   Hongling


在 2026年05月27日 12:48, Sean Wang 写道:
> Hi,
>
> On Tue, May 26, 2026 at 10:44 PM Hongling Zeng <zenghongling@kylinos.cn> wrote:
>> When pcim_iomap_region() or devm_kmemdup() fail, the code returns
>> directly without cleaning up previously allocated resources:
>>    - mt76_device allocated by mt76_alloc_device()
>>    - pci irq vectors allocated by pci_alloc_irq_vectors()
>> Fix this by jumping to the existing error cleanup path instead of
>> returning directly.
>>
>> To avoid using an uninitialized variable in the error path, move the
>> dev initialization before the error checks.
>>
>> Fixes: 234738ea3390 ("phy: ti-pipe3: move clk initialization to a separate function")
>> Signed-off-by: Hongling Zeng <zenghongling@kylinos.cn>
>>
> The code change itself looks correct, but commit message Fixes: tag is wrong.
>
> It points to 234738ea3390 ("phy: ti-pipe3: move clk initialization to
> a separate function"), which is unrelated to mt76/mt7921.
>
> The two direct-return leaks appear to come from:
>        - ee5bb35d2b83 ("wifi: mt76: mt7921: Replace deprecated PCI
> function") for the pcim_iomap_region() path.
>        - 222606f43b58 ("wifi: mt76: mt7921: handle MT7902 irq_map quirk
> with mutable copy") for the MT7902 devm_kmemdup() path.
>
>> ---
>>   Change in v1
>>     --fix uninitialized variable warning
>> ---
>>   drivers/net/wireless/mediatek/mt76/mt7921/pci.c | 15 ++++++++++-----
>>   1 file changed, 10 insertions(+), 5 deletions(-)
>>
>> diff --git a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
>> index 7a790ddf43bb..49a37185f056 100644
>> --- a/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
>> +++ b/drivers/net/wireless/mediatek/mt76/mt7921/pci.c
>> @@ -343,11 +343,14 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
>>
>>          pci_set_drvdata(pdev, mdev);
>>
>> +       dev = container_of(mdev, struct mt792x_dev, mt76);
>> +
>>          regs =  pcim_iomap_region(pdev, 0, pci_name(pdev));
>> -       if (IS_ERR(regs))
>> -               return PTR_ERR(regs);
>> +       if (IS_ERR(regs)) {
>> +               ret = PTR_ERR(regs);
>> +               goto err_free_dev;
>> +       }
>>
>> -       dev = container_of(mdev, struct mt792x_dev, mt76);
>>          dev->fw_features = features;
>>          dev->hif_ops = &mt7921_pcie_ops;
>>          dev->irq_map = &irq_map;
>> @@ -359,8 +362,10 @@ static int mt7921_pci_probe(struct pci_dev *pdev,
>>                  /* MT7902 needs a mutable copy because wm2_complete_mask differs */
>>                  map = devm_kmemdup(&pdev->dev, &irq_map,
>>                                     sizeof(irq_map), GFP_KERNEL);
>> -               if (!map)
>> -                       return -ENOMEM;
>> +               if (!map) {
>> +                       ret = -ENOMEM;
>> +                       goto err_free_dev;
>> +               }
>>
>>                  map->rx.wm2_complete_mask = 0;
>>                  dev->irq_map = map;
>> --
>> 2.25.1
>>
>>