netdev - Re: [PATCH net 1/3] net: ethernet: mtk_eth_soc: fix possible memory leak in mtk

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Date:   Mon, 17 Oct 2022 11:55:00 +0200
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Yang Yingliang <yangyingliang@...wei.com>,
        linux-mediatek@...ts.infradead.org, netdev@...r.kernel.org
Cc:     nbd@....name, davem@...emloft.net
Subject: Re: [PATCH net 1/3] net: ethernet: mtk_eth_soc: fix possible memory
 leak in mtk_probe()

Il 17/10/22 05:51, Yang Yingliang ha scritto:
> If mtk_wed_add_hw() has been called, mtk_wed_exit() needs be called
> in error path or removing module to free the memory allocated in
> mtk_wed_add_hw().
> 
> Fixes: 804775dfc288 ("net: ethernet: mtk_eth_soc: add support for Wireless Ethernet Dispatch (WED)")
> Signed-off-by: Yang Yingliang <yangyingliang@...wei.com>
> ---
>   drivers/net/ethernet/mediatek/mtk_eth_soc.c | 17 ++++++++++++-----
>   1 file changed, 12 insertions(+), 5 deletions(-)
> 
> diff --git a/drivers/net/ethernet/mediatek/mtk_eth_soc.c b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> index 4fba7cb0144b..7cd381530aa4 100644
> --- a/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> +++ b/drivers/net/ethernet/mediatek/mtk_eth_soc.c
> @@ -4060,19 +4060,23 @@ static int mtk_probe(struct platform_device *pdev)
>   			eth->irq[i] = platform_get_irq(pdev, i);
>   		if (eth->irq[i] < 0) {
>   			dev_err(&pdev->dev, "no IRQ%d resource found\n", i);
> -			return -ENXIO;
> +			err = -ENXIO;
> +			goto err_wed_exit;
>   		}
>   	}
>   	for (i = 0; i < ARRAY_SIZE(eth->clks); i++) {
>   		eth->clks[i] = devm_clk_get(eth->dev,
>   					    mtk_clks_source_name[i]);
>   		if (IS_ERR(eth->clks[i])) {
> -			if (PTR_ERR(eth->clks[i]) == -EPROBE_DEFER)
> -				return -EPROBE_DEFER;
> +			if (PTR_ERR(eth->clks[i]) == -EPROBE_DEFER) {
> +				err = -EPROBE_DEFER;
> +				goto err_wed_exit;
> +			}
>   			if (eth->soc->required_clks & BIT(i)) {
>   				dev_err(&pdev->dev, "clock %s not found\n",
>   					mtk_clks_source_name[i]);
> -				return -EINVAL;
> +				err = -EINVAL;
> +				goto err_wed_exit;
>   			}
>   			eth->clks[i] = NULL;
>   		}
> @@ -4083,7 +4087,7 @@ static int mtk_probe(struct platform_device *pdev)
>   
>   	err = mtk_hw_init(eth);
>   	if (err)
> -		return err;
> +		goto err_wed_exit;
>   
>   	eth->hwlro = MTK_HAS_CAPS(eth->soc->caps, MTK_HWLRO);
>   
> @@ -4179,6 +4183,8 @@ static int mtk_probe(struct platform_device *pdev)
>   	mtk_free_dev(eth);
>   err_deinit_hw:
>   	mtk_hw_deinit(eth);
> +err_wed_exit:
> +	mtk_wed_exit();

mtk_wed_add_hw() happens *only* if eth->soc->offload_version == true.
To make this clean, mtk_wed_exit() should be called only if mtk_wed_add_hw()
was ever called, so you have to add a check here.


>   
>   	return err;
>   }
> @@ -4198,6 +4204,7 @@ static int mtk_remove(struct platform_device *pdev)
>   		phylink_disconnect_phy(mac->phylink);
>   	}
>   
> +	mtk_wed_exit();

...and here as well.

Regards,
Angelo

>   	mtk_hw_deinit(eth);
>   
>   	netif_napi_del(&eth->tx_napi);