* [PATCH net-next v6 1/4] net: stmmac: guard against NULL pointers when freeing TX/RX buffers
2026-10-24 7:13 [PATCH net-next v6 0/4] net: stmmac: fix DMA descriptor ring cleanup issues Ding Hui
@ 2026-10-24 7:13 ` Ding Hui
2026-10-24 7:13 ` [PATCH net-next v6 2/4] net: stmmac: clean up partially allocated DMA rings on error Ding Hui
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Ding Hui @ 2026-10-24 7:13 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
open list:STMMAC ETHERNET DRIVER,
moderated list:ARM/STM32 ARCHITECTURE,
moderated list:ARM/STM32 ARCHITECTURE, open list
Cc: dinghui, xiasanbo, yangchen11, liuxuanjun
From: Ding Hui <dinghui@lixiang.com>
The DMA resource free helpers may be invoked on queues whose buffers
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.
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.
Signed-off-by: Ding Hui <dinghui@lixiang.com>
---
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 32 +++++++++++++++++--
1 file changed, 29 insertions(+), 3 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b20e5038eead..2f03600cc5bd 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -1766,7 +1766,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]);
@@ -1778,7 +1778,7 @@ static void stmmac_free_tx_buffer(struct stmmac_priv *priv,
tx_q->tx_skbuff_dma[i].buf_type = STMMAC_TXBUF_T_SKB;
}
- 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;
@@ -1801,6 +1801,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);
}
@@ -1842,6 +1846,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];
@@ -2137,6 +2145,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++)
@@ -2194,13 +2206,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,
@@ -2242,9 +2261,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,
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net-next v6 2/4] net: stmmac: clean up partially allocated DMA rings on error
2026-10-24 7:13 [PATCH net-next v6 0/4] net: stmmac: fix DMA descriptor ring cleanup issues Ding Hui
2026-10-24 7:13 ` [PATCH net-next v6 1/4] net: stmmac: guard against NULL pointers when freeing TX/RX buffers Ding Hui
@ 2026-10-24 7:13 ` Ding Hui
2026-10-24 7:13 ` [PATCH net-next v6 3/4] net: stmmac: release descriptor resources through priv->dma_conf on open failure Ding Hui
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Ding Hui @ 2026-10-24 7:13 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
open list:STMMAC ETHERNET DRIVER,
moderated list:ARM/STM32 ARCHITECTURE,
moderated list:ARM/STM32 ARCHITECTURE, open list
Cc: dinghui, xiasanbo, yangchen11, liuxuanjun
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, relying on the NULL guards added previously.
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 of allocation.
Also 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.
Signed-off-by: Ding Hui <dinghui@lixiang.com>
---
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 40 +++++++++++++++----
1 file changed, 33 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 2f03600cc5bd..a8401b04d79f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2333,15 +2333,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;
@@ -2357,10 +2361,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,
@@ -2413,14 +2429,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;
@@ -2430,6 +2446,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,
@@ -2472,6 +2496,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;
}
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net-next v6 3/4] net: stmmac: release descriptor resources through priv->dma_conf on open failure
2026-10-24 7:13 [PATCH net-next v6 0/4] net: stmmac: fix DMA descriptor ring cleanup issues Ding Hui
2026-10-24 7:13 ` [PATCH net-next v6 1/4] net: stmmac: guard against NULL pointers when freeing TX/RX buffers Ding Hui
2026-10-24 7:13 ` [PATCH net-next v6 2/4] net: stmmac: clean up partially allocated DMA rings on error Ding Hui
@ 2026-10-24 7:13 ` Ding Hui
2026-10-24 7:13 ` [PATCH net-next v6 4/4] net: stmmac: hold RTNL when reading descriptor rings in descriptors_status Ding Hui
2026-10-24 7:21 ` [PATCH net-next v6 0/4] net: stmmac: fix DMA descriptor ring cleanup issues netdev-bot+sinfo
4 siblings, 0 replies; 6+ messages in thread
From: Ding Hui @ 2026-10-24 7:13 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
open list:STMMAC ETHERNET DRIVER,
moderated list:ARM/STM32 ARCHITECTURE,
moderated list:ARM/STM32 ARCHITECTURE, open list
Cc: dinghui, xiasanbo, yangchen11, liuxuanjun
From: Ding Hui <dinghui@lixiang.com>
__stmmac_open() copies the temporary dma_conf passed by its callers
into the long-lived priv->dma_conf before setting up the hardware.
When the open fails after this copy, the descriptor resources now
belong to priv->dma_conf, but the error path returns without
releasing them and leaves stale pointers behind. The callers then
free the already-copied resources through the temporary dma_conf,
which aliases priv->dma_conf after the memcpy.
Fix this by making __stmmac_open() release the descriptor resources
through priv->dma_conf on failure. Callers of __stmmac_open() only
need to free the temporary dma_conf container in that case, while
failures before the call (PHY setup, serdes power up) still release
the resources through the temporary dma_conf via the existing
err_dma_resources path in stmmac_open().
stmmac_open() no longer jumps to the serdes label chain after
__stmmac_open() fails, as that chain would double-release the
descriptor resources and double-put the runtime PM reference, so the
cleanup is done inline instead.
The resource release is merged with the error unwinding that stops
the DMA engines and the MAC: failures after the hardware setup
first stop the DMA engines and the MAC, and then fall through to
the common init_error label that releases the descriptor resources,
while a stmmac_hw_setup() failure jumps to init_error directly as
the engines were never started.
Signed-off-by: Ding Hui <dinghui@lixiang.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 a8401b04d79f..b093119e752f 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -4246,7 +4246,7 @@ static int __stmmac_open(struct net_device *dev,
ret = stmmac_hw_setup(dev);
if (ret < 0) {
netdev_err(priv->dev, "%s: Hw setup failed\n", __func__);
- return ret;
+ goto init_error;
}
ret = stmmac_setup_ptp(priv);
@@ -4282,6 +4282,8 @@ static int __stmmac_open(struct net_device *dev,
stmmac_stop_all_dma(priv);
stmmac_mac_set(priv, priv->ioaddr, false);
+init_error:
+ free_dma_desc_resources(priv, &priv->dma_conf);
return ret;
}
@@ -4314,8 +4316,13 @@ static int stmmac_open(struct net_device *dev)
}
ret = __stmmac_open(dev, dma_conf);
- if (ret)
- goto err_serdes;
+ if (ret) {
+ stmmac_legacy_serdes_power_down(priv);
+ phylink_disconnect_phy(priv->phylink);
+ pm_runtime_put(priv->device);
+ kfree(dma_conf);
+ return ret;
+ }
kfree(dma_conf);
@@ -4324,8 +4331,6 @@ static int stmmac_open(struct net_device *dev)
return ret;
-err_serdes:
- stmmac_legacy_serdes_power_down(priv);
err_disconnect_phy:
phylink_disconnect_phy(priv->phylink);
err_runtime_pm:
@@ -6343,7 +6348,6 @@ static int stmmac_change_mtu(struct net_device *dev, int new_mtu)
ret = __stmmac_open(dev, dma_conf);
if (ret) {
- free_dma_desc_resources(priv, dma_conf);
kfree(dma_conf);
netdev_err(priv->dev, "failed reopening the interface after MTU change\n");
return ret;
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* [PATCH net-next v6 4/4] net: stmmac: hold RTNL when reading descriptor rings in descriptors_status
2026-10-24 7:13 [PATCH net-next v6 0/4] net: stmmac: fix DMA descriptor ring cleanup issues Ding Hui
` (2 preceding siblings ...)
2026-10-24 7:13 ` [PATCH net-next v6 3/4] net: stmmac: release descriptor resources through priv->dma_conf on open failure Ding Hui
@ 2026-10-24 7:13 ` Ding Hui
2026-10-24 7:21 ` [PATCH net-next v6 0/4] net: stmmac: fix DMA descriptor ring cleanup issues netdev-bot+sinfo
4 siblings, 0 replies; 6+ messages in thread
From: Ding Hui @ 2026-10-24 7:13 UTC (permalink / raw)
To: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
open list:STMMAC ETHERNET DRIVER,
moderated list:ARM/STM32 ARCHITECTURE,
moderated list:ARM/STM32 ARCHITECTURE, open list
Cc: dinghui, xiasanbo, yangchen11, liuxuanjun
From: Ding Hui <dinghui@lixiang.com>
The descriptors_status debugfs entry inspects the descriptor rings
without any synchronization against the control-plane paths that
tear them down (ndo_stop, MTU change, XDP program release, XSK pool
reconfiguration). All of those paths hold RTNL, so take the same
lock around the reader.
Holding RTNL also makes it safe to report queues whose descriptor
rings are not allocated: the interface may be IFF_UP with unallocated
rings when an internal reopen fails (e.g. an MTU change whose
__stmmac_open() call fails), as those paths do not clear IFF_UP.
Skip such queues with a hint instead of dereferencing a NULL pointer
in stmmac_rings_status_show().
Signed-off-by: Ding Hui <dinghui@lixiang.com>
---
.../net/ethernet/stmicro/stmmac/stmmac_main.c | 27 ++++++++++++++++++-
1 file changed, 26 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index b093119e752f..0f4af7ec3fc8 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -6707,8 +6707,10 @@ static int stmmac_rings_status_show(struct seq_file *seq, void *v)
u8 tx_count = priv->plat->tx_queues_to_use;
u8 queue;
+ rtnl_lock();
+
if ((dev->flags & IFF_UP) == 0)
- return 0;
+ goto out;
for (queue = 0; queue < rx_count; queue++) {
struct stmmac_rx_queue *rx_q = &priv->dma_conf.rx_queue[queue];
@@ -6716,10 +6718,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);
@@ -6732,16 +6744,29 @@ 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);
}
}
+out:
+ rtnl_unlock();
+
return 0;
}
DEFINE_SHOW_ATTRIBUTE(stmmac_rings_status);
--
2.34.1
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH net-next v6 0/4] net: stmmac: fix DMA descriptor ring cleanup issues
2026-10-24 7:13 [PATCH net-next v6 0/4] net: stmmac: fix DMA descriptor ring cleanup issues Ding Hui
` (3 preceding siblings ...)
2026-10-24 7:13 ` [PATCH net-next v6 4/4] net: stmmac: hold RTNL when reading descriptor rings in descriptors_status Ding Hui
@ 2026-10-24 7:21 ` netdev-bot+sinfo
4 siblings, 0 replies; 6+ messages in thread
From: netdev-bot+sinfo @ 2026-10-24 7:21 UTC (permalink / raw)
To: Ding Hui
Cc: Maxime Chevallier, Andrew Lunn, David S. Miller, Eric Dumazet,
Jakub Kicinski, Paolo Abeni, Maxime Coquelin, Alexandre Torgue,
netdev, linux-stm32, linux-arm-kernel, linux-kernel, dinghui,
xiasanbo, yangchen11, liuxuanjun
Hi!
This is an automated message. This series looks like a fix, but its
commit messages seem to be missing some information:
- 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.
^ permalink raw reply [flat|nested] 6+ messages in thread