From: Sasha Levin <sashal@kernel.org>
To: stable@vger.kernel.org
Cc: Sam Edwards <cfsworks@gmail.com>,
Russell King <linux@armlinux.org.uk>,
Sam Edwards <CFSworks@gmail.com>, Paolo Abeni <pabeni@redhat.com>,
Sasha Levin <sashal@kernel.org>
Subject: [PATCH 6.1.y 3/3] net: stmmac: Prevent NULL deref when RX memory exhausted
Date: Sun, 10 May 2026 11:46:32 -0400 [thread overview]
Message-ID: <20260510154632.158995-3-sashal@kernel.org> (raw)
In-Reply-To: <20260510154632.158995-1-sashal@kernel.org>
From: Sam Edwards <cfsworks@gmail.com>
[ Upstream commit 0bb05e6adfa99a2ea1fee1125cc0953409f83ed8 ]
The CPU receives frames from the MAC through conventional DMA: the CPU
allocates buffers for the MAC, then the MAC fills them and returns
ownership to the CPU. For each hardware RX queue, the CPU and MAC
coordinate through a shared ring array of DMA descriptors: one
descriptor per DMA buffer. Each descriptor includes the buffer's
physical address and a status flag ("OWN") indicating which side owns
the buffer: OWN=0 for CPU, OWN=1 for MAC. The CPU is only allowed to set
the flag and the MAC is only allowed to clear it, and both must move
through the ring in sequence: thus the ring is used for both
"submissions" and "completions."
In the stmmac driver, stmmac_rx() bookmarks its position in the ring
with the `cur_rx` index. The main receive loop in that function checks
for rx_descs[cur_rx].own=0, gives the corresponding buffer to the
network stack (NULLing the pointer), and increments `cur_rx` modulo the
ring size. After the loop exits, stmmac_rx_refill(), which bookmarks its
position with `dirty_rx`, allocates fresh buffers and rearms the
descriptors (setting OWN=1). If it fails any allocation, it simply stops
early (leaving OWN=0) and will retry where it left off when next called.
This means descriptors have a three-stage lifecycle (terms my own):
- `empty` (OWN=1, buffer valid)
- `full` (OWN=0, buffer valid and populated)
- `dirty` (OWN=0, buffer NULL)
But because stmmac_rx() only checks OWN, it confuses `full`/`dirty`. In
the past (see 'Fixes:'), there was a bug where the loop could cycle
`cur_rx` all the way back to the first descriptor it dirtied, resulting
in a NULL dereference when mistaken for `full`. The aforementioned
commit resolved that *specific* failure by capping the loop's iteration
limit at `dma_rx_size - 1`, but this is only a partial fix: if the
previous stmmac_rx_refill() didn't complete, then there are leftover
`dirty` descriptors that the loop might encounter without needing to
cycle fully around. The current code therefore panics (see 'Closes:')
when stmmac_rx_refill() is memory-starved long enough for `cur_rx` to
catch up to `dirty_rx`.
Fix this by explicitly checking, before advancing `cur_rx`, if the next
entry is dirty; exit the loop if so. This prevents processing of the
final, used descriptor until stmmac_rx_refill() succeeds, but
fully prevents the `cur_rx == dirty_rx` ambiguity as the previous bugfix
intended: so remove the clamp as well. Since stmmac_rx_zc() is a
copy-paste-and-tweak of stmmac_rx() and the code structure is identical,
any fix to stmmac_rx() will also need a corresponding fix for
stmmac_rx_zc(). Therefore, apply the same check there.
In stmmac_rx() (not stmmac_rx_zc()), a related bug remains: after the
MAC sets OWN=0 on the final descriptor, it will be unable to send any
further DMA-complete IRQs until it's given more `empty` descriptors.
Currently, the driver simply *hopes* that the next stmmac_rx_refill()
succeeds, risking an indefinite stall of the receive process if not. But
this is not a regression, so it can be addressed in a future change.
Fixes: b6cb4541853c7 ("net: stmmac: avoid rx queue overrun")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221010
Cc: stable@vger.kernel.org
Suggested-by: Russell King <linux@armlinux.org.uk>
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
Link: https://patch.msgid.link/20260422044503.5349-1-CFSworks@gmail.com
Signed-off-by: Paolo Abeni <pabeni@redhat.com>
Signed-off-by: Sasha Levin <sashal@kernel.org>
---
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index bb9ab8a823f7d..16c4640e8e689 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5103,9 +5103,12 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
break;
/* Prefetch the next RX descriptor */
- rx_q->cur_rx = STMMAC_NEXT_ENTRY(rx_q->cur_rx,
- priv->dma_conf.dma_rx_size);
- next_entry = rx_q->cur_rx;
+ next_entry = STMMAC_NEXT_ENTRY(rx_q->cur_rx,
+ priv->dma_conf.dma_rx_size);
+ if (unlikely(next_entry == rx_q->dirty_rx))
+ break;
+
+ rx_q->cur_rx = next_entry;
if (priv->extend_desc)
np = (struct dma_desc *)(rx_q->dma_erx + next_entry);
@@ -5239,7 +5242,6 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
dma_dir = page_pool_get_dma_dir(rx_q->page_pool);
bufsz = DIV_ROUND_UP(priv->dma_conf.dma_buf_sz, PAGE_SIZE) * PAGE_SIZE;
- limit = min(priv->dma_conf.dma_rx_size - 1, (unsigned int)limit);
if (netif_msg_rx_status(priv)) {
void *rx_head;
@@ -5295,9 +5297,12 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
if (unlikely(status & dma_own))
break;
- rx_q->cur_rx = STMMAC_NEXT_ENTRY(rx_q->cur_rx,
- priv->dma_conf.dma_rx_size);
- next_entry = rx_q->cur_rx;
+ next_entry = STMMAC_NEXT_ENTRY(rx_q->cur_rx,
+ priv->dma_conf.dma_rx_size);
+ if (unlikely(next_entry == rx_q->dirty_rx))
+ break;
+
+ rx_q->cur_rx = next_entry;
if (priv->extend_desc)
np = (struct dma_desc *)(rx_q->dma_erx + next_entry);
--
2.53.0
prev parent reply other threads:[~2026-05-10 15:46 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-05-04 12:48 FAILED: patch "[PATCH] net: stmmac: Prevent NULL deref when RX memory exhausted" failed to apply to 6.1-stable tree gregkh
2026-05-10 15:46 ` [PATCH 6.1.y 1/3] net: stmmac: avoid shadowing global buf_sz Sasha Levin
2026-05-10 15:46 ` [PATCH 6.1.y 2/3] net: stmmac: rename STMMAC_GET_ENTRY() -> STMMAC_NEXT_ENTRY() Sasha Levin
2026-05-10 15:46 ` Sasha Levin [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20260510154632.158995-3-sashal@kernel.org \
--to=sashal@kernel.org \
--cc=cfsworks@gmail.com \
--cc=linux@armlinux.org.uk \
--cc=pabeni@redhat.com \
--cc=stable@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.