[PATCH v2 net] r8169: prevent potential deadlock in rtl8169_close

[PATCH v2 net] r8169: prevent potential deadlock in rtl8169_close

[Heiner Kallweit]

ndo_stop() is RTNL-protected by net core, and the worker function takes
RTNL as well. Therefore we will deadlock when trying to execute a
pending work synchronously. To fix this execute any pending work
asynchronously. This will do no harm because netif_running() is false
in ndo_stop(), and therefore the work function is effectively a no-op.
However we have to ensure that no task is running or pending after
rtl_remove_one(), therefore add a call to cancel_work_sync().

Fixes: abe5fc42f9ce ("r8169: use RTNL to protect critical sections")
Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
---
v2:
- add call to cancel_work_sync() in rtl_remove_one()
---
 drivers/net/ethernet/realtek/r8169_main.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c
index 0aed99a20..dd04c6358 100644
--- a/drivers/net/ethernet/realtek/r8169_main.c
+++ b/drivers/net/ethernet/realtek/r8169_main.c
@@ -4603,7 +4603,7 @@ static int rtl8169_close(struct net_device *dev)
 	rtl8169_down(tp);
 	rtl8169_rx_clear(tp);
 
-	cancel_work_sync(&tp->wk.work);
+	cancel_work(&tp->wk.work);
 
 	free_irq(tp->irq, tp);
 
@@ -4837,6 +4837,8 @@ static void rtl_remove_one(struct pci_dev *pdev)
 	if (pci_dev_run_wake(pdev))
 		pm_runtime_get_noresume(&pdev->dev);
 
+	cancel_work_sync(&tp->wk.work);
+
 	unregister_netdev(tp->dev);
 
 	if (tp->dash_type != RTL_DASH_NONE)
-- 
2.43.0

Re: [PATCH v2 net] r8169: prevent potential deadlock in rtl8169_close

[patchwork-bot+netdevbpf]

Hello:

This patch was applied to netdev/net.git (main)
by Paolo Abeni <pabeni@redhat.com>:

On Sun, 26 Nov 2023 23:01:02 +0100 you wrote:
> ndo_stop() is RTNL-protected by net core, and the worker function takes
> RTNL as well. Therefore we will deadlock when trying to execute a
> pending work synchronously. To fix this execute any pending work
> asynchronously. This will do no harm because netif_running() is false
> in ndo_stop(), and therefore the work function is effectively a no-op.
> However we have to ensure that no task is running or pending after
> rtl_remove_one(), therefore add a call to cancel_work_sync().
> 
> [...]

Here is the summary with links:
  - [v2,net] r8169: prevent potential deadlock in rtl8169_close
    https://git.kernel.org/netdev/net/c/91d3d149978b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html