Skip to contentSkip to conversation

forcedeth: fix UAF of txrx_stats in nv_remove

PATCH Applied Chenguang Zhao 4 participants 4 messages Jul 23 – Jul 28

Subsystems (3)
Networking Network drivers Forcedeth gigabit Ethernet driver
lore
Share & tools
lore

Chenguang Zhao

author
#0 lore raw

PATCH 1/1 forcedeth: fix UAF of txrx_stats in nv_remove

From: Chenguang Zhao <[email protected]>

nv_remove() frees the per-CPU txrx_stats before unregister_netdev(). Until unregister completes, ndo_get_stats64, the NAPI/xmit data path, and nv_close()/drain may still access txrx_stats, leading to a use-after-free.

Free the stats only after unregister_netdev().

Fixes f4b633b911fd ("forcedeth: use per cpu to collect xmit/recv statistics")
Signed-off-by Chenguang Zhao <[email protected]>
---
 drivers/net/ethernet/nvidia/forcedeth.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/ethernet/nvidia/forcedeth.c +4 -2
index 5b0435d7bc39..106885bd0f6b 100644
@@ -6187,9 +6187,11 @@ static void nv_remove(struct pci_dev *pci_dev)
 	struct net_device *dev = pci_get_drvdata(pci_dev);
 	struct fe_priv *np = netdev_priv(dev);
 
-	free_percpu(np->txrx_stats);
-
+	/* txrx_stats is used by ndo_get_stats64 and the data path until
+	 * unregister_netdevice() has completed.
+	 */
 	unregister_netdev(dev);
+	free_percpu(np->txrx_stats);
 
 	nv_restore_mac_addr(pci_dev);
Signature · 1 line
2.25.1
Reviewed-by on v1 1/1
#1 lore raw
Quoted 23 lines · Chenguang Zhao

From: Chenguang Zhao <[email protected]>

nv_remove() frees the per-CPU txrx_stats before unregister_netdev(). Until unregister completes, ndo_get_stats64, the NAPI/xmit data path, and nv_close()/drain may still access txrx_stats, leading to a use-after-free.

Free the stats only after unregister_netdev().

Fixes f4b633b911fd ("forcedeth: use per cpu to collect xmit/recv statistics")
Signed-off-by Chenguang Zhao <[email protected]>
---
  drivers/net/ethernet/nvidia/forcedeth.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/ethernet/nvidia/forcedeth.c +0 -0
index 5b0435d7bc39..106885bd0f6b 100644
@@ -6187,9 +6187,11 @@ static void nv_remove(struct pci_dev *pci_dev)
  	struct net_device *dev = pci_get_drvdata(pci_dev);
  	struct fe_priv *np = netdev_priv(dev);

drivers/net/ethernet/nvidia/forcedeth.c static void nv_remove(struct pci_dev *pci_dev) · line 6197

Quoted 10 lines
drivers/net/ethernet/nvidia/forcedeth.c +4 -2
-	free_percpu(np->txrx_stats);
-
+	/* txrx_stats is used by ndo_get_stats64 and the data path until
+	 * unregister_netdevice() has completed.
+	 */
  	unregister_netdev(dev);
+	free_percpu(np->txrx_stats);
 
  	nv_restore_mac_addr(pci_dev);
Reviewed-by Vadim Fedorenko <[email protected]>
Reviewed-by on v1 1/1
#2 lore raw
In reply to Chenguang Zhao
Quoted 11 lines · Chenguang Zhao

From: Chenguang Zhao <[email protected]>

nv_remove() frees the per-CPU txrx_stats before unregister_netdev(). Until unregister completes, ndo_get_stats64, the NAPI/xmit data path, and nv_close()/drain may still access txrx_stats, leading to a use-after-free.

Free the stats only after unregister_netdev().

Fixes f4b633b911fd ("forcedeth: use per cpu to collect xmit/recv statistics")
Signed-off-by Chenguang Zhao <[email protected]>

Thanks a lot. I am fine with it.

Reviewed-by Zhu Yanjun <[email protected]>

Zhu Yanjun

Quoted 22 lines
---
  drivers/net/ethernet/nvidia/forcedeth.c | 6 ++++--
  1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/ethernet/nvidia/forcedeth.c +4 -2
index 5b0435d7bc39..106885bd0f6b 100644
@@ -6187,9 +6187,11 @@ static void nv_remove(struct pci_dev *pci_dev)
  	struct net_device *dev = pci_get_drvdata(pci_dev);
  	struct fe_priv *np = netdev_priv(dev);
 
-	free_percpu(np->txrx_stats);
-
+	/* txrx_stats is used by ndo_get_stats64 and the data path until
+	 * unregister_netdevice() has completed.
+	 */
  	unregister_netdev(dev);
+	free_percpu(np->txrx_stats);
 
  	nv_restore_mac_addr(pci_dev);
Signature · 2 lines
Best Regards,
Yanjun.Zhu
maintainer Applied
#3 lore raw
In reply to Chenguang Zhao
Quoted 20 lines · On Thu, 23 Jul 2026 17:26:37 +0800 Chenguang Zhao wrote:

From: Chenguang Zhao <[email protected]>

nv_remove() frees the per-CPU txrx_stats before unregister_netdev(). Until unregister completes, ndo_get_stats64, the NAPI/xmit data path, and nv_close()/drain may still access txrx_stats, leading to a use-after-free.

Free the stats only after unregister_netdev().

Fixes f4b633b911fd ("forcedeth: use per cpu to collect xmit/recv statistics")
Signed-off-by Chenguang Zhao <[email protected]>
---
 drivers/net/ethernet/nvidia/forcedeth.c | 6 ++++--
 1 file changed, 4 insertions(+), 2 deletions(-)
drivers/net/ethernet/nvidia/forcedeth.c +0 -0
index 5b0435d7bc39..106885bd0f6b 100644

drivers/net/ethernet/nvidia/forcedeth.c static void nv_remove(struct pci_dev *pci_dev) · line 6192

Quoted 8 lines
drivers/net/ethernet/nvidia/forcedeth.c +3 -2
@@ -6187,9 +6187,11 @@ static void nv_remove(struct pci_dev *pci_dev)
 	struct net_device *dev = pci_get_drvdata(pci_dev);
 	struct fe_priv *np = netdev_priv(dev);
 
-	free_percpu(np->txrx_stats);
-
+	/* txrx_stats is used by ndo_get_stats64 and the data path until
+	 * unregister_netdevice() has completed.
+	 */

Applied, but without this stupid comment and preserving the original spacing.

Quoted 5 lines
drivers/net/ethernet/nvidia/forcedeth.c +1 -0
 	unregister_netdev(dev);
+	free_percpu(np->txrx_stats);
 
 	nv_restore_mac_addr(pci_dev);