* [PATCH net] net: fec: Better handle pm_runtime_get() failing in .remove()
@ 2023-05-10 20:00 Uwe Kleine-König
2023-05-10 21:41 ` Andrew Lunn
2023-05-12 1:20 ` patchwork-bot+netdevbpf
0 siblings, 2 replies; 3+ messages in thread
From: Uwe Kleine-König @ 2023-05-10 20:00 UTC (permalink / raw)
To: Wei Fang, Shenwei Wang, Clark Wang, NXP Linux Team,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni
Cc: Fugang Duan, Chuhong Yuan, netdev, kernel
In the (unlikely) event that pm_runtime_get() (disguised as
pm_runtime_resume_and_get()) fails, the remove callback returned an
error early. The problem with this is that the driver core ignores the
error value and continues removing the device. This results in a
resource leak. Worse the devm allocated resources are freed and so if a
callback of the driver is called later the register mapping is already
gone which probably results in a crash.
Fixes: a31eda65ba21 ("net: fec: fix clock count mis-match")
Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix•de>
---
drivers/net/ethernet/freescale/fec_main.c | 15 +++++++++++----
1 file changed, 11 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/freescale/fec_main.c b/drivers/net/ethernet/freescale/fec_main.c
index 42ec6ca3bf03..241df41d500f 100644
--- a/drivers/net/ethernet/freescale/fec_main.c
+++ b/drivers/net/ethernet/freescale/fec_main.c
@@ -4478,9 +4478,11 @@ fec_drv_remove(struct platform_device *pdev)
struct device_node *np = pdev->dev.of_node;
int ret;
- ret = pm_runtime_resume_and_get(&pdev->dev);
+ ret = pm_runtime_get_sync(&pdev->dev);
if (ret < 0)
- return ret;
+ dev_err(&pdev->dev,
+ "Failed to resume device in remove callback (%pe)\n",
+ ERR_PTR(ret));
cancel_work_sync(&fep->tx_timeout_work);
fec_ptp_stop(pdev);
@@ -4493,8 +4495,13 @@ fec_drv_remove(struct platform_device *pdev)
of_phy_deregister_fixed_link(np);
of_node_put(fep->phy_node);
- clk_disable_unprepare(fep->clk_ahb);
- clk_disable_unprepare(fep->clk_ipg);
+ /* After pm_runtime_get_sync() failed, the clks are still off, so skip
+ * disabling them again.
+ */
+ if (ret >= 0) {
+ clk_disable_unprepare(fep->clk_ahb);
+ clk_disable_unprepare(fep->clk_ipg);
+ }
pm_runtime_put_noidle(&pdev->dev);
pm_runtime_disable(&pdev->dev);
base-commit: ac9a78681b921877518763ba0e89202254349d1b
--
2.39.2
^ permalink raw reply related [flat|nested] 3+ messages in thread* Re: [PATCH net] net: fec: Better handle pm_runtime_get() failing in .remove()
2023-05-10 20:00 [PATCH net] net: fec: Better handle pm_runtime_get() failing in .remove() Uwe Kleine-König
@ 2023-05-10 21:41 ` Andrew Lunn
2023-05-12 1:20 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: Andrew Lunn @ 2023-05-10 21:41 UTC (permalink / raw)
To: Uwe Kleine-König
Cc: Wei Fang, Shenwei Wang, Clark Wang, NXP Linux Team,
David S. Miller, Eric Dumazet, Jakub Kicinski, Paolo Abeni,
Fugang Duan, Chuhong Yuan, netdev, kernel
On Wed, May 10, 2023 at 10:00:20PM +0200, Uwe Kleine-König wrote:
> In the (unlikely) event that pm_runtime_get() (disguised as
> pm_runtime_resume_and_get()) fails, the remove callback returned an
> error early. The problem with this is that the driver core ignores the
> error value and continues removing the device. This results in a
> resource leak. Worse the devm allocated resources are freed and so if a
> callback of the driver is called later the register mapping is already
> gone which probably results in a crash.
>
> Fixes: a31eda65ba21 ("net: fec: fix clock count mis-match")
> Signed-off-by: Uwe Kleine-König <u.kleine-koenig@pengutronix•de>
Reviewed-by: Andrew Lunn <andrew@lunn•ch>
Andrew
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH net] net: fec: Better handle pm_runtime_get() failing in .remove()
2023-05-10 20:00 [PATCH net] net: fec: Better handle pm_runtime_get() failing in .remove() Uwe Kleine-König
2023-05-10 21:41 ` Andrew Lunn
@ 2023-05-12 1:20 ` patchwork-bot+netdevbpf
1 sibling, 0 replies; 3+ messages in thread
From: patchwork-bot+netdevbpf @ 2023-05-12 1:20 UTC (permalink / raw)
To: =?utf-8?q?Uwe_Kleine-K=C3=B6nig_=3Cu=2Ekleine-koenig=40pengutronix=2Ede=3E?=
Cc: wei.fang, shenwei.wang, xiaoning.wang, linux-imx, davem, edumazet,
kuba, pabeni, fugang.duan, hslester96, netdev, kernel
Hello:
This patch was applied to netdev/net.git (main)
by Jakub Kicinski <kuba@kernel•org>:
On Wed, 10 May 2023 22:00:20 +0200 you wrote:
> In the (unlikely) event that pm_runtime_get() (disguised as
> pm_runtime_resume_and_get()) fails, the remove callback returned an
> error early. The problem with this is that the driver core ignores the
> error value and continues removing the device. This results in a
> resource leak. Worse the devm allocated resources are freed and so if a
> callback of the driver is called later the register mapping is already
> gone which probably results in a crash.
>
> [...]
Here is the summary with links:
- [net] net: fec: Better handle pm_runtime_get() failing in .remove()
https://git.kernel.org/netdev/net/c/f816b9829b19
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2023-05-12 1:20 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2023-05-10 20:00 [PATCH net] net: fec: Better handle pm_runtime_get() failing in .remove() Uwe Kleine-König
2023-05-10 21:41 ` Andrew Lunn
2023-05-12 1:20 ` patchwork-bot+netdevbpf
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox