Re: [PATCH net] net: hip04: fix RX buffer leak on build_skb failure

[Jacob Keller]

On 7/12/2026 7:27 AM, Fan Wu wrote:
> When build_skb() fails in hip04_rx_poll(), the driver jumps to the
> refill path without releasing the current RX buffer and its DMA mapping.
> Installing a replacement buffer then overwrites the slot references and
> leaks both resources.
> 
> Keep the current slot intact and return budget so NAPI retries the same
> buffer.  Also free a newly allocated RX fragment when dma_map_single()
> fails.
> 
> This issue was found by an in-house static analysis tool.
> 
> Fixes: 701a0fd52318 ("hip04_eth: fix missing error handle for build_skb failed")
> Cc: stable@xxxxxxxxxxxxxxx
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Fan Wu <fanwu01@xxxxxxxxxx>
> ---
> 

You say this was found by static analysis. I imagine that build_skb
rarely fails (without some sort of fault injection). That means this is
likely difficult to reproduce in practice. I know we've been trying to
err on the side of increasing the burden of proof on AI-assisted fixes
like this.

Based on a quick search, it does seem that this drivers response of only
logging a debug message is not correct...

>  drivers/net/ethernet/hisilicon/hip04_eth.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/hisilicon/hip04_eth.c b/drivers/net/ethernet/hisilicon/hip04_eth.c
> index 18376bcc7..1d03039b4 100644
> --- a/drivers/net/ethernet/hisilicon/hip04_eth.c
> +++ b/drivers/net/ethernet/hisilicon/hip04_eth.c
> @@ -594,7 +594,10 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
>  		skb = build_skb(buf, priv->rx_buf_size);
>  		if (unlikely(!skb)) {
>  			net_dbg_ratelimited("build_skb failed\n");
> -			goto refill;
> +			/* Retain the slot; return budget so NAPI retries this buffer.
> +			 * Refill would overwrite rx_buf[]/rx_phys[] and leak them.
> +			 */
> +			return budget;
>  		}
>  
>  		dma_unmap_single(priv->dev, priv->rx_phys[priv->rx_head],
> @@ -622,14 +625,15 @@ static int hip04_rx_poll(struct napi_struct *napi, int budget)
>  			rx++;
>  		}
>  
> -refill:
>  		buf = netdev_alloc_frag(priv->rx_buf_size);
>  		if (!buf)
>  			goto done;
>  		phys = dma_map_single(priv->dev, buf,
>  				      RX_BUF_SIZE, DMA_FROM_DEVICE);
> -		if (dma_mapping_error(priv->dev, phys))
> +		if (dma_mapping_error(priv->dev, phys)) {
> +			skb_free_frag(buf);
>  			goto done;
> +		}
>  		priv->rx_buf[priv->rx_head] = buf;
>  		priv->rx_phys[priv->rx_head] = phys;
>  		hip04_set_recv_desc(priv, phys);
> 
>