netdev - Re: fix a potential memory leak in stmmac_xdp_enable

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <20260127030447.3838706-1-kuba@kernel.org>
Date: Mon, 26 Jan 2026 19:04:47 -0800
From: Jakub Kicinski <kuba@...nel.org>
To: lihaoxiang@...c.iscas.ac.cn
Cc: Jakub Kicinski <kuba@...nel.org>,
	andrew+netdev@...n.ch,
	linux-kernel@...r.kernel.org,
	sdf@...ichev.me,
	bpf@...r.kernel.org,
	ast@...nel.org,
	stable@...r.kernel.org,
	netdev@...r.kernel.org,
	john.fastabend@...il.com,
	mcoquelin.stm32@...il.com,
	linux-arm-kernel@...ts.infradead.org,
	daniel@...earbox.net,
	linux-stm32@...md-mailman.stormreply.com,
	boon.leong.ong@...el.com,
	hawk@...nel.org,
	davem@...emloft.net,
	pabeni@...hat.com,
	edumazet@...gle.com,
	alexandre.torgue@...s.st.com
Subject: Re: fix a potential memory leak in stmmac_xdp_enable_pool()

This is an AI-generated review of your patch. The human sending this
email has considered the AI review valid, or at least plausible.

For local reproduction steps see https://netdev-ai.bots.linux.dev/ai-local.html
---
fix a potential memory leak in stmmac_xdp_enable_pool()

This patch adds xsk_pool_dma_unmap() to the error path when
stmmac_xsk_wakeup() fails, to release the DMA mapping allocated
earlier by xsk_pool_dma_map().

> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> index d7e4db7224b0..7d89d6066ea3 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c
> @@ -48,8 +48,10 @@ static int stmmac_xdp_enable_pool(struct stmmac_priv *priv,
>  		napi_enable(&ch->rxtx_napi);
>
>  		err = stmmac_xsk_wakeup(priv->dev, queue, XDP_WAKEUP_RX);
> -		if (err)
> +		if (err) {
> +			xsk_pool_dma_unmap(pool, STMMAC_RX_DMA_ATTR);
>  			return err;
> +		}

Is the error cleanup here complete? Looking at stmmac_xdp_enable_pool(),
before this point the code has already executed:

  set_bit(queue, priv->af_xdp_zc_qps);
  stmmac_enable_rx_queue(priv, queue);
  stmmac_enable_tx_queue(priv, queue);
  napi_enable(&ch->rxtx_napi);

When stmmac_xsk_wakeup() fails, the patch unmaps the DMA pool but does not
appear to reverse these other state changes. Comparing with the cleanup
sequence in stmmac_xdp_disable_pool(), should the error path also:

  - call clear_bit(queue, priv->af_xdp_zc_qps)
  - disable the queues and NAPI that were just enabled

The DMA descriptors allocated by stmmac_enable_rx_queue() and
stmmac_enable_tx_queue() would also be leaked on this error path.

>  	}
>
>  	return 0;
-- 
pw-bot: cr