| Message ID | 1ec5982e-a68d-4837-af56-619e87a59741@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net] r8169: prevent potential deadlock in rtl8169_close | expand |
On 25.11.2023 18:25, Heiner Kallweit 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. > > Fixes: abe5fc42f9ce ("r8169: use RTNL to protect critical sections") > Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> > --- > drivers/net/ethernet/realtek/r8169_main.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c > index 0aed99a20..0ee3579ce 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); > I noticed there's a potential issue with my approach. So I have to rework this, please do not apply.
diff --git a/drivers/net/ethernet/realtek/r8169_main.c b/drivers/net/ethernet/realtek/r8169_main.c index 0aed99a20..0ee3579ce 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);
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. Fixes: abe5fc42f9ce ("r8169: use RTNL to protect critical sections") Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com> --- drivers/net/ethernet/realtek/r8169_main.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)