netdev - Re: [PATCH] net: stmmac: call stmmac_remove_config_dt() in error paths in stmmac_probe_config

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] [thread-next>] [day] [month] [year] [list]

Message-ID: <a9c9ed92-e5fb-4c5a-b8a8-91f5a2e1bea4@pf.is.s.u-tokyo.ac.jp>
Date: Wed, 18 Dec 2024 12:24:37 +0900
From: Joe Hattori <joe@...is.s.u-tokyo.ac.jp>
To: Krzysztof Kozlowski <krzk@...nel.org>, alexandre.torgue@...s.st.com,
 joabreu@...opsys.com, andrew+netdev@...n.ch, davem@...emloft.net,
 edumazet@...gle.com, kuba@...nel.org, pabeni@...hat.com,
 mcoquelin.stm32@...il.com
Cc: netdev@...r.kernel.org
Subject: Re: [PATCH] net: stmmac: call stmmac_remove_config_dt() in error
 paths in stmmac_probe_config_dt()



On 12/17/24 19:58, Krzysztof Kozlowski wrote:
> On 17/12/2024 04:32, Joe Hattori wrote:
>> Current implementation of stmmac_probe_config_dt() does not release the
>> OF node reference obtained by of_parse_phandle() when stmmac_dt_phy()
>> fails, thus call stmmac_remove_config_dt(). The error_hw_init and
>> error_pclk_get labels can be removed as just calling
>> stmmac_remove_config_dt() suffices. Also, remove the first argument of
>> stmmac_remove_config_dt() as it is not used.
>>
>> This bug was found by an experimental static analysis tool that I am
>> developing.
>>
>> Fixes: 4838a5405028 ("net: stmmac: Fix wrapper drivers not detecting PHY")
>> Signed-off-by: Joe Hattori <joe@...is.s.u-tokyo.ac.jp>
>> ---
>>   .../ethernet/stmicro/stmmac/stmmac_platform.c | 37 +++++++------------
>>   1 file changed, 13 insertions(+), 24 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> index 3ac32444e492..4f1a9f7aae6e 100644
>> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_platform.c
>> @@ -407,13 +407,11 @@ static int stmmac_of_get_mac_mode(struct device_node *np)
>>   
>>   /**
>>    * stmmac_remove_config_dt - undo the effects of stmmac_probe_config_dt()
>> - * @pdev: platform_device structure
>>    * @plat: driver data platform structure
>>    *
>>    * Release resources claimed by stmmac_probe_config_dt().
>>    */
>> -static void stmmac_remove_config_dt(struct platform_device *pdev,
>> -				    struct plat_stmmacenet_data *plat)
>> +static void stmmac_remove_config_dt(struct plat_stmmacenet_data *plat)
>>   {
>>   	clk_disable_unprepare(plat->stmmac_clk);
>>   	clk_disable_unprepare(plat->pclk);
>> @@ -436,7 +434,6 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>>   	struct plat_stmmacenet_data *plat;
>>   	struct stmmac_dma_cfg *dma_cfg;
>>   	int phy_mode;
>> -	void *ret;
>>   	int rc;
>>   
>>   	plat = devm_kzalloc(&pdev->dev, sizeof(*plat), GFP_KERNEL);
>> @@ -490,8 +487,10 @@ stmmac_probe_config_dt(struct platform_device *pdev, u8 *mac)
>>   		dev_warn(&pdev->dev, "snps,phy-addr property is deprecated\n");
>>   
>>   	rc = stmmac_mdio_setup(plat, np, &pdev->dev);
>> -	if (rc)
>> +	if (rc) {
>> +		stmmac_remove_config_dt(plat);
>>   		return ERR_PTR(rc);
> You now double unprepare the clocks. This is either buggy or just
> confusing (assuming clocks are NULL).

Confusing indeed. Replaced with of_node_put() in the v2 patch.

> 
> Best regards,
> Krzysztof

Best,
Joe