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

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

[Fan Wu]

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@vger.kernel.org
Assisted-by: Codex:gpt-5.5
Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
---

 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);

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@vger.kernel.org
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
> ---
> 

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);
> 
> 

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

[Fan Wu]

Hi Jacob,

Thanks for the review. I agree this is a rare allocation-failure path.

The leak is nevertheless deterministic from the ownership contract and
the driver's control flow. __build_skb() documents that, on failure, it
returns NULL without freeing the caller-provided data.

hip04_rx_poll() currently jumps to refill before dma_unmap_single().
Refill then replaces rx_buf[rx_head] and rx_phys[rx_head], so the old
fragment and its streaming DMA mapping are no longer reachable. The
stop and free-ring paths only operate on entries still referenced by
those arrays.

The fix leaves the slot, descriptor, rx_head, and rx_cnt_remaining
unchanged, and returns budget. This tells NAPI that RX work remains
outstanding and retries build_skb() using the same buffer. The additional
skb_free_frag() handles the separate case where a replacement fragment
was allocated but dma_map_single() failed before it was installed.

I hope this clarifies why the failure path is a deterministic leak even
though I do not have a dynamic reproduction on HIP04 hardware.

Thanks,
Fan

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

[Jacob Keller]

On 7/15/2026 12:30 AM, Fan Wu wrote:
> Hi Jacob,
> 
> Thanks for the review. I agree this is a rare allocation-failure path.
> 
> The leak is nevertheless deterministic from the ownership contract and
> the driver's control flow. __build_skb() documents that, on failure, it
> returns NULL without freeing the caller-provided data.
> 
> hip04_rx_poll() currently jumps to refill before dma_unmap_single().
> Refill then replaces rx_buf[rx_head] and rx_phys[rx_head], so the old
> fragment and its streaming DMA mapping are no longer reachable. The
> stop and free-ring paths only operate on entries still referenced by
> those arrays.
> 
> The fix leaves the slot, descriptor, rx_head, and rx_cnt_remaining
> unchanged, and returns budget. This tells NAPI that RX work remains
> outstanding and retries build_skb() using the same buffer. The additional
> skb_free_frag() handles the separate case where a replacement fragment
> was allocated but dma_map_single() failed before it was installed.
> 
> I hope this clarifies why the failure path is a deterministic leak even
> though I do not have a dynamic reproduction on HIP04 hardware.
> 

Thanks,
Jake

> Thanks,
> Fan
> 

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@vger.kernel.org
> Assisted-by: Codex:gpt-5.5
> Signed-off-by: Fan Wu <fanwu01@zju.edu.cn>
> ---
> 

Reviewed-by: Jacob Keller <jacob.e.keller@intel.com>