[PATCH net v4] net: stmmac: do not keep the new TSO MSS cached on mapping failures

[PATCH net v4] net: stmmac: do not keep the new TSO MSS cached on mapping failures

[Linkui Xiao]

From: Linkui Xiao <xiaolinkui@kylinos.cn>

stmmac_tso_xmit() fills the MSS context descriptor and stores the new MSS
in tx_q->mss right away, but the descriptor only gets its OWN bit much
later, right before the frame is handed to the DMA. Every error path in
between - the dma_map_single() of the linear part and the
skb_frag_dma_map() of each fragment - returns with tx_q->mss already
updated while the MAC is still programmed with the previous MSS.

The context descriptor is now handled like the data descriptors are:
tx_q->cur_tx is not advanced while it is being filled, so the slot stays
where the next transmit fills it, and the error paths release it
explicitly instead of leaving a descriptor the DMA will stop on in the
middle of the ring. With the queue no longer wedged by that descriptor,
the stale cached MSS is what remains: the next skb carrying the same
gso_size compares equal to the cached value, no context descriptor is
emitted, and the hardware segments the TCP stream with the MSS of an
earlier frame, generating frames whose payload size does not match what
the stack accounted for.

Drop the cached value on those paths instead. Zero is what
stmmac_reset_tx_queue() leaves behind, so the next TSO frame programs the
context descriptor again.

The store itself stays ahead of the mappings, as before, rather than
moving next to the OWN bit: stmmac_tx_err() reinitialises the channel and
clears tx_q->mss from the DMA interrupt handler, which takes no TX queue
lock, so it can run in the middle of stmmac_tso_xmit(). Publishing the
cache after the descriptor has been handed over would widen the window in
which such a reset is overwritten with an MSS that the reinitialised
channel was never programmed with.

Fixes: f748be531d70 ("stmmac: support new GMAC4")
Cc: stable@vger.kernel.org
Signed-off-by: Linkui Xiao <xiaolinkui@kylinos.cn>
---
v3:
- Link: https://lore.kernel.org/netdev/20260922124408.645496-1-xiaolinkui@126.com/
Changes in v4:
- Drop the cached MSS on the mapping failure paths instead of publishing it
  after the context descriptor has been given to the DMA. The deferred store
  widened the window in which a stmmac_tx_err() from the DMA interrupt
  handler, which takes no TX queue lock, is overwritten again with the new
  MSS for a channel that was never programmed with it; zero cannot
  republish anything, as it is what the reset itself leaves behind.
  (Sashiko AI review)
- Not carrying over the Acked-by from Lorenzo Bianconi, as the cache
  handling changed again.

 .../net/ethernet/stmicro/stmmac/stmmac_main.c    | 16 +++++++++++++---
 1 file changed, 13 insertions(+), 3 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index af2d38a2bb3d..a20b2366f61a 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4564,9 +4564,6 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
 
 		stmmac_set_mss(priv, mss_desc, mss);
 		tx_q->mss = mss;
-		tx_q->cur_tx = STMMAC_NEXT_ENTRY(tx_q->cur_tx,
-						priv->dma_conf.dma_tx_size);
-		WARN_ON(tx_q->tx_skbuff[tx_q->cur_tx]);
 	}
 
 	if (netif_msg_tx_queued(priv)) {
@@ -4577,6 +4574,9 @@ static netdev_tx_t stmmac_tso_xmit(struct sk_buff *skb, struct net_device *dev)
 	}
 
 	first_entry = tx_q->cur_tx;
+	if (mss_desc)
+		first_entry = STMMAC_NEXT_ENTRY(first_entry,
+						priv->dma_conf.dma_tx_size);
 	entry = first_entry;
 
 	WARN_ON(tx_q->tx_skbuff[entry]);
@@ -4745,6 +4745,16 @@ error_dma_unmap:
 						priv->dma_conf.dma_tx_size);
 	}
 error:
+	if (mss_desc) {
+		/* The context descriptor never reached the DMA, so the MAC is
+		 * still programmed with the previous MSS. Invalidate the cache
+		 * instead of leaving it ahead of the hardware; zero is the
+		 * value stmmac_reset_tx_queue() leaves behind.
+		 */
+		stmmac_release_tx_desc(priv, mss_desc, priv->descriptor_mode);
+		tx_q->mss = 0;
+	}
+
 	dev_err(priv->device, "Tx dma map failed\n");
 	dev_kfree_skb(skb);
 	priv->xstats.tx_dropped++;
-- 
2.25.1

Re: [PATCH net v4] net: stmmac: do not keep the new TSO MSS cached on mapping failures

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re: [PATCH net v4] net: stmmac: do not keep the new TSO MSS cached on mapping failures

[Linkui Xiao]

Hi,

Thanks for the reminder. Here is the missing information:

How discovered:
The original issue was found by code inspection of the error paths in
stmmac_tso_xmit(). The v3 regression - deferring tx_q->mss = mss until
the context descriptor OWN bit is set - was raised by the Sashiko AI
review of v3, which noted that a concurrent stmmac_tx_err() could reset
and clear tx_q->mss before the deferred store republished it. v4 avoids
that by keeping the store in place and clearing tx_q->mss on the DMA
mapping failure paths instead.

Triggered:
Not triggered on real hardware. This is a code-inspection/review finding.
The ring-wedge part requires a DMA mapping failure in the TSO transmit
path; the stale-MSS part then requires a later TSO frame with the same
gso_size to take the mss == tx_q->mss path and skip the context
descriptor. No stack trace, error message or syzbot report is available.

Testing:
Not tested on real hardware. No hardware test was performed.

On 2026/9/28 20:39, netdev-bot+sinfo@kernel.org wrote:
> Hi!
> 
> This is an automated message. This series looks like a fix, but its
> commit messages seem to be missing some information:
> 
>   - How the issue was discovered, e.g. hit in production, hit during
>     development, syzbot report, manual code inspection, LLM or static
>     analysis tool scan.
> 
>   - Whether the issue was actually triggered, or is only theoretical
>     (e.g. found by code inspection). If it was triggered please include
>     the symptoms, like the stack trace or error messages.
> 
>   - What hardware the change was tested on. For driver fixes please
>     mention the device (and if relevant firmware version) used for
>     testing, or say that the change was not tested on real hardware.
> 
> Please do not repost the series just to address the above. Instead,
> reply to this email with the missing information, so that reviewers
> can take it into account. If the series needs another revision for
> other reasons, please include the information in the commit messages
> then.
> 
> The evaluation is done by an LLM so it may be wrong, if you think
> that is the case please reply and explain.