drivers/net/ethernet/qualcomm/emac/emac.c | 6 ++++++ 1 file changed, 6 insertions(+)
In emac_probe, &adpt->work_thread is bound with
emac_work_thread. Then it will be started by timeout
handler emac_tx_timeout or a IRQ handler emac_isr.
If we remove the driver which will call emac_remove
to make cleanup, there may be a unfinished work.
The possible sequence is as follows:
Fix it by finishing the work before cleanup in the emac_remove
and disable timeout response.
CPU0 CPU1
|emac_work_thread
emac_remove |
free_netdev |
kfree(netdev); |
|emac_reinit_locked
|emac_mac_down
|//use netdev
Fixes: b9b17debc69d ("net: emac: emac gigabit ethernet controller driver")
Signed-off-by: Zheng Wang <zyytlz.wz@163.com>
---
v2:
- cancel the work after unregister_netdev suggested by Jakub
---
drivers/net/ethernet/qualcomm/emac/emac.c | 6 ++++++
1 file changed, 6 insertions(+)
diff --git a/drivers/net/ethernet/qualcomm/emac/emac.c b/drivers/net/ethernet/qualcomm/emac/emac.c
index 3115b2c12898..eaa50050aa0b 100644
--- a/drivers/net/ethernet/qualcomm/emac/emac.c
+++ b/drivers/net/ethernet/qualcomm/emac/emac.c
@@ -724,9 +724,15 @@ static int emac_remove(struct platform_device *pdev)
struct net_device *netdev = dev_get_drvdata(&pdev->dev);
struct emac_adapter *adpt = netdev_priv(netdev);
+ netif_carrier_off(netdev);
+ netif_tx_disable(netdev);
+
unregister_netdev(netdev);
netif_napi_del(&adpt->rx_q.napi);
+ free_irq(adpt->irq.irq, &adpt->irq);
+ cancel_work_sync(&adpt->work_thread);
+
emac_clks_teardown(adpt);
put_device(&adpt->phydev->mdio.dev);
--
2.25.1
Hello: This patch was applied to netdev/net.git (main) by David S. Miller <davem@davemloft.net>: On Sat, 18 Mar 2023 16:05:26 +0800 you wrote: > In emac_probe, &adpt->work_thread is bound with > emac_work_thread. Then it will be started by timeout > handler emac_tx_timeout or a IRQ handler emac_isr. > > If we remove the driver which will call emac_remove > to make cleanup, there may be a unfinished work. > > [...] Here is the summary with links: - [net,v2] net: qcom/emac: Fix use after free bug in emac_remove due to race condition https://git.kernel.org/netdev/net/c/6b6bc5b8bd2d You are awesome, thank you! -- Deet-doot-dot, I am a bot. https://korg.docs.kernel.org/patchwork/pwbot.html
On Mon, 20 Mar 2023 09:20:17 +0000 patchwork-bot+netdevbpf@kernel.org wrote: > Here is the summary with links: > - [net,v2] net: qcom/emac: Fix use after free bug in emac_remove due to race condition > https://git.kernel.org/netdev/net/c/6b6bc5b8bd2d Don't think this is correct FWIW, randomly shutting things down without holding any locks and before unregister_netdev() is called has got to be racy. Oh, eh.
Jakub Kicinski <kuba@kernel.org> 于2023年3月21日周二 03:23写道: > > On Mon, 20 Mar 2023 09:20:17 +0000 patchwork-bot+netdevbpf@kernel.org > wrote: > > Here is the summary with links: > > - [net,v2] net: qcom/emac: Fix use after free bug in emac_remove due to race condition > > https://git.kernel.org/netdev/net/c/6b6bc5b8bd2d > > Don't think this is correct FWIW, randomly shutting things down without > holding any locks and before unregister_netdev() is called has got to > be racy. Oh, eh. Dear Jakubju, Sorry for my late reply. I had a busy week. I have taken a look at similar fixes implemented in other drivers, but I do think your advice is more precious for I'm not familiar with the driver. Based on your experience and expertise, what do you think would be the most effective solution to address the race condition issue that you have identified in the emac_remove function of the qcom/emac driver? I appreciate any insights or suggestions that you might have on this matter. Thank you for your time and help. Best regards, Zheng Wang
© 2016 - 2024 Red Hat, Inc.