From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 82B13C624D6 for ; Sat, 5 Sep 2026 15:47:57 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender:List-Subscribe:List-Help :List-Post:List-Archive:List-Unsubscribe:List-Id:Content-Transfer-Encoding: MIME-Version:Message-Id:Date:Subject:Cc:To:From:Reply-To:Content-Type: Content-ID:Content-Description:Resent-Date:Resent-From:Resent-Sender: Resent-To:Resent-Cc:Resent-Message-ID:In-Reply-To:References:List-Owner; bh=YDDpwBogn8o6pQZ7QySkCWhHGi/gThTcBwx20NL7VvQ=; b=gRGLsPI18iJjoMkxIV7HM7aTQw C4HYWS/q55AQC8a+wDWKyt02IE39PAg92FAow5jt2eBIIulEDX3E/KAj8Bn85L+2mtdGjDp6dK4LJ Z3mBPav0BlAs2JjA/boBRqkATgaWwqfo5+gpOP4aqYL+r3a4xyC+7uVlMs+kMwxAXog2LO8kejz8F CDqHZ57HHI35tUtBhcROaqJml6K1mEqEralzlVetasKYw89pzTwXnLvykCvjqd4OYF/afjzT+GvP7 jzFYBjKN91KwYwSirQe2SnhviXSTJbSJ7uA+FQSBCyZoDn0qPE7NwPNhzkn8cdyenQEXYFEKdMfEr ak9PYRQQ==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.99.1 #2 (Red Hat Linux)) id 1x2scM-00000004Ere-3Fgm; Sat, 05 Sep 2026 15:47:46 +0000 Received: from m16.mail.163.com ([220.197.31.4]) by bombadil.infradead.org with esmtps (Exim 4.99.1 #2 (Red Hat Linux)) id 1x2scJ-00000004EqF-2Yiu for linux-arm-kernel@lists.infradead.org; Sat, 05 Sep 2026 15:47:45 +0000 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=163.com; s=s110527; h=From:To:Subject:Date:Message-Id:MIME-Version; bh=YD DpwBogn8o6pQZ7QySkCWhHGi/gThTcBwx20NL7VvQ=; b=Npi1FTRFEQCNK/nAlw BoUxQq00no77jA2w5gyY/GChiQgVq4Fcax9bU5kIPbLqwylxJI5+0rzS3IV5fIQY up4b3j2fm+fhbJu1rQgNXp/lKUlrlgUrqb1eQmo2pO1aF+/Tl/lEWpqku6eUA3JU XIQ+FlZPc87C7pc3qXv5Ydztc= Received: from 4CV529F122.company.local (unknown []) by gzga-smtp-mtada-g1-4 (Coremail) with SMTP id _____wD3t3BwOZxqmpCdAw--.21992S2; Sat, 05 Sep 2026 23:47:08 +0800 (CST) From: Ding Hui To: andrew@lunn.ch, Maxime Chevallier , Andrew Lunn , "David S. Miller" , Eric Dumazet , Jakub Kicinski , Paolo Abeni , Maxime Coquelin , Alexandre Torgue , netdev@vger.kernel.org (open list:STMMAC ETHERNET DRIVER), linux-stm32@st-md-mailman.stormreply.com (moderated list:ARM/STM32 ARCHITECTURE), linux-arm-kernel@lists.infradead.org (moderated list:ARM/STM32 ARCHITECTURE), linux-kernel@vger.kernel.org (open list) Cc: dinghui@lixiang.com, xiasanbo@lixiang.com, yangchen11@lixiang.com, liuxuanjun@lixiang.com Subject: [PATCH net-next v2] net: stmmac: fix error path cleanup in DMA descriptor ring allocation Date: Sat, 5 Sep 2026 23:46:47 +0800 Message-Id: <20260905154654.1725313-1-dinghui1111@163.com> X-Mailer: git-send-email 2.34.1 MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-CM-TRANSID: _____wD3t3BwOZxqmpCdAw--.21992S2 X-Coremail-Antispam: 1Uf129KBjvJXoW3Gr1rXr1ktFW5uw17WryUJrb_yoWxGr4rpF 48Cw4qkryjqr13Ga1DJw48X3W5AayFyr4YgFWIgws7uFnIkryFgFyUAryj9rW8CrykuF1f K398C3Z8AF1UJrUanT9S1TB71UUUUU7qnTZGkaVYY2UrUUUUjbIjqfuFe4nvWSU5nxnvy2 9KBjDUYxBIdaVFxhVjvjDU0xZFpf9x07bbrcfUUUUU= X-CM-SenderInfo: pglqwx1xlriiqr6rljoofrz/xtbC8xwvW2qcOXx5lQAA3E X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.9.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20260905_084744_175750_FEFFD1B9 X-CRM114-Status: GOOD ( 16.08 ) X-BeenThere: linux-arm-kernel@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: "linux-arm-kernel" Errors-To: linux-arm-kernel-bounces+linux-arm-kernel=archiver.kernel.org@lists.infradead.org From: Ding Hui __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. Signed-off-by: Ding Hui --- 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 | 59 ++++++++++++++++--- 1 file changed, 50 insertions(+), 9 deletions(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c index f2fc89176654..f0e06c011b8d 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++) @@ -2272,15 +2284,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 +2312,27 @@ 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: + if (priv->extend_desc) + dma_free_coherent(priv->device, size, rx_q->dma_erx, + rx_q->dma_rx_phy); + else + dma_free_coherent(priv->device, size, rx_q->dma_rx, + 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 +2385,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 +2402,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, -- 2.34.1