[PATCH 0/3] stmmac crash/stall fixes when under memory pressure

[PATCH 0/3] stmmac crash/stall fixes when under memory pressure

[Sam Edwards]

Hi netdev maintainers,

This is a pair of bugfixes for the stmmac driver's receive pipeline. These
issues occur when stmmac_rx_refill() does not (fully) succeed, which happens
more frequently when free memory is low.

The first patch closes Bugzilla bug #221010, where stmmac_rx() can circle
around to a still-dirty descriptor (with a NULL buffer pointer), mistake it for
a filled descriptor (due to OWN=0), and attempt to dereference the buffer.

In testing that patch, I discovered a second issue: starvation of available RX
buffers causes the NIC to stop sending interrupts; if the driver stops polling,
it will wait indefinitely for an interrupt that will never come. (Note: the
first patch makes this issue more prominent -- mostly because it lets the
system survive long enough to exhibit it -- but doesn't *cause* it.) The second
patch addresses that problem as well.

The first two patches are minimal and intended for stable. The third is a
general cleanup that removes the `limit` clamp and *solves* the issue of the
loop mistaking dirty entries for filled ones, rather than *avoiding* it.
Because this final patch changes the range of stmmac_rx_dirty()'s possible
return values, it's intended for -next, not stable.

I look forward to your feedback,
Sam

Sam Edwards (3):
  net: stmmac: Fix NULL deref when RX encounters a dirty descriptor
  net: stmmac: Prevent indefinite RX stall on buffer exhaustion
  net: stmmac: Remove stmmac_rx()'s `limit`, check desc validity instead

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

-- 
2.52.0

[PATCH 1/3] net: stmmac: Fix NULL deref when RX encounters a dirty descriptor

[Sam Edwards]

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

[PATCH 2/3] net: stmmac: Prevent indefinite RX stall on buffer exhaustion

[Sam Edwards]

The stmmac driver handles interrupts in the usual NAPI way: an interrupt
arrives, the NAPI instance is scheduled and interrupts are masked, and
the actual work occurs in the NAPI polling function. Once no further
work remains, interrupts are unmasked and the NAPI instance is put to
sleep to await a future interrupt. In the receive case, the MAC only
sends the interrupt when a DMA operation completes; thus the driver must
make sure a usable RX DMA descriptor exists before expecting a future
interrupt.

The main receive loop in stmmac_rx() exits under one of 3 conditions:
1) It encounters a DMA descriptor with OWN=1, indicating that no further
   pending data exists. The MAC will use this descriptor for the next
   RX DMA operation, so the driver can expect a future interrupt.
2) It exhausts the NAPI budget. In this case, the driver doesn't know
   whether the MAC has any usable DMA descriptors. But when the driver
   consumes its full budget, that signals NAPI to keep polling, so the
   question is moot.
3) It runs out of (non-dirty) descriptors in the RX ring. In this case,
   the MAC will only have a usable descriptor if stmmac_rx_refill()
   succeeds (at least partially).

Currently, stmmac_rx() lacks any check against scenario #3 and
stmmac_rx_refill() failing: it will stop NAPI polling and unmask
interrupts to await an interrupt that will never arrive, stalling the
receive pipeline indefinitely.

Fix this by checking if stmmac_rx_dirty() returns its maximal value,
returning the full budget (which tells NAPI to keep polling) if so.

Cc: stable@vger.kernel.org
Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f98b070073c0..d18ee145f5ca 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -5593,6 +5593,7 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
  */
 static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 {
+	int budget = limit;
 	u32 rx_errors = 0, rx_dropped = 0, rx_bytes = 0, rx_packets = 0;
 	struct stmmac_rxq_stats *rxq_stats = &priv->xstats.rxq_stats[queue];
 	struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue];
@@ -5870,6 +5871,12 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 	priv->xstats.rx_dropped += rx_dropped;
 	priv->xstats.rx_errors += rx_errors;
 
+	/* If the RX queue is completely dirty, we can't expect a future
+	 * interrupt; tell NAPI to keep polling.
+	 */
+	if (unlikely(stmmac_rx_dirty(priv, queue) == priv->dma_conf.dma_rx_size - 1))
+		return budget;
+
 	return count;
 }
 
-- 
2.52.0

[PATCH 3/3] net: stmmac: Remove stmmac_rx()'s `limit`, check desc validity instead

[Sam Edwards]

A previous patch ("net: stmmac: Fix NULL deref when RX encounters a
dirty descriptor") fixed a bug where the receive loop may advance to a
still-dirty descriptor (i.e. one with OWN=0 but its buffer(s)
removed+NULLed), causing a panic. That fix worked by tightening the
loop's iteration limit so that it must stop short of the last non-dirty
descriptor in the ring.

This works, and is minimal enough for stable, but isn't an overall clean
approach: it deliberately ignores a (potentially-ready) descriptor, and
is avoiding the real issue -- that both "dirty" and "ready" descriptors
are OWN=0, and the loop doesn't understand the ambiguity.

Thus, strengthen the loop by explicitly checking whether the page(s) are
allocated for each descriptor, disambiguating "ready" pages from "dirty"
ones. Next, because `cur_rx` is now allowed to advance to a dirty
page, also remove the clamp from the beginning of stmmac_rx(). Finally,
resolve the "head == tail ring buffer ambiguity" problem this creates in
stmmac_rx_dirty() by explicitly checking if `cur_rx` is missing its
buffer(s).

Note that this changes the valid range of stmmac_rx_dirty()'s return
value from `0 <= x < dma_rx_size` to `0 <= x <= dma_rx_size`.

Signed-off-by: Sam Edwards <CFSworks@gmail.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c    | 16 ++++++++++------
 1 file changed, 10 insertions(+), 6 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index d18ee145f5ca..9074668db8be 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -375,8 +375,11 @@ static inline u32 stmmac_rx_dirty(struct stmmac_priv *priv, u32 queue)
 {
 	struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue];
 	u32 dirty;
+	struct stmmac_rx_buffer *buf = &rx_q->buf_pool[rx_q->cur_rx];
 
-	if (rx_q->dirty_rx <= rx_q->cur_rx)
+	if (!buf->page || (priv->sph_active && !buf->sec_page))
+		dirty = priv->dma_conf.dma_rx_size;
+	else if (rx_q->dirty_rx <= rx_q->cur_rx)
 		dirty = rx_q->cur_rx - rx_q->dirty_rx;
 	else
 		dirty = priv->dma_conf.dma_rx_size - rx_q->dirty_rx + rx_q->cur_rx;
@@ -5593,7 +5596,6 @@ static int stmmac_rx_zc(struct stmmac_priv *priv, int limit, u32 queue)
  */
 static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 {
-	int budget = limit;
 	u32 rx_errors = 0, rx_dropped = 0, rx_bytes = 0, rx_packets = 0;
 	struct stmmac_rxq_stats *rxq_stats = &priv->xstats.rxq_stats[queue];
 	struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue];
@@ -5610,8 +5612,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 - stmmac_rx_dirty(priv, queue) - 1,
-		    (unsigned int)limit);
 
 	if (netif_msg_rx_status(priv)) {
 		void *rx_head;
@@ -5656,6 +5656,10 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 		entry = next_entry;
 		buf = &rx_q->buf_pool[entry];
 
+		/* don't eat our own tail */
+		if (unlikely(!buf->page || (priv->sph_active && !buf->sec_page)))
+			break;
+
 		if (priv->extend_desc)
 			p = (struct dma_desc *)(rx_q->dma_erx + entry);
 		else
@@ -5874,8 +5878,8 @@ static int stmmac_rx(struct stmmac_priv *priv, int limit, u32 queue)
 	/* If the RX queue is completely dirty, we can't expect a future
 	 * interrupt; tell NAPI to keep polling.
 	 */
-	if (unlikely(stmmac_rx_dirty(priv, queue) == priv->dma_conf.dma_rx_size - 1))
-		return budget;
+	if (unlikely(stmmac_rx_dirty(priv, queue) == priv->dma_conf.dma_rx_size))
+		return limit;
 
 	return count;
 }
-- 
2.52.0

Re: [PATCH 0/3] stmmac crash/stall fixes when under memory pressure

[Jakub Kicinski]

On Sun, 15 Mar 2026 19:10:06 -0700 Sam Edwards wrote:
> This is a pair of bugfixes for the stmmac driver's receive pipeline. These
> issues occur when stmmac_rx_refill() does not (fully) succeed, which happens
> more frequently when free memory is low.
> 
> The first patch closes Bugzilla bug #221010, where stmmac_rx() can circle
> around to a still-dirty descriptor (with a NULL buffer pointer), mistake it for
> a filled descriptor (due to OWN=0), and attempt to dereference the buffer.
> 
> In testing that patch, I discovered a second issue: starvation of available RX
> buffers causes the NIC to stop sending interrupts; if the driver stops polling,
> it will wait indefinitely for an interrupt that will never come. (Note: the
> first patch makes this issue more prominent -- mostly because it lets the
> system survive long enough to exhibit it -- but doesn't *cause* it.) The second
> patch addresses that problem as well.
> 
> The first two patches are minimal and intended for stable. The third is a
> general cleanup that removes the `limit` clamp and *solves* the issue of the
> loop mistaking dirty entries for filled ones, rather than *avoiding* it.
> Because this final patch changes the range of stmmac_rx_dirty()'s possible
> return values, it's intended for -next, not stable.

Please (ask your LLM to) read
https://www.kernel.org/doc/html/next/process/maintainer-netdev.html

You can't submit fixes and next changes in one go.

Please have a critical look at the commit messages.
They are very bloviated and not slop looking.
-- 
pw-bot: cr

Re: [PATCH 0/3] stmmac crash/stall fixes when under memory pressure

[Sam Edwards]

On Tue, Mar 17, 2026 at 8:27 PM Jakub Kicinski <kuba@kernel.org> wrote:
> ...

Hi Jakub,

> Please (ask your LLM to) read
> https://www.kernel.org/doc/html/next/process/maintainer-netdev.html

Thanks, I wasn't aware that this subsystem-specific documentation
existed. The reverse-xmas-tree rule also means I need to move the `int
budget` variable lower in the declarations.

> You can't submit fixes and next changes in one go.

ACK, I'll drop patch 3 for now and designate a v2 for `net`.

> Please have a critical look at the commit messages.
> They are very bloviated and not slop looking.

Thank you for saying my messages aren't slop. :) I'll try to dial back
the verbosity, though. What kind of bloviation do I have? Should I be
leaving off more context, or should I focus more on preserving the
same detail but using fewer words?

Cheers,
Sam

Re: [PATCH 0/3] stmmac crash/stall fixes when under memory pressure

[Jakub Kicinski]

On Tue, 17 Mar 2026 20:46:00 -0700 Sam Edwards wrote:
> > Please have a critical look at the commit messages.
> > They are very bloviated and not slop looking.  
> 
> Thank you for saying my messages aren't slop. :)

Typo, of course. Are you saying you haven't use an LLM at all to write
these patches or commit messages?

> I'll try to dial back the verbosity, though. What kind of bloviation
> do I have? Should I be leaving off more context, or should I focus
> more on preserving the same detail but using fewer words?

IDK, writing is hard. I read the commit message for patch 1 and I felt
like I understood less than when I started :S Try to imagine you don't
know the code very well and think what information is crucial to
understanding the change.

Re: [PATCH 0/3] stmmac crash/stall fixes when under memory pressure

[Sam Edwards]

On Wed, Mar 18, 2026 at 3:11 PM Jakub Kicinski <kuba@kernel.org> wrote:
>
> On Tue, 17 Mar 2026 20:46:00 -0700 Sam Edwards wrote:
> > > Please have a critical look at the commit messages.
> > > They are very bloviated and not slop looking.
> >
> > Thank you for saying my messages aren't slop. :)
>
> Typo, of course. Are you saying you haven't use an LLM at all to write
> these patches or commit messages?

That's right: My crash analysis, source code study, bug reporting, fix
implementation, testing, commit messages, and correspondence were done
the pre-LLM way. (Perhaps it's time for me to adopt a disclaimer a la
xkcd.com/3126? /s)

> > I'll try to dial back the verbosity, though. What kind of bloviation
> > do I have? Should I be leaving off more context, or should I focus
> > more on preserving the same detail but using fewer words?
>
> IDK, writing is hard. I read the commit message for patch 1 and I felt
> like I understood less than when I started :S

Ah yeah, I get it: Sometimes with negative feedback, people don't know
*why* something is off, only *that* it's off.

(Aside: I originally wrote the patch 1 message last month as a comment
explaining my issue on Bugzilla, and later reused most of it when
preparing this series. Patches 2/3 were written fresh as I was getting
ready to hit send. Maybe I was relying on context from the Bugzilla
thread more than I thought.)

> Try to imagine you don't
> know the code very well and think what information is crucial to
> understanding the change.

I have a bit of an edge then: I'm actually pretty new to this driver,
and used the commit messages to document what I had to learn in the
process.

I'll try to workshop the commit messages in preparation for a series
v2. Thank you for taking the time to speak with me about this.

Best wishes,
Sam