From: Ding Hui <dinghui@lixiang.com>

The DMA descriptor ring allocation in __init_dma_rx_desc_rings() and
__alloc_dma_tx_desc_resources() is split into multiple steps, each of
which may fail and return early while the per-queue cleanup paths still
call the free helpers for the partially-initialized queue.

When an intermediate allocation fails, several ring buffers may never
have been allocated and their pointers remain NULL:

  - rx_q->buf_pool can be NULL if its kzalloc_objs() failed, yet
    dma_free_rx_skbufs()/dma_free_rx_xskbufs() dereference
    rx_q->buf_pool[i] via stmmac_free_rx_buffer().

  - tx_q->tx_skbuff_dma can be NULL if its kzalloc_objs() failed, yet
    dma_free_tx_skbufs() dereferences tx_q->tx_skbuff_dma[i] via
    stmmac_free_tx_buffer().

  - tx_q->tx_skbuff (aliased with tx_q->xdpf through a union) can be
    NULL if its allocation failed while tx_skbuff_dma succeeded; in that
    case dma_free_tx_skbufs() does not bail out and
    stmmac_free_tx_buffer() dereferences tx_q->xdpf[i] / tx_skbuff[i].

Guard all of these accesses with NULL checks so the cleanup paths are
safe to run on a queue whose allocations failed part-way through.

Fixes: 2af6106ae949 ("net: stmmac: Introducing support for Page Pool")
Fixes: be8b38a722e6 ("net: stmmac: Add support for XDP_TX action")

Signed-off-by: Ding Hui <dinghui@lixiang.com>
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c    | 16 ++++++++++++++--
 1 file changed, 14 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f2fc89176654..71c6a941fb91 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1728,7 +1728,7 @@ static void stmmac_free_tx_buffer(struct stmmac_priv *priv,
 					 DMA_TO_DEVICE);
 	}
 
-	if (tx_q->xdpf[i] &&
+	if (tx_q->xdpf && tx_q->xdpf[i] &&
 	    (tx_q->tx_skbuff_dma[i].buf_type == STMMAC_TXBUF_T_XDP_TX ||
 	     tx_q->tx_skbuff_dma[i].buf_type == STMMAC_TXBUF_T_XDP_NDO)) {
 		xdp_return_frame(tx_q->xdpf[i]);
@@ -1738,7 +1738,7 @@ static void stmmac_free_tx_buffer(struct stmmac_priv *priv,
 	if (tx_q->tx_skbuff_dma[i].buf_type == STMMAC_TXBUF_T_XSK_TX)
 		tx_q->xsk_frames_done++;
 
-	if (tx_q->tx_skbuff[i] &&
+	if (tx_q->tx_skbuff && tx_q->tx_skbuff[i] &&
 	    tx_q->tx_skbuff_dma[i].buf_type == STMMAC_TXBUF_T_SKB) {
 		dev_kfree_skb_any(tx_q->tx_skbuff[i]);
 		tx_q->tx_skbuff[i] = NULL;
@@ -1761,6 +1761,10 @@ static void dma_free_rx_skbufs(struct stmmac_priv *priv,
 	struct stmmac_rx_queue *rx_q = &dma_conf->rx_queue[queue];
 	int i;
 
+	/* buf_pool may not be allocated if alloc failed early */
+	if (!rx_q->buf_pool)
+		return;
+
 	for (i = 0; i < dma_conf->dma_rx_size; i++)
 		stmmac_free_rx_buffer(priv, rx_q, i);
 }
@@ -1802,6 +1806,10 @@ static void dma_free_rx_xskbufs(struct stmmac_priv *priv,
 	struct stmmac_rx_queue *rx_q = &dma_conf->rx_queue[queue];
 	int i;
 
+	/* buf_pool may not be allocated if alloc failed early */
+	if (!rx_q->buf_pool)
+		return;
+
 	for (i = 0; i < dma_conf->dma_rx_size; i++) {
 		struct stmmac_rx_buffer *buf = &rx_q->buf_pool[i];
 
@@ -2097,6 +2105,10 @@ static void dma_free_tx_skbufs(struct stmmac_priv *priv,
 	struct stmmac_tx_queue *tx_q = &dma_conf->tx_queue[queue];
 	int i;
 
+	/* tx_skbuff_dma may not be allocated if alloc failed early */
+	if (!tx_q->tx_skbuff_dma)
+		return;
+
 	tx_q->xsk_frames_done = 0;
 
 	for (i = 0; i < dma_conf->dma_tx_size; i++)
-- 
2.34.1

On Sun, Aug 30, 2026 at 12:06:08PM +0800, Ding Hui wrote:
> From: Ding Hui <dinghui@lixiang.com>
> 
> The DMA descriptor ring allocation in __init_dma_rx_desc_rings() and
> __alloc_dma_tx_desc_resources() is split into multiple steps, each of
> which may fail and return early while the per-queue cleanup paths still
> call the free helpers for the partially-initialized queue.

"and return early", is the real problem here. When a function returns
an error, it should first undo what it has done, up to the point of
the error.

Rather than add extra NULL checks, please work on
__init_dma_rx_desc_rings() and __alloc_dma_tx_desc_resources() and
make them cleanup on error.

I would say the problem you are trying to fix does not bother anybody,
so is not for stable. So i aim the patches for net-next.

	 Andrew

At 2026-08-30 22:06:16, "Andrew Lunn" <andrew@lunn.ch> wrote:
>On Sun, Aug 30, 2026 at 12:06:08PM +0800, Ding Hui wrote:
>> From: Ding Hui <dinghui@lixiang.com>
>> 
>> The DMA descriptor ring allocation in __init_dma_rx_desc_rings() and
>> __alloc_dma_tx_desc_resources() is split into multiple steps, each of
>> which may fail and return early while the per-queue cleanup paths still
>> call the free helpers for the partially-initialized queue.
>
>"and return early", is the real problem here. When a function returns
>an error, it should first undo what it has done, up to the point of
>the error.
>
>Rather than add extra NULL checks, please work on
>__init_dma_rx_desc_rings() and __alloc_dma_tx_desc_resources() and
>make them cleanup on error.
>
>I would say the problem you are trying to fix does not bother anybody,
>so is not for stable. So i aim the patches for net-next.

Thanks for the review and the direction.
You are right. The proper fix is to make __alloc_dma_rx_desc_resources()
and __alloc_dma_tx_desc_resources() clean up their own allocations on
error, rather than relying on the callers to handle partially-initialized
state.
I've reworked the patch accordingly: each function now will undo what they
have done before they return error. I also kept the NULL checks in the free
helpers as a defensive measure.