[PATCH net] net: pxa168_eth: quiesce the interface before releasing resources

[Myeonghun Pak]

Canceling tx_timeout_task at the start of remove does not prevent the
still-live netdev watchdog from queuing it again. The recovery worker
can then stop and reopen the interface while remove releases its
resources.

For an up interface, phy_disconnect() clears dev->phydev before
unregister_netdev() invokes ndo_stop(), which then calls phy_stop()
with a NULL pointer.

Disable and drain the recovery work first so subsequent watchdog queue
attempts are rejected. Close the interface while holding RTNL and before
disconnecting the PHY: ndo_stop calls phy_stop() and must stop DMA
before the coherent hash table is freed. Keep RTNL through
unregistration so the interface cannot be reopened in between.

Disconnect the PHY before unregistering the netdev, while both devices'
sysfs directories still exist. Release the MDIO bus and hash table only
after the interface has been unregistered.

This issue was identified as part of our ongoing research while
reviewing kernel code.

Fixes: bd70957438f0 ("net: pxa168_eth: Fix a potential data race in pxa168_eth_remove")
Co-developed-by: Ijae Kim <ae878000@xxxxxxxxx>
Signed-off-by: Ijae Kim <ae878000@xxxxxxxxx>
Signed-off-by: Myeonghun Pak <mhun512@xxxxxxxxx>

---
Compile-tested with Clang, W=1. No hardware runtime testing.

Based on mainline 0fcb05b72344f40806cbe810de8872f1698bf053.

diff --git a/drivers/net/ethernet/marvell/pxa168_eth.c b/drivers/net/ethernet/marvell/pxa168_eth.c
index bbbbbba..2d59491 100644
--- a/drivers/net/ethernet/marvell/pxa168_eth.c
+++ b/drivers/net/ethernet/marvell/pxa168_eth.c
@@ -27,6 +27,7 @@
 #include <linux/phy.h>
 #include <linux/platform_device.h>
 #include <linux/pxa168_eth.h>
+#include <linux/rtnetlink.h>
 #include <linux/tcp.h>
 #include <linux/types.h>
 #include <linux/udp.h>
@@ -1528,18 +1529,21 @@ static void pxa168_eth_remove(struct platform_device *pdev)
 	struct net_device *dev = platform_get_drvdata(pdev);
 	struct pxa168_eth_private *pep = netdev_priv(dev);
 
-	cancel_work_sync(&pep->tx_timeout_task);
+	disable_work_sync(&pep->tx_timeout_task);
+	rtnl_lock();
+	dev_close(dev);
+	if (dev->phydev)
+		phy_disconnect(dev->phydev);
+	unregister_netdevice(dev);
+	rtnl_unlock();
+
 	if (pep->htpr) {
 		dma_free_coherent(pep->dev->dev.parent, HASH_ADDR_TABLE_SIZE,
 				  pep->htpr, pep->htpr_dma);
 		pep->htpr = NULL;
 	}
-	if (dev->phydev)
-		phy_disconnect(dev->phydev);
-
 	mdiobus_unregister(pep->smi_bus);
 	mdiobus_free(pep->smi_bus);
-	unregister_netdev(dev);
 	free_netdev(dev);
 }