Re: [PATCH net] net: stmmac: take ownership of saved RX state at poll entry
flat view
From: Lorenzo Bianconi <lorenzo@kernel.org>
Date: 2026-10-01 08:32:46
Also in:
linux-arm-kernel, lkml
When a saved partial packet completes with a poll budget of one, the
old loop can leave state_saved and state.skb pointing at an skb that
has already been delivered or freed. The next poll then reuses that
pointer, causing a use-after-free or double free.
Take the saved state at poll entry and clear the stored ownership
immediately. Save it again only if the packet remains incomplete,
including when the next descriptor is still DMA-owned. Release a
saved partial skb when the RX ring is destroyed.
Fixes: ec222003bd94 ("net: stmmac: Prepare to add Split Header support")
Signed-off-by: James Hilliard <redacted>Hi James, I guess we have a similar issue for stmmac_rx_zc() path as well, can you please fix it as well?
quoted hunk ↗ jump to hunk
--- drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 25 ++++++++++++++++------- 1 file changed, 18 insertions(+), 7 deletions(-)diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index ec62fa7418f4..1a4d03aaaf78 100644 --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c@@ -2149,6 +2149,11 @@ static void __free_dma_rx_desc_resources(struct stmmac_priv *priv, else dma_free_rx_skbufs(priv, dma_conf, queue); + if (rx_q->state_saved) + dev_kfree_skb_any(rx_q->state.skb);
nit: you can drop if (rx_q->state_saved) and just run dev_kfree_skb_any().
+ rx_q->state.skb = NULL; + rx_q->state_saved = 0;
nit: rx_q->state_saved = false;
quoted hunk ↗ jump to hunk
+ rx_q->buf_alloc_num = 0; rx_q->xsk_pool = NULL;@@ -5726,6 +5731,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) struct stmmac_xdp_buff ctx; bool fcs_stripped = false; int xdp_status = 0; + bool in_progress = rx_q->state_saved;
can you please respect RCT here? Regards, Lorenzo
quoted hunk ↗ jump to hunk
int bufsz; dma_dir = page_pool_get_dma_dir(rx_q->page_pool);@@ -5740,6 +5746,14 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) stmmac_display_ring(priv, rx_head, priv->dma_conf.dma_rx_size, true, rx_q->dma_rx_phy, desc_size); } + if (in_progress) { + skb = rx_q->state.skb; + error = rx_q->state.error; + len = rx_q->state.len; + rx_q->state.skb = NULL; + rx_q->state_saved = false; + } + while (count < limit) { unsigned int buf1_len = 0, buf2_len = 0; enum pkt_hash_types hash_type;@@ -5748,12 +5762,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) int entry; u32 hash; - if (!count && rx_q->state_saved) { - skb = rx_q->state.skb; - error = rx_q->state.error; - len = rx_q->state.len; - } else { - rx_q->state_saved = false; + if (!in_progress) { skb = NULL; error = 0; len = 0;@@ -5787,6 +5796,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) prefetch(np); + in_progress = status & rx_not_ls; + if (priv->extend_desc) stmmac_rx_extended_status(priv, &priv->xstats, rx_q->dma_erx + entry); if (unlikely(status == discard_frame)) {@@ -5971,7 +5982,7 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue) count++; } - if (status & rx_not_ls || skb) { + if (in_progress || skb) { rx_q->state_saved = true; rx_q->state.skb = skb; rx_q->state.error = error;--- base-commit: 7375d38364a9aa66fb31716bcefef38aecad75d8 change-id: 20260930-stmmac-rx-state-041371e43e8c Best regards, -- James Hilliard [off-list ref]
Attachments
- signature.asc [application/pgp-signature] 228 bytes