public inbox for netdev@vger.kernel.org
 help / color / mirror / Atom feed
From: Sam Edwards <cfsworks@gmail.com>
To: Andrew Lunn <andrew+netdev@lunn.ch>,
	"David S . Miller" <davem@davemloft.net>,
	Eric Dumazet <edumazet@google.com>,
	Jakub Kicinski <kuba@kernel.org>, Paolo Abeni <pabeni@redhat.com>
Cc: Russell King <rmk+kernel@armlinux.org.uk>,
	Maxime Chevallier <maxime.chevallier@bootlin.com>,
	Ovidiu Panait <ovidiu.panait.rb@renesas.com>,
	Vladimir Oltean <vladimir.oltean@nxp.com>,
	Baruch Siach <baruch@tkos.co.il>,
	Serge Semin <fancer.lancer@gmail.com>,
	netdev@vger.kernel.org, linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, Sam Edwards <CFSworks@gmail.com>,
	stable@vger.kernel.org
Subject: [PATCH 1/3] net: stmmac: Fix NULL deref when RX encounters a dirty descriptor
Date: Sun, 15 Mar 2026 19:10:07 -0700	[thread overview]
Message-ID: <20260316021009.262358-2-CFSworks@gmail.com> (raw)
In-Reply-To: <20260316021009.262358-1-CFSworks@gmail.com>

Under typical conditions, `stmmac_rx_refill()` rearms every descriptor
in the RX ring. However, if it fails to allocate memory, it will stop
early and try again the next time it's called. In this situation, it
(correctly) leaves OWN=0 because the hardware is not yet allowed to
reclaim the descriptor.

`stmmac_rx()`, however, does not anticipate this scenario: it assumes
`cur_rx` always points to a valid descriptor, and that OWN=0 means the
buffer is ready for the driver. A `min()` clamp at the start prevents
`cur_rx` from wrapping all the way around the buffer (see Fixes:),
apparently intended to prevent the "head=tail ambiguity problem" from
breaking `stmmac_rx_refill()`. But this safeguard is incomplete because
the threshold to stay behind is actually `dirty_rx`, not `cur_rx`. It
works most of the time only by coincidence: `stmmac_rx_refill()` usually
succeeds often enough that it leaves `dirty_rx == cur_rx`. But when
`stmmac_rx()` is called when `dirty_rx != cur_rx` and the NAPI budget is
high, `cur_rx` can advance to a still-dirty descriptor, violating the
invariant and triggering a panic when the driver attempts to access a
missing buffer.

This can easily be fixed by subtracting `stmmac_rx_dirty()` from the
clamp. Because that function currently interprets `dirty_rx == cur_rx`
to mean "none dirty," its maximum return value is `dma_rx_size - 1`, so
doing this carries no risk of underflow, though does (like the Fixes:)
leave a clean buffer unreachable.

Fixes: b6cb4541853c7 ("net: stmmac: avoid rx queue overrun")
Closes: https://bugzilla.kernel.org/show_bug.cgi?id=221010
Cc: stable@vger.kernel.org
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 6827c99bde8c..f98b070073c0 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5609,7 +5609,8 @@ 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);
+	limit = min(priv->dma_conf.dma_rx_size - stmmac_rx_dirty(priv, queue) - 1,
+		    (unsigned int)limit);
 
 	if (netif_msg_rx_status(priv)) {
 		void *rx_head;
-- 
2.52.0


  reply	other threads:[~2026-03-16  2:10 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-16  2:10 [PATCH 0/3] stmmac crash/stall fixes when under memory pressure Sam Edwards
2026-03-16  2:10 ` Sam Edwards [this message]
2026-03-16  2:10 ` [PATCH 2/3] net: stmmac: Prevent indefinite RX stall on buffer exhaustion Sam Edwards
2026-03-16  2:10 ` [PATCH 3/3] net: stmmac: Remove stmmac_rx()'s `limit`, check desc validity instead Sam Edwards
2026-03-18  3:27 ` [PATCH 0/3] stmmac crash/stall fixes when under memory pressure Jakub Kicinski
2026-03-18  3:46   ` Sam Edwards
2026-03-18 22:11     ` Jakub Kicinski
2026-03-18 23:18       ` Sam Edwards

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=20260316021009.262358-2-CFSworks@gmail.com \
    --to=cfsworks@gmail.com \
    --cc=andrew+netdev@lunn.ch \
    --cc=baruch@tkos.co.il \
    --cc=davem@davemloft.net \
    --cc=edumazet@google.com \
    --cc=fancer.lancer@gmail.com \
    --cc=kuba@kernel.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=maxime.chevallier@bootlin.com \
    --cc=netdev@vger.kernel.org \
    --cc=ovidiu.panait.rb@renesas.com \
    --cc=pabeni@redhat.com \
    --cc=rmk+kernel@armlinux.org.uk \
    --cc=stable@vger.kernel.org \
    --cc=vladimir.oltean@nxp.com \
    /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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox