[PATCH v2] net: stmmac: fix RX DMA leak on TX alloc failure

[PATCH v2] net: stmmac: fix RX DMA leak on TX alloc failure

[Abid Ali via B4 Relay]

From: Abid Ali <dev.taqnialabs@gmail.com>

Free RX DMA resources when alloc_dma_tx_desc_resources() fails in
alloc_dma_desc_resources().

Signed-off-by: Abid Ali <dev.taqnialabs@gmail.com>
---
Changes in v2:
- Restructured return path based on feedback.
- Link to v1: https://lore.kernel.org/r/20260425-stmmac-rx-desc-cleanup-v1-1-1a18a704c422@gmail.com
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_main.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 13d3cac05..240453daa 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2370,6 +2370,8 @@ static int alloc_dma_desc_resources(struct stmmac_priv *priv,
 		return ret;
 
 	ret = alloc_dma_tx_desc_resources(priv, dma_conf);
+	if (ret)
+		free_dma_rx_desc_resources(priv, dma_conf);
 
 	return ret;
 }

---
base-commit: 028ef9c96e96197026887c0f092424679298aae8
change-id: 20260425-stmmac-rx-desc-cleanup-440f05845492

Best regards,
-- 
Abid Ali <dev.taqnialabs@gmail.com>

Re: [PATCH v2] net: stmmac: fix RX DMA leak on TX alloc failure

[Abid Ali]

> 	ret = alloc_dma_tx_desc_resources(priv, dma_conf);
>+	if (ret)
>+		free_dma_rx_desc_resources(priv, dma_conf);
>
> 	return ret;
> }

The sashiko-gemini analysis [1] flagged two issues.

1) Double-free via XDP path:

stmmac_xdp_set_prog() ignores the return of stmmac_xdp_open(), so
if alloc_dma_tx_desc_resources() fails inside that path,
rx_q->buf_pool and rx_q->dma_rx are freed for Rx queues.

The interface stays UP, so a later stmmac_release() calls
free_dma_desc_resources() on the same freed pointers.

Without this patch, the same failure path leaks RX resources
instead. Either way the root cause seems to be stmmac_xdp_set_prog() not
handling errors from stmmac_xdp_open().

The reported issue seems to be valid, but I'm not sure why XDP doesn't handle
a possible error in reinit in the first place.

2) NULL deref on partial queue alloc:

If alloc_dma_rx_desc_resources() fails for queue N,
e.g. rx_q->page_pool = page_pool_create() fails, buf_pool is NULL.
The cleanup free_dma_rx_desc_resources() iterates through all
queues and will hit a NULL pointer deref in:

static void stmmac_free_rx_buffer(struct stmmac_priv *priv,
				  struct stmmac_rx_queue *rx_q,
				  int i)
{
	struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i];

The same could happen without the patch, and similar risk exists for
rx_q->buf_pool, rx_q->dma_rx, and rx_q->dma_erx which are all freed
without guards in __free_dma_rx_desc_resources().

I can add the necessary NULL guards in __free_dma_rx_desc_resources()
for V3 if necessary.

[1] https://sashiko.dev/#/patchset/20260522-stmmac-rx-desc-cleanup-v2-1-76e78eb471e1@gmail.com

- Abid

Re: [PATCH v2] net: stmmac: fix RX DMA leak on TX alloc failure

[Paolo Abeni]

On 5/23/26 2:17 PM, Abid Ali wrote:
>> 	ret = alloc_dma_tx_desc_resources(priv, dma_conf);
>> +	if (ret)
>> +		free_dma_rx_desc_resources(priv, dma_conf);
>>
>> 	return ret;
>> }
> 
> The sashiko-gemini analysis [1] flagged two issues.
> 
> 1) Double-free via XDP path:
> 
> stmmac_xdp_set_prog() ignores the return of stmmac_xdp_open(), so
> if alloc_dma_tx_desc_resources() fails inside that path,
> rx_q->buf_pool and rx_q->dma_rx are freed for Rx queues.
> 
> The interface stays UP, so a later stmmac_release() calls
> free_dma_desc_resources() on the same freed pointers.
> 
> Without this patch, the same failure path leaks RX resources
> instead. Either way the root cause seems to be stmmac_xdp_set_prog() not
> handling errors from stmmac_xdp_open().
> 
> The reported issue seems to be valid, but I'm not sure why XDP doesn't handle
> a possible error in reinit in the first place.
> 
> 2) NULL deref on partial queue alloc:
> 
> If alloc_dma_rx_desc_resources() fails for queue N,
> e.g. rx_q->page_pool = page_pool_create() fails, buf_pool is NULL.
> The cleanup free_dma_rx_desc_resources() iterates through all
> queues and will hit a NULL pointer deref in:
> 
> static void stmmac_free_rx_buffer(struct stmmac_priv *priv,
> 				  struct stmmac_rx_queue *rx_q,
> 				  int i)
> {
> 	struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i];
> 
> The same could happen without the patch, and similar risk exists for
> rx_q->buf_pool, rx_q->dma_rx, and rx_q->dma_erx which are all freed
> without guards in __free_dma_rx_desc_resources().
> 
> I can add the necessary NULL guards in __free_dma_rx_desc_resources()
> for V3 if necessary.
My take is following: even if the sashiko reported issues are
pre-existing, they are so strictly tied to this code path that it makes
sense to address all of them together (different patches in the same
series).

Also all of the above looks like 'net' material, deserving fixes tag in
each patch.

/P