[PATCH net] net: hns: retire service work before releasing device resources

[PATCH net] net: hns: retire service work before releasing device resources

[Myeonghun Pak]

The remove path releases ring, PHY and accelerator resources before
canceling service work, allowing use-after-free. A pending worker can
also reinitialize the device or publish linkwatch events after
unregister.

Set REMOVING, synchronize an in-flight TX timeout, then shut down the
service timer and drain work before unregister invokes ndo_stop.
Serialize reset and link-update state checks and operations with
unregister under RTNL.

Compile-tested with Clang 22.1.2 (W=1); not tested on hardware.

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

Fixes: b5996f11ea54 ("net: add Hisilicon Network Subsystem basic ethernet support")
Assisted-by: LLM
Co-developed-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Ijae Kim <ae878000@gmail.com>
Signed-off-by: Myeonghun Pak <mhun512@gmail.com>
---
 drivers/net/ethernet/hisilicon/hns/hns_enet.c | 37 ++++++++++++++-----
 1 file changed, 27 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/hisilicon/hns/hns_enet.c b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
index e905f10b894e..977f054b00c3 100644
--- a/drivers/net/ethernet/hisilicon/hns/hns_enet.c
+++ b/drivers/net/ethernet/hisilicon/hns/hns_enet.c
@@ -2024,21 +2024,22 @@ static void hns_nic_reset_subtask(struct hns_nic_priv *priv)
 		return;
 	clear_bit(NIC_STATE2_RESET_REQUESTED, &priv->state);
 
-	/* If we're already down, removing or resetting, just bail */
+	rtnl_lock();
+	/* Serialize the reset decision with ndo_stop and unregister. */
 	if (test_bit(NIC_STATE_DOWN, &priv->state) ||
 	    test_bit(NIC_STATE_REMOVING, &priv->state) ||
 	    test_bit(NIC_STATE_RESETTING, &priv->state))
-		return;
+		goto out;
 
 	hns_nic_dump(priv);
 	netdev_info(priv->netdev, "try to reset %s port!\n",
 		    (type == HNAE_PORT_DEBUG ? "debug" : "service"));
 
-	rtnl_lock();
 	/* put off any impending NetWatchDogTimeout */
 	netif_trans_update(priv->netdev);
 	hns_nic_net_reinit(priv->netdev);
 
+out:
 	rtnl_unlock();
 }
 
@@ -2058,7 +2059,14 @@ static void hns_nic_service_task(struct work_struct *work)
 	struct hnae_handle *h = priv->ae_handle;
 
 	hns_nic_reset_subtask(priv);
-	hns_nic_update_link_status(priv->netdev);
+
+	rtnl_lock();
+	/* Keep linkwatch events within the netdev's registered lifetime. */
+	if (!test_bit(NIC_STATE_DOWN, &priv->state) &&
+	    !test_bit(NIC_STATE_REMOVING, &priv->state))
+		hns_nic_update_link_status(priv->netdev);
+	rtnl_unlock();
+
 	h->dev->ops->update_led_status(h);
 	hns_nic_update_stats(priv->netdev);
 
@@ -2398,6 +2406,21 @@ static void hns_nic_dev_remove(struct platform_device *pdev)
 	struct net_device *ndev = platform_get_drvdata(pdev);
 	struct hns_nic_priv *priv = netdev_priv(ndev);
 
+	set_bit(NIC_STATE_REMOVING, &priv->state);
+	if (priv->notifier_block.notifier_call)
+		hnae_unregister_notifier(&priv->notifier_block);
+	priv->notifier_block.notifier_call = NULL;
+
+	/* Join a TX timeout that passed the REMOVING check before work drain. */
+	if (ndev->reg_state != NETREG_UNINITIALIZED) {
+		netif_tx_lock_bh(ndev);
+		netif_tx_unlock_bh(ndev);
+	}
+
+	/* Stop the timer and drain work before ndo_stop touches the hardware. */
+	timer_shutdown_sync(&priv->service_timer);
+	cancel_work_sync(&priv->service_task);
+
 	if (ndev->reg_state != NETREG_UNINITIALIZED)
 		unregister_netdev(ndev);
 
@@ -2411,12 +2434,6 @@ static void hns_nic_dev_remove(struct platform_device *pdev)
 	if (!IS_ERR_OR_NULL(priv->ae_handle))
 		hnae_put_handle(priv->ae_handle);
 	priv->ae_handle = NULL;
-	if (priv->notifier_block.notifier_call)
-		hnae_unregister_notifier(&priv->notifier_block);
-	priv->notifier_block.notifier_call = NULL;
-
-	set_bit(NIC_STATE_REMOVING, &priv->state);
-	(void)cancel_work_sync(&priv->service_task);
 
 	/* safe for ACPI FW */
 	of_node_put(to_of_node(priv->fwnode));