| Message ID | 12395867-1d17-4cac-aa7d-c691938fcddf@gmail.com (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | 91d3d149978ba7b238198dd80e4b823756aa7cfa |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [v2,net] r8169: prevent potential deadlock in rtl8169_close | expand |
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!
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)
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(-)