Re: [PATCH v3] net: fec: fix pinctrl default state restore order on resume

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 4 potential issue(s) to consider:

Pre-existing issues:
- [High] The return value of `pm_runtime_force_resume()` is ignored in `fec_resume()`, potentially leading to a synchronous external abort.
- [High] Unbalanced `regulator_disable()` in the `fec_resume()` error path when Wake-on-LAN (WoL) is enabled.
- [High] Race condition in `fec_drv_remove()` with `tx_timeout_work` leading to a Use-After-Free.
- [High] The error path in `fec_resume()` fails to revert the effects of `pm_runtime_force_resume()`, leaking clocks and PM state.
--

commit c65b833021159f6537105f2b80073f286520673a
Author: Tapio Reijonen <tapio.reijonen@vaisala.com>

net: fec: fix pinctrl default state restore order on resume

This commit changes the order in fec_resume() so that the default pinctrl
state is restored before enabling the clocks. This fixes an issue where PHYs
were not receiving a hardware reset pulse after clock enable because the
GPIO pin was still in sleep state.

> diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
> index f89aa94ce0202..6ebde65d7f1b8 100644
> --- a/drivers/net/ethernet/freescale/fec_main.c
> +++ b/drivers/net/ethernet/freescale/fec_main.c
> @@ -5594,6 +5594,7 @@ static int fec_resume(struct device *dev)
>  		if (fep->rpm_active)
>  			pm_runtime_force_resume(dev);

[Severity: High]
This is a pre-existing issue, but the return value of
pm_runtime_force_resume() is ignored here. If this call fails (for example,
due to clock enablement failure), the bus clocks will remain disabled.

When fec_restart() is subsequently called, it directly accesses memory-mapped
hardware registers. Can accessing these registers without the bus clocks
enabled result in a synchronous external abort and a kernel panic?

> [ ... ]

While reviewing this code, I also noticed a few other pre-existing issues
related to the error path in fec_resume():

drivers/net/ethernet/freescale/fec_main.c:fec_resume() {
    ...
	if (fep->reg_phy && !(fep->wol_flag & FEC_WOL_FLAG_ENABLE)) {
		ret = regulator_enable(fep->reg_phy);
        ...
failed_clk:
	if (fep->reg_phy)
		regulator_disable(fep->reg_phy);
	return ret;
}

[Severity: High]
This isn't a bug introduced by this patch, but the error path appears to
have an unbalanced regulator_disable() call when Wake-on-LAN is enabled.

At the start of fec_resume(), the PHY regulator is only enabled if WoL is
NOT enabled. If a subsequent step like fec_enet_clk_enable() fails, the
code jumps to this failed_clk label and unconditionally disables the
regulator.

Does this corrupt the regulator's use count and potentially power off the
PHY unexpectedly?

[Severity: High]
This is another pre-existing issue in this same error path. If
pm_runtime_force_resume() succeeded earlier in the function, it
transitioned the device to RPM_ACTIVE and enabled the clocks.

If fec_enet_clk_enable() then fails, we return an error without calling
pm_runtime_force_suspend() or otherwise reverting the runtime PM state.

Does this leave the device stuck in an inconsistent active PM state with
leaked AHB/IPG clocks?


I also noticed an unrelated pre-existing issue in fec_drv_remove():

drivers/net/ethernet/freescale/fec_main.c:fec_drv_remove() {
    ...
	cancel_work_sync(&fep->tx_timeout_work);
	fec_ptp_stop(pdev);
	unregister_netdev(ndev);
    ...
}

[Severity: High]
This is a pre-existing issue, but calling cancel_work_sync() before
unregister_netdev() seems to create a race condition.

If a transmit timeout occurs immediately after this call but before
unregister_netdev() finishes taking the interface down, the networking
stack's watchdog timer will call the driver's ndo_tx_timeout callback,
which re-arms the work via schedule_work().

When the scheduled work eventually executes, it will access the freed
structures. Could this result in a Use-After-Free crash?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260529-b4-fec-resume-pinctrl-order-v3-1-56bb52b80ebf@vaisala.com?part=1