[PATCH net v4] net: stmmac: fix error path cleanup in DMA descriptor ring allocation

[PATCH net v4] net: stmmac: fix error path cleanup in DMA descriptor ring allocation

[Ding Hui]

From: Ding Hui <dinghui@lixiang.com>

__alloc_dma_rx_desc_resources() and __alloc_dma_tx_desc_resources()
allocate resources in multiple steps but return early on failure
without cleaning up what they have already allocated. The outer
error paths then call the free helpers on partially-initialized
queues, which dereference pointers that were never allocated:

  - dma_free_rx_skbufs() and dma_free_rx_xskbufs() dereference
    rx_q->buf_pool[i] via stmmac_free_rx_buffer(), but buf_pool
    may be NULL if its kzalloc_objs() failed.

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

  - stmmac_free_tx_buffer() dereferences tx_q->xdpf[i] and
    tx_q->tx_skbuff[i] (aliased through a union), but tx_skbuff
    may be NULL if its allocation failed while tx_skbuff_dma
    succeeded.

Fix this by making each allocation function responsible for undoing
its own allocations on error, following the standard kernel error
handling pattern of cleaning up in reverse order. Also add NULL
checks in the free helpers as a defensive measure, since they may
be called on partially-initialized queues.
Additionally, make __free_dma_rx_desc_resources() and
__free_dma_tx_desc_resources() clear the pointers they free, so that
the NULL guards in the free helpers hold reliably when the long-lived
priv->dma_conf is reused across XDP open/release cycles.
Free the RX resources in alloc_dma_desc_resources() when the TX
allocation fails, as the callers only free dma_conf itself on
that path. And skip queues whose descriptor rings are not allocated
in stmmac_rings_status_show() so reading the rings sysfs entry will
never dereference a NULL pointer.

Signed-off-by: Ding Hui <dinghui@lixiang.com>

---
Changes in v4:
  - Free the RX resources in alloc_dma_desc_resources() when the TX
    allocation fails.
  - Skip queues whose descriptor rings are not allocated in
    stmmac_rings_status_show() and print a hint instead.
  - Link to v3:
    https://lore.kernel.org/netdev/20260919121436.1642724-1-dinghui1111@163.com/
Changes in v3:
  - Use addr directly in __alloc_dma_rx_desc_resources() error path
    instead of the if/else block on rx_q->dma_erx/dma_rx.
  - Clear the freed pointers in __free_dma_rx_desc_resources() and
    __free_dma_tx_desc_resources().
  - Link to v2:
    https://lore.kernel.org/netdev/20260905154654.1725313-1-dinghui1111@163.com/
Changes in v2:
  - Instead of only adding NULL checks in the free helpers, also fix
    __alloc_dma_rx_desc_resources() and __alloc_dma_tx_desc_resources()
    to clean up their own allocations on error, as suggested by Andrew.
  - Update commit message.
  - Link to v1:
    https://lore.kernel.org/netdev/20260830040610.1156008-1-dinghui1111@163.com/
---
 .../net/ethernet/stmicro/stmmac/stmmac_main.c | 92 +++++++++++++++++--
 1 file changed, 82 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index f2fc89176654..e79dd726b1ca 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++)
@@ -2154,13 +2166,20 @@ static void __free_dma_rx_desc_resources(struct stmmac_priv *priv,
 	size = stmmac_get_rx_desc_size(priv) * dma_conf->dma_rx_size;
 
 	dma_free_coherent(priv->device, size, addr, rx_q->dma_rx_phy);
+	rx_q->dma_erx = NULL;
+	rx_q->dma_rx = NULL;
+	rx_q->dma_rx_phy = 0;
 
 	if (xdp_rxq_info_is_reg(&rx_q->xdp_rxq))
 		xdp_rxq_info_unreg(&rx_q->xdp_rxq);
 
 	kfree(rx_q->buf_pool);
-	if (rx_q->page_pool)
+	rx_q->buf_pool = NULL;
+
+	if (rx_q->page_pool) {
 		page_pool_destroy(rx_q->page_pool);
+		rx_q->page_pool = NULL;
+	}
 }
 
 static void free_dma_rx_desc_resources(struct stmmac_priv *priv,
@@ -2202,9 +2221,16 @@ static void __free_dma_tx_desc_resources(struct stmmac_priv *priv,
 	size = stmmac_get_tx_desc_size(priv, tx_q) * dma_conf->dma_tx_size;
 
 	dma_free_coherent(priv->device, size, addr, tx_q->dma_tx_phy);
+	tx_q->dma_etx = NULL;
+	tx_q->dma_entx = NULL;
+	tx_q->dma_tx = NULL;
+	tx_q->dma_tx_phy = 0;
 
 	kfree(tx_q->tx_skbuff_dma);
+	tx_q->tx_skbuff_dma = NULL;
+
 	kfree(tx_q->tx_skbuff);
+	tx_q->tx_skbuff = NULL;
 }
 
 static void free_dma_tx_desc_resources(struct stmmac_priv *priv,
@@ -2272,15 +2298,19 @@ static int __alloc_dma_rx_desc_resources(struct stmmac_priv *priv,
 	}
 
 	rx_q->buf_pool = kzalloc_objs(*rx_q->buf_pool, dma_conf->dma_rx_size);
-	if (!rx_q->buf_pool)
-		return -ENOMEM;
+	if (!rx_q->buf_pool) {
+		ret = -ENOMEM;
+		goto err_destroy_pool;
+	}
 
 	size = stmmac_get_rx_desc_size(priv) * dma_conf->dma_rx_size;
 
 	addr = dma_alloc_coherent(priv->device, size, &rx_q->dma_rx_phy,
 				  GFP_KERNEL);
-	if (!addr)
-		return -ENOMEM;
+	if (!addr) {
+		ret = -ENOMEM;
+		goto err_free_buf_pool;
+	}
 
 	if (priv->extend_desc)
 		rx_q->dma_erx = addr;
@@ -2296,10 +2326,22 @@ static int __alloc_dma_rx_desc_resources(struct stmmac_priv *priv,
 	ret = xdp_rxq_info_reg(&rx_q->xdp_rxq, priv->dev, queue, napi_id);
 	if (ret) {
 		netdev_err(priv->dev, "Failed to register xdp rxq info\n");
-		return -EINVAL;
+		goto err_free_dma;
 	}
 
 	return 0;
+
+err_free_dma:
+	dma_free_coherent(priv->device, size, addr, rx_q->dma_rx_phy);
+	rx_q->dma_erx = NULL;
+	rx_q->dma_rx = NULL;
+err_free_buf_pool:
+	kfree(rx_q->buf_pool);
+	rx_q->buf_pool = NULL;
+err_destroy_pool:
+	page_pool_destroy(rx_q->page_pool);
+	rx_q->page_pool = NULL;
+	return ret;
 }
 
 static int alloc_dma_rx_desc_resources(struct stmmac_priv *priv,
@@ -2352,14 +2394,14 @@ static int __alloc_dma_tx_desc_resources(struct stmmac_priv *priv,
 
 	tx_q->tx_skbuff = kzalloc_objs(struct sk_buff *, dma_conf->dma_tx_size);
 	if (!tx_q->tx_skbuff)
-		return -ENOMEM;
+		goto err_free_skbuff_dma;
 
 	size = stmmac_get_tx_desc_size(priv, tx_q) * dma_conf->dma_tx_size;
 
 	addr = dma_alloc_coherent(priv->device, size,
 				  &tx_q->dma_tx_phy, GFP_KERNEL);
 	if (!addr)
-		return -ENOMEM;
+		goto err_free_skbuff;
 
 	if (priv->extend_desc)
 		tx_q->dma_etx = addr;
@@ -2369,6 +2411,14 @@ static int __alloc_dma_tx_desc_resources(struct stmmac_priv *priv,
 		tx_q->dma_tx = addr;
 
 	return 0;
+
+err_free_skbuff:
+	kfree(tx_q->tx_skbuff);
+	tx_q->tx_skbuff = NULL;
+err_free_skbuff_dma:
+	kfree(tx_q->tx_skbuff_dma);
+	tx_q->tx_skbuff_dma = NULL;
+	return -ENOMEM;
 }
 
 static int alloc_dma_tx_desc_resources(struct stmmac_priv *priv,
@@ -2411,6 +2461,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;
 }
@@ -6510,10 +6562,20 @@ static int stmmac_rings_status_show(struct seq_file *seq, void *v)
 		seq_printf(seq, "RX Queue %d:\n", queue);
 
 		if (priv->extend_desc) {
+			if (!rx_q->dma_erx) {
+				seq_puts(seq, "Extended descriptor ring not allocated\n");
+				continue;
+			}
+
 			seq_printf(seq, "Extended descriptor ring:\n");
 			sysfs_display_ring((void *)rx_q->dma_erx,
 					   priv->dma_conf.dma_rx_size, 1, seq, rx_q->dma_rx_phy);
 		} else {
+			if (!rx_q->dma_rx) {
+				seq_puts(seq, "Descriptor ring not allocated\n");
+				continue;
+			}
+
 			seq_printf(seq, "Descriptor ring:\n");
 			sysfs_display_ring((void *)rx_q->dma_rx,
 					   priv->dma_conf.dma_rx_size, 0, seq, rx_q->dma_rx_phy);
@@ -6526,10 +6588,20 @@ static int stmmac_rings_status_show(struct seq_file *seq, void *v)
 		seq_printf(seq, "TX Queue %d:\n", queue);
 
 		if (priv->extend_desc) {
+			if (!tx_q->dma_etx) {
+				seq_puts(seq, "Extended descriptor ring not allocated\n");
+				continue;
+			}
+
 			seq_printf(seq, "Extended descriptor ring:\n");
 			sysfs_display_ring((void *)tx_q->dma_etx,
 					   priv->dma_conf.dma_tx_size, 1, seq, tx_q->dma_tx_phy);
 		} else if (!(tx_q->tbs & STMMAC_TBS_AVAIL)) {
+			if (!tx_q->dma_tx) {
+				seq_puts(seq, "Descriptor ring not allocated\n");
+				continue;
+			}
+
 			seq_printf(seq, "Descriptor ring:\n");
 			sysfs_display_ring((void *)tx_q->dma_tx,
 					   priv->dma_conf.dma_tx_size, 0, seq, tx_q->dma_tx_phy);
-- 
2.34.1

Re: [PATCH net v4] net: stmmac: fix error path cleanup in DMA descriptor ring allocation

[netdev-bot+sinfo]

Hi!

This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:

 - How the issue was discovered, e.g. hit in production, hit during
   development, syzbot report, manual code inspection, LLM or static
   analysis tool scan.

 - Whether the issue was actually triggered, or is only theoretical
   (e.g. found by code inspection). If it was triggered please include
   the symptoms, like the stack trace or error messages.

 - What hardware the change was tested on. For driver fixes please
   mention the device (and if relevant firmware version) used for
   testing, or say that the change was not tested on real hardware.

Please do not repost the series just to address the above. Instead,
reply to this email with the missing information, so that reviewers
can take it into account. If the series needs another revision for
other reasons, please include the information in the commit messages
then.

The evaluation is done by an LLM so it may be wrong, if you think
that is the case please reply and explain.

Re:Re: [PATCH net v4] net: stmmac: fix error path cleanup in DMA descriptor ring allocation

[Ding Hui]

At 2026-10-01 11:58:59, netdev-bot+sinfo@kernel.org wrote:
>Hi!
>
>This is an automated message. This series looks like a fix, but its
>commit messages seem to be missing some information:
>
> - How the issue was discovered, e.g. hit in production, hit during
>   development, syzbot report, manual code inspection, LLM or static
>   analysis tool scan.

This issue was discovered during a stress testing scenario in the development process.

>
> - Whether the issue was actually triggered, or is only theoretical
>   (e.g. found by code inspection). If it was triggered please include
>   the symptoms, like the stack trace or error messages.
>

The specific scenario triggering this issue involves performing `ifconfig down/up` operations
on a network device during an OOM (Out of Memory) condition. The call stack at the time of the
failure may like this:
[540776.647689] Call trace:
[540776.647691]  dump_backtrace+0x98/0xf8
[540776.647701]  show_stack+0x20/0x38
[540776.647704]  dump_stack_lvl+0xbc/0xd0
[540776.647719]  dump_stack+0x18/0x28
[540776.647723]  warn_alloc+0x138/0x1d0
[540776.647731]  __alloc_pages_noprof+0x4e8/0xfd0
[540776.647735]  ___kmalloc_large_node+0xb8/0x1a8
[540776.647740]  __kmalloc_large_node_noprof+0x34/0x118
[540776.647743]  __kmalloc_noprof+0x2d4/0x378
[540776.647747]  __alloc_dma_tx_desc_resources+0x4c/0x118
[540776.647753]  alloc_dma_desc_resources+0xd8/0x150
[540776.647756]  stmmac_setup_dma_desc+0x118/0x270
[540776.647759]  stmmac_open+0x30/0xe8
[540776.647762]  __dev_open+0x108/0x1f8
[540776.647767]  __dev_change_flags+0x1d4/0x268
[540776.647770]  dev_change_flags+0x2c/0x80
[540776.647773]  devinet_ioctl+0x2dc/0x618
[540776.647778]  inet_ioctl+0x1d4/0x1f0
[540776.647781]  sock_do_ioctl+0x68/0x130
[540776.647786]  sock_ioctl+0x288/0x398
[540776.647788]  __arm64_sys_ioctl+0xb0/0x100
[540776.647795]  invoke_syscall+0x84/0x108
[540776.647801]  el0_svc_common.constprop.0+0xc8/0xf0
[540776.647805]  do_el0_svc+0x24/0x38
[540776.647808]  el0_svc+0x38/0x120
[540776.647813]  el0t_64_sync_handler+0x120/0x130
[540776.647816]  el0t_64_sync+0x190/0x198

[540776.648028] Unable to handle kernel NULL pointer dereference at virtual address 0000000000000000
[540776.648031] Mem abort info:
[540776.648033]   ESR = 0x0000000096000006
[540776.648035]   EC = 0x25: DABT (current EL), IL = 32 bits
[540776.648038]   SET = 0, FnV = 0
[540776.648039]   EA = 0, S1PTW = 0
[540776.648041]   FSC = 0x06: level 2 translation fault
[540776.648043] Data abort info:
[540776.648045]   ISV = 0, ISS = 0x00000006, ISS2 = 0x00000000
[540776.648047]   CM = 0, WnR = 0, TnD = 0, TagAccess = 0
[540776.648049]   GCS = 0, Overlay = 0, DirtyBit = 0, Xs = 0
[540776.648051] user pgtable: 4k pages, 39-bit VAs, pgdp=00000013c6eed000
[540776.648053] [0000000000000000] pgd=08000013b0800003, p4d=08000013b0800003, pud=08000013b0800003, pmd=0000000000000000
[540776.648063] Internal error: Oops: 0000000096000006 [#1] PREEMPT_RT SMP

[540776.648145] pstate: 20401005 (nzCv daif +PAN -UAO -TCO -DIT +SSBS BTYPE=--)
[540776.648147] pc : dma_free_tx_skbufs+0x108/0x1b8
[540776.648151] lr : __free_dma_tx_desc_resources+0x2c/0xb8
[540776.648154] sp : ffffffc0bc5f3610
[540776.648156] x29: ffffffc0bc5f3610 x28: ffffff83e4a2c600 x27: ffffff87de595a00
[540776.648162] x26: ffffff87de8a8000 x25: 0000000000001003 x24: ffffff83dcc26000
[540776.648168] x23: 0000000000000000 x22: ffffff87de8a8a00 x21: 0000000000000000
[540776.648174] x20: 0000000000000000 x19: ffffff83dcc26100 x18: ffffffc0bc5f2f20
[540776.648179] x17: 0000000000000000 x16: 0000000000000000 x15: ffffffe62e994454
[540776.648185] x14: ffffffe62e994440 x13: 0a64656e6f73696f x12: 7077682073656761
[540776.648190] x11: 0000000000000000 x10: 0000000000000000 x9 : ffffffe62d20dc4c
[540776.648196] x8 : ffffffc0bc5f34c8 x7 : 0000000000000000 x6 : 0000000000000001
[540776.648201] x5 : ffffffe62e41c000 x4 : ffffffe62e41c600 x3 : 0000000000000000
[540776.648207] x2 : 0000000000000000 x1 : ffffff83dcc26000 x0 : 0000000000001000
[540776.648213] Call trace:
[540776.648215]  dma_free_tx_skbufs+0x108/0x1b8
[540776.648217]  __free_dma_tx_desc_resources+0x2c/0xb8
[540776.648219]  alloc_dma_desc_resources+0x104/0x150
[540776.648222]  stmmac_setup_dma_desc+0x118/0x270
[540776.648225]  stmmac_open+0x30/0xe8
[540776.648228]  __dev_open+0x108/0x1f8
[540776.648231]  __dev_change_flags+0x1d4/0x268
[540776.648234]  dev_change_flags+0x2c/0x80
[540776.648236]  devinet_ioctl+0x2dc/0x618
[540776.648240]  inet_ioctl+0x1d4/0x1f0
[540776.648243]  sock_do_ioctl+0x68/0x130
[540776.648246]  sock_ioctl+0x288/0x398
[540776.648248]  __arm64_sys_ioctl+0xb0/0x100
[540776.648252]  invoke_syscall+0x84/0x108
[540776.648256]  el0_svc_common.constprop.0+0xc8/0xf0
[540776.648260]  do_el0_svc+0x24/0x38
[540776.648263]  el0_svc+0x38/0x120
[540776.648267]  el0t_64_sync_handler+0x120/0x130
[540776.648270]  el0t_64_sync+0x190/0x198
[540776.648274] Code: 54000389 f9449262 93797eb4 937d7eb7 (f8746843) 
[540776.648277] ---[ end trace 0000000000000000 ]---
[540776.681767] Kernel panic - not syncing: Oops: Fatal exception

Alternatively, a simpler method to reproduce the issue is by injecting a fault
through stubbing `alloc_dma_tx_desc_resources`.

> - What hardware the change was tested on. For driver fixes please
>   mention the device (and if relevant firmware version) used for
>   testing, or say that the change was not tested on real hardware.
>

This modification was tested and verified on an in-house developed SoC
platform (featuring Synopsys XGMAC).

>Please do not repost the series just to address the above. Instead,
>reply to this email with the missing information, so that reviewers
>can take it into account. If the series needs another revision for
>other reasons, please include the information in the commit messages
>then.
>
>The evaluation is done by an LLM so it may be wrong, if you think
>that is the case please reply and explain.