Re: [PATCH] net: stmmac: fix integer underflow in chain mode jumbo_frm

Simon Horman
2026-03-23T14:18:27Z

On Fri, Mar 20, 2026 at 11:10:58PM -0500, Tyllis Xu wrote:
> The jumbo_frm() chain-mode implementation unconditionally computes
>
> len = nopaged_len - bmax;
>
> where nopaged_len = skb_headlen(skb) (linear bytes only) and bmax is
> BUF_SIZE_8KiB or BUF_SIZE_2KiB. However, the caller stmmac_xmit()
> decides to invoke jumbo_frm() based on skb->len (total length including
> page fragments):
>
> is_jumbo = stmmac_is_jumbo_frm(priv, skb->len, enh_desc);
>
> When a packet has a small linear portion (nopaged_len <= bmax) but a
> large total length due to page fragments (skb->len > bmax), the
> subtraction wraps as an unsigned integer, producing a huge len value
> (~0xFFFFxxxx). This causes the while (len != 0) loop to execute
> hundreds of thousands of iterations, passing skb->data + bmax * i
> pointers far beyond the skb buffer to dma_map_single(). On IOMMU-less
> SoCs (the typical deployment for stmmac), this maps arbitrary kernel
> memory to the DMA engine, constituting a kernel memory disclosure and
> potential memory corruption from hardware.
>
> The ring-mode counterpart already guards against this with:
>
> if (nopaged_len > BUF_SIZE_8KiB) { ... use len ... }
> else { ... map nopaged_len directly ... }
>
> Apply the same pattern to chain mode: guard the chunked-DMA path with
> if (nopaged_len > bmax), and add an else branch that maps the entire
> linear portion as a single descriptor when it fits within bmax. The
> fragment loop in stmmac_xmit() handles page fragments afterward.
>
> Fixes: 286a83721720 ("stmmac: add CHAINED descriptor mode support (V4)")
> Cc: stable@vger.kernel.org
> Signed-off-by: Tyllis Xu <LivelyCarpet87@gmail.com>

As a fix for code present in net this patch should be targeted at the net
tree like this:

Subject: [PATCH net] net: stmmac: fix integer underflow in chain mode

As is, our CI tries to apply this patch to the default tree, net-next.
Which fails due to a conflict with commit 6b4286e05508 ("net: stmmac:
rename STMMAC_GET_ENTRY() -> STMMAC_NEXT_ENTRY()"). So no CI tests were
run.

> ---
> drivers/net/ethernet/stmicro/stmmac/chain_mode.c | 71 ++++++++++++++---------
> 1 file changed, 44 insertions(+), 27 deletions(-)

The bulk of this patch is whitespace change (indentation).
So seems useful to examine this patch with whitespace changes ignored.

git diff -w yeilds;

diff --git a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
index 120a009c9992..c8980482dea2 100644
--- a/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
+++ b/drivers/net/ethernet/stmicro/stmmac/chain_mode.c
@@ -31,6 +31,7 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb,
else
bmax = BUF_SIZE_2KiB;

+ if (nopaged_len > bmax) {
len = nopaged_len - bmax;

des2 = dma_map_single(priv->device, skb->data,
@@ -77,6 +78,18 @@ static int jumbo_frm(struct stmmac_tx_queue *tx_q, struct sk_buff *skb,
len = 0;
}
}
+ } else {
+ des2 = dma_map_single(priv->device, skb->data,
+ nopaged_len, DMA_TO_DEVICE);
+ desc->des2 = cpu_to_le32(des2);
+ if (dma_mapping_error(priv->device, des2))
+ return -1;
+ tx_q->tx_skbuff_dma[entry].buf = des2;
+ tx_q->tx_skbuff_dma[entry].len = nopaged_len;
+ stmmac_prepare_tx_desc(priv, desc, 1, nopaged_len, csum,
+ STMMAC_CHAIN_MODE, 0, !skb_is_nonlinear(skb),
+ skb->len);
+ }

tx_q->cur_tx = entry;

The code in the else arm of the new condition is quite similar to
the (not visible in the diff above) code at the top of the non-else
arm of the condition.

I do see this is consistent with the ring-mode code. So perhaps it is
appropriate as a fix. But I do wonder if this could be consolidated - e.g.
by setting up some local variables rather than moving the mapping logic
into a condition.