[PATCH net v3 0/2] net/stmmac: Fix panic during interface shutdown & apply STMMAC_DOWN flag

[PATCH net v3 0/2] net/stmmac: Fix panic during interface shutdown & apply STMMAC_DOWN flag

[Jakub Raczynski]

When testing interface shutdown during XDP operation using stmmac driver,
we have encountered kernel panics, either poison overwritten or wrong
memory access, caused by XDP processing data after shutting down NAPI.

To fix this problem:
- Add STMMAC_DOWN handling to all XDP paths
- Change location of synchronize_rcu() to proper place
- Apply STMMAC_DOWN flag on interface open()/release().
  This flag is used for XDP only and does improve handling in edge cases

Proper order of ensuring proper XDP shutdown is already present in functions
from stmmac_xdp.c during xdp_disable_pool(), since XDP can still have
data with NAPI disabled, as disabling NAPI does not ensure XDP finish.
synchronize_rcu() must be executed after IRQ's & DMA channels are disabled to
flush data.

---
Changes in v3:
- Discard almost all changes from v2.
  This was a mistake as napi_disable() does ensure napi_synchronize() and
  it didn't fix anything in the end.
- Return STMMAC_DOWN flag set/clear
- Create new function that executes synchronize_rcu() and call it from
  proper places
- Remove synchronize_rcu() from stmmac_disable_all_queues()
- Remove barren stmmac_disable_all_queues() and replace it with
  __stmmac_disable_all_queues()
- Fix memory leak in STMMAC_DOWN handling in modified XDP paths

Changes in v2:
- Split patch into two: one for XDP paths and second for general fix
- Change commit messages & title
- Fix all cases of NAPI release, not only XDP, via modyfying
  stmmac_disable_all_queues() instead of separate later call
- Drop setting/clearing of STMMAC_DOWN flag in release()/open()

Link to v2:
https://lore.kernel.org/all/20260601163258.554300-3-j.raczynski@samsung.com
Link to v1:
https://lore.kernel.org/all/20260511165045.3091475-1-j.raczynski@samsung.com


Jakub Raczynski (2):
  net/stmmac: Check for STMMAC_DOWN flag in all XDP paths
  net/stmmac: Fix free-after-use panic when interface goes does with XDP

 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 40 ++++++++++++++-----
 1 file changed, 31 insertions(+), 9 deletions(-)

-- 
2.34.1

[PATCH net v3 1/2] net/stmmac: Check for STMMAC_DOWN flag in all XDP paths

[Jakub Raczynski]

Currently STMMAC_DOWN flag is only set/cleared by stmmac_reset_subtask(),
to notify driver to stop processing of TX/RX frames. One of these processing
paths is for XDP, but it is only ever checked in stmmac_xdp_xmit(), which
leaves all other XDP paths vulnerable to processing data while interface is
restarting.

Make verification of STMMAC_DOWN flag consistent by applying check to
all XDP paths.

Fixes: 8b278a5b69a22 ("net: stmmac: Add support for XDP_REDIRECT action")
Co-developed-by: Chang-Sub Lee <cs0617.lee@samsung.com>
Signed-off-by: Chang-Sub Lee <cs0617.lee@samsung.com>
Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 2a0d7eff88d3..b9ffff001baf 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5260,12 +5260,18 @@ static int stmmac_xdp_xmit_back(struct stmmac_priv *priv,
 				struct xdp_buff *xdp)
 {
 	bool zc = !!(xdp->rxq->mem.type == MEM_TYPE_XSK_BUFF_POOL);
-	struct xdp_frame *xdpf = xdp_convert_buff_to_frame(xdp);
+	struct xdp_frame *xdpf;
 	int cpu = smp_processor_id();
 	struct netdev_queue *nq;
 	int queue;
 	int res;
 
+	if (unlikely(test_bit(STMMAC_DOWN, &priv->state))) {
+		xsk_buff_free(xdp);
+		return STMMAC_XSK_CONSUMED;
+	}
+
+	xdpf = xdp_convert_buff_to_frame(xdp);
 	if (unlikely(!xdpf))
 		return STMMAC_XDP_CONSUMED;
 
@@ -5310,7 +5316,9 @@ static int __stmmac_xdp_run_prog(struct stmmac_priv *priv,
 		res = stmmac_xdp_xmit_back(priv, xdp);
 		break;
 	case XDP_REDIRECT:
-		if (xdp_do_redirect(priv->dev, xdp, prog) < 0)
+		if (unlikely(test_bit(STMMAC_DOWN, &priv->state)))
+			res = STMMAC_XDP_CONSUMED;
+		else if (xdp_do_redirect(priv->dev, xdp, prog) < 0)
 			res = STMMAC_XDP_CONSUMED;
 		else
 			res = STMMAC_XDP_REDIRECT;
-- 
2.34.1

[PATCH net v3 2/2] net/stmmac: Fix free-after-use panic when interface goes does with XDP

[Jakub Raczynski]

When running XDP forwarding and interface gets shut down, kernel might panic
or show SLUB "poison overwritten" errors due to a race condition between
NAPI polling and resource freeing.

Observed error is one of following:
- Poison overwrriten
[ 1889.547746] eth1: Link is Down
[ 1889.549940] =============================================================================
[ 1889.549954] BUG kmalloc-4k (Tainted: G    B             ): Poison overwritten
[ 1889.549959] -----------------------------------------------------------------------------
[ 1889.549963] 0xffffff882dcc4d80-0xffffff882dcc4da7 @offset=19840. First byte 0x0 instead of 0x6b
[ 1889.549969] Allocated in __alloc_dma_tx_desc_resources+0x60/0x10c [stmmac] age=169 cpu=7 pid=27759
[ 1889.550020]  __kmem_cache_alloc_node+0x100/0x2e8
[ 1889.550032]  __kmalloc+0x58/0x1a0
[ 1889.550039]  __alloc_dma_tx_desc_resources+0x60/0x10c [stmmac]
[ 1889.550052]  alloc_dma_desc_resources+0xec/0x164 [stmmac]
[ 1889.550064]  stmmac_setup_dma_desc+0xec/0x1e4 [stmmac]
[ 1889.550076]  stmmac_open+0x28/0x94 [stmmac]
[...]

- Wrong memory address
[ 1901.546692] Unable to handle kernel paging request at virtual address dead000000000122
[...]
[ 1902.964068] Call trace:
[ 1902.967193]  free_to_partial_list+0x560/0x600
[ 1902.972227]  __slab_free+0x1a8/0x420
[ 1902.976480]  __kmem_cache_free+0x204/0x218
[ 1902.981254]  kfree+0x6c/0x128
[ 1902.984900]  kvfree+0x3c/0x4c
[ 1902.988545]  page_pool_release+0x234/0x27c
[ 1902.993320]  page_pool_destroy+0xcc/0x190
[ 1902.998006]  __free_dma_rx_desc_resources+0x100/0x360 [stmmac]
[ 1903.004516]  free_dma_desc_resources+0x8c/0xac [stmmac]
[ 1903.010419]  stmmac_release+0x1c0/0x2b4 [stmmac]
[...]

Root cause is stmmac_release() stops DMA and frees TX/RX ring buffers and
page pools while XDP could still be accessing these resources in the
background, because napi_synchronize() from napi_disable() does not ensure
XDP is done.

This makes small window where IRQ is still possible after NAPI has finished.
Problem is that stmmac_release() handles closing XDP in different order than
stmmac_xdp_disable_pool(), which is not affected by this issue, where
synchronize_rcu() is executed after napi_disable() and disable_rx/tx_queue().

Fix this by following:
- Set STMMAC_DOWN flag before stopping DMA to signal XDP to stop and discard
- Call synchronize_rcu() after stopping DMA but before freeing resources to
  ensure all ongoing NAPI operations complete, in similar order to
  stmmac_xdp_disable_pool()
- Clear STMMAC_DOWN flag in __stmmac_open() to restore normal operation.
  This was only done for stmmac_reset_subtask() during abnormal operation,
  which is not enough. This flag does not affect normal operation as it is
  used only for XDP apps. Usage of such flags is far from optimal and would
  be good to rewrite, but it would be quite an effort

Also replace stmmac_disable_all_queues() with __stmmac_disable_all_queues(),
as it is barren after this change.

Fixes: bba2556efad66 ("net: stmmac: Enable RX via AF_XDP zero-copy")
Co-developed-by: Chang-Sub Lee <cs0617.lee@samsung.com>
Signed-off-by: Chang-Sub Lee <cs0617.lee@samsung.com>
Signed-off-by: Jakub Raczynski <j.raczynski@samsung.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 28 ++++++++++++++-----
 1 file changed, 21 insertions(+), 7 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b9ffff001baf..9d971ae35d48 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -262,7 +262,11 @@ static void stmmac_verify_args(void)
 		pr_warn("stmmac: module parameter 'flow_ctrl' is obsolete - please remove from your module configuration\n");
 }
 
-static void __stmmac_disable_all_queues(struct stmmac_priv *priv)
+/**
+ * stmmac_disable_all_queues - Disable all queues
+ * @priv: driver private structure
+ */
+static void stmmac_disable_all_queues(struct stmmac_priv *priv)
 {
 	u8 rx_queues_cnt = priv->plat->rx_queues_to_use;
 	u8 tx_queues_cnt = priv->plat->tx_queues_to_use;
@@ -286,16 +290,15 @@ static void __stmmac_disable_all_queues(struct stmmac_priv *priv)
 }
 
 /**
- * stmmac_disable_all_queues - Disable all queues
+ * stmmac_drain_xdp - Cleanup for XDP apps
  * @priv: driver private structure
  */
-static void stmmac_disable_all_queues(struct stmmac_priv *priv)
+static void stmmac_drain_xdp(struct stmmac_priv *priv)
 {
 	u8 rx_queues_cnt = priv->plat->rx_queues_to_use;
 	struct stmmac_rx_queue *rx_q;
 	u8 queue;
 
-	/* synchronize_rcu() needed for pending XDP buffers to drain */
 	for (queue = 0; queue < rx_queues_cnt; queue++) {
 		rx_q = &priv->dma_conf.rx_queue[queue];
 		if (rx_q->xsk_pool) {
@@ -303,8 +306,6 @@ static void stmmac_disable_all_queues(struct stmmac_priv *priv)
 			break;
 		}
 	}
-
-	__stmmac_disable_all_queues(priv);
 }
 
 /**
@@ -4149,6 +4150,9 @@ static int __stmmac_open(struct net_device *dev,
 
 	stmmac_reset_queues_param(priv);
 
+	/* Clear DOWN flag when opening the interface */
+	clear_bit(STMMAC_DOWN, &priv->state);
+
 	ret = stmmac_hw_setup(dev);
 	if (ret < 0) {
 		netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
@@ -4243,6 +4247,9 @@ static void __stmmac_release(struct net_device *dev)
 	/* Stop and disconnect the PHY */
 	phylink_stop(priv->phylink);
 
+	/* Set DOWN flag to prevent XDP from processing new packets */
+	set_bit(STMMAC_DOWN, &priv->state);
+
 	stmmac_disable_all_queues(priv);
 
 	for (chan = 0; chan < priv->plat->tx_queues_to_use; chan++)
@@ -4256,6 +4263,8 @@ static void __stmmac_release(struct net_device *dev)
 	/* Stop TX/RX DMA and clear the descriptors */
 	stmmac_stop_all_dma(priv);
 
+	stmmac_drain_xdp(priv);
+
 	/* Release and free the Rx/Tx resources */
 	free_dma_desc_resources(priv, &priv->dma_conf);
 
@@ -6412,7 +6421,7 @@ static int stmmac_setup_tc_block_cb(enum tc_setup_type type, void *type_data,
 	if (!tc_cls_can_offload_and_chain0(priv->dev, type_data))
 		return ret;
 
-	__stmmac_disable_all_queues(priv);
+	stmmac_disable_all_queues(priv);
 
 	switch (type) {
 	case TC_SETUP_CLSU32:
@@ -7122,6 +7131,9 @@ void stmmac_xdp_release(struct net_device *dev)
 	/* Stop TX/RX DMA channels */
 	stmmac_stop_all_dma(priv);
 
+	/* Drain leftover XDP buffers */
+	stmmac_drain_xdp(priv);
+
 	/* Release and free the Rx/Tx resources */
 	free_dma_desc_resources(priv, &priv->dma_conf);
 
@@ -8208,6 +8220,8 @@ int stmmac_suspend(struct device *dev)
 	/* Stop TX/RX DMA */
 	stmmac_stop_all_dma(priv);
 
+	stmmac_drain_xdp(priv);
+
 	stmmac_legacy_serdes_power_down(priv);
 
 	/* Enable Power down mode by programming the PMT regs */
-- 
2.34.1