Re: [PATCH net v2] bnxt: fix head underflow on XDP head-grow

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] If `bp->rx_skb_func()` fails, `bnxt_rx_pkt()` skips aggregate buffer cleanup, permanently leaking `rx_agg_ring` hardware slots and `netmem` references.
--

commit 374adfb9335a0a96705993cee5701e4d6ce2423c
Author: Joe Damato <joe@dama.to>

bnxt: fix head underflow on XDP head-grow

This commit fixes a head underflow bug in bnxt_rx_multi_page_skb() when
handling XDP adjust head grow cases. It accomplishes this by adding an offset
field to struct bnxt_sw_rx_bd and using it to compute the correct fragment
start address.

> diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> index 35e1f8f663c78..12e73ffff6c3d 100644
> --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c
> +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c

[ ... ]

> @@ -1185,13 +1191,13 @@ static struct sk_buff *bnxt_rx_multi_page_skb(struct bnxt *bp,
>  	dma_addr -= bp->rx_dma_offset;
>  	dma_sync_single_for_cpu(&bp->pdev->dev, dma_addr, rxr->rx_page_size,
>  				bp->rx_dir);
> -	skb = napi_build_skb(data_ptr - bp->rx_offset, rxr->rx_page_size);
> +	skb = napi_build_skb(frag_start, rxr->rx_page_size);
>  	if (!skb) {
>  		page_pool_recycle_direct(rxr->page_pool, page);
>  		return NULL;

[Severity: High]
This isn't a bug introduced by this patch, but does returning NULL here
leak aggregate buffers in bnxt_rx_pkt()?

Looking at bnxt_rx_pkt(), when bp->rx_skb_func() (which maps to
bnxt_rx_multi_page_skb() in page mode) returns NULL, the code jumps
to oom_next_rx:

drivers/net/ethernet/broadcom/bnxt/bnxt.c:bnxt_rx_pkt()
	skb = bp->rx_skb_func(bp, rxr, cons, data, data_ptr, dma_addr,
			      payload | len);
	if (!skb)
		goto oom_next_rx;

If the packet had aggregate buffers (agg_bufs > 0), this bypasses the
cleanup block later in the function that would recycle these buffers via
bnxt_reuse_rx_agg_bufs().

Can this omission cause a permanent leak of hardware aggregate slots and
netmem references, eventually wedging the RX ring under memory pressure?

>  	}
>  	skb_mark_for_recycle(skb);
> -	skb_reserve(skb, bp->rx_offset);
> +	skb_reserve(skb, data_ptr - (u8 *)frag_start);
>  	__skb_put(skb, len);
>  
>  	return skb;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260609204458.2237787-2-joe@dama.to?part=1