Re:Re: [PATCH net v3 6/7] net: ravb: release managed IRQs before freeing netdev

[jiale yao]

At 2026-10-03 17:59:21, "Niklas Söderlund" <niklas.soderlund@xxxxxxxxxxxx> wrote:
>Hi Jiale,
>
>On 2026-10-03 16:59:37 +0800, Jiale Yao wrote:
>> ravb_remove() frees the netdev before devres releases the managed IRQs.
>> The handlers use the netdev as their data pointer, so an interrupt during
>> that window can access freed memory. Probe error paths have the same
>> ordering problem.
>> 
>> Keep the netdev manually managed and place only the IRQ resources in a
>> dedicated devres group. Release the group after unregistering the netdev
>> and before freeing it, and release it on probe failures as well. This
>> keeps the existing runtime PM error handling unchanged.
>> 
>> This issue was found by a static analysis method used in our research.
>
>What happened to switching to use devm_alloc_etherdev_mqs() instead of 
>adding this complex thing, as we discussed in v2?

You're right. I missed this point while working through a large number
of patches...>_<
I should take a break and post an updated revision after the
24-hour waiting period.

>
>Nacked-by: Niklas Söderlund <niklas.soderlund+renesas@xxxxxxxxxxxx>
>
>> 
>> Fixes: 32f012b8c01c ("net: ravb: Move getting/requesting IRQs in the probe() method")
>> Cc: stable@xxxxxxxxxxxxxxx
>> Signed-off-by: Jiale Yao <yaojiale02@xxxxxxx>
>> ---
>>  drivers/net/ethernet/renesas/ravb_main.c | 18 ++++++++++++++----
>>  1 file changed, 14 insertions(+), 4 deletions(-)
>> 
>> diff --git a/drivers/net/ethernet/renesas/ravb_main.c b/drivers/net/ethernet/renesas/ravb_main.c
>> index ea1c7e536791..ab4703888778 100644
>> --- a/drivers/net/ethernet/renesas/ravb_main.c
>> +++ b/drivers/net/ethernet/renesas/ravb_main.c
>> @@ -2963,28 +2963,35 @@ static int ravb_probe(struct platform_device *pdev)
>>  		priv->num_rx_ring[RAVB_NC] = NC_RX_RING_SIZE;
>>  	}
>>  
>> +	if (!devres_open_group(&pdev->dev, priv, GFP_KERNEL)) {
>> +		error = -ENOMEM;
>> +		goto out_reset_assert;
>> +	}
>> +
>>  	error = ravb_setup_irqs(priv);
>>  	if (error)
>> -		goto out_reset_assert;
>> +		goto out_release_irq_group;
>> +
>> +	devres_close_group(&pdev->dev, priv);
>>  
>>  	priv->clk = devm_clk_get(&pdev->dev, NULL);
>>  	if (IS_ERR(priv->clk)) {
>>  		error = PTR_ERR(priv->clk);
>> -		goto out_reset_assert;
>> +		goto out_release_irq_group;
>>  	}
>>  
>>  	if (info->gptp_ref_clk) {
>>  		priv->gptp_clk = devm_clk_get(&pdev->dev, "gptp");
>>  		if (IS_ERR(priv->gptp_clk)) {
>>  			error = PTR_ERR(priv->gptp_clk);
>> -			goto out_reset_assert;
>> +			goto out_release_irq_group;
>>  		}
>>  	}
>>  
>>  	priv->refclk = devm_clk_get_optional(&pdev->dev, "refclk");
>>  	if (IS_ERR(priv->refclk)) {
>>  		error = PTR_ERR(priv->refclk);
>> -		goto out_reset_assert;
>> +		goto out_release_irq_group;
>>  	}
>>  	clk_prepare(priv->refclk);
>>  
>> @@ -3124,6 +3131,8 @@ static int ravb_probe(struct platform_device *pdev)
>>  	pm_runtime_disable(&pdev->dev);
>>  	pm_runtime_dont_use_autosuspend(&pdev->dev);
>>  	clk_unprepare(priv->refclk);
>> +out_release_irq_group:
>> +	devres_release_group(&pdev->dev, priv);
>>  out_reset_assert:
>>  	reset_control_assert(rstc);
>>  out_free_netdev:
>> @@ -3144,6 +3153,7 @@ static void ravb_remove(struct platform_device *pdev)
>>  		return;
>>  
>>  	unregister_netdev(ndev);
>> +	devres_release_group(dev, priv);
>>  	if (info->nc_queues)
>>  		netif_napi_del(&priv->napi[RAVB_NC]);
>>  	netif_napi_del(&priv->napi[RAVB_BE]);
>> -- 
>> 2.34.1
>> 
>
>-- 
>Kind Regards,
>Niklas Söderlund