[PATCH] fix a potential memory leak in stmmac_xdp_enable_pool()

[PATCH] fix a potential memory leak in stmmac_xdp_enable_pool()

[Haoxiang Li]

In stmmac_xdp_enable_pool(), if stmmac_xsk_wakeup() fails,
the memory allocated by xsk_pool_dma_map() should be released.
Add xsk_pool_dma_unmap() to do so.

Fixes: bba2556efad6 ("net: stmmac: Enable RX via AF_XDP zero-copy")
Cc: stable@vger.kernel.org
Signed-off-by: Haoxiang Li <lihaoxiang@isrc.iscas.ac.cn>
---
 drivers/net/ethernet/stmicro/stmmac/stmmac_xdp.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

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;
+		}
 	}
 
 	return 0;
-- 
2.25.1

Re: fix a potential memory leak in stmmac_xdp_enable_pool()

[Jakub Kicinski]

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