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

[Michael Chan]

On Tue, Jun 9, 2026 at 1:45 PM Joe Damato <joe@xxxxxxx> wrote:
>
> The xdp.py test test_xdp_native_adjst_head_grow_data crashes when run on
> a bnxt machine (and also crashes in NIPA).
>
> It seems that the bug is an underflow in bnxt_rx_multi_page_skb, which
> builds the skb head:
>
>   napi_build_skb(data_ptr - bp->rx_offset, rxr->rx_page_size);
>
> The problem with this expression is that in page mode, rx_offset is:
>
>   bp->rx_offset = NET_IP_ALIGN + XDP_PACKET_HEADROOM;
>
> Which evaluates (at least on x86_64) to 258.
>
> The test test_xdp_native_adjst_head_grow_data tests a case where the
> head is adjusted by -256.
>
> When this test runs, data_ptr is shifted to frag_start + 2 (where
> frag_start = page_address(page) + offset).
>
> Then, bnxt_rx_multi_page_skb is invoked and the napi_build_skb
> expression subtracts 258, landing at an address before frag_start. This
> could be either the previous fragment or the previous physical page when
> the offset is < 256 (e.g. if the fragment started at offset 0).
>
> When the skb is freed, the page pool fragment reference is dropped on
> either the wrong page or the wrong frag of the right page. In either
> case, the corrupted reference count can lead to the page being
> prematurely recycled while still in use. Once (incorrectly) recycled, it
> can be handed out again and on driver teardown this would result in a
> double free.
>
> The commit under fixes updated this code to handle the case where the
> native page size is >= 64k, but it unintentionally broke the head grow
> case.
>
> To fix this, add an offset field to struct bnxt_sw_rx_bd, mirroring the
> existing offset field in struct bnxt_sw_rx_agg_bd. Populate it on
> allocation and preserve it on reuse.
>
> In bnxt_rx_multi_page_skb, use the newly added offset field to compute
> the fragment start and pass that to napi_build_skb. Adjust the layout
> with skb_reserve.
>
> There are two cases, the non-adjustment case and the adjustment case.
>
> In both cases, the skb is built at page_address(page) + offset to
> account for the case where the native page size >= 64K and skb_reserve
> is called with data_ptr - (page_address(page) + offset). That
> difference equals bp->rx_offset when data_ptr was not moved, or
> bp->rx_offset + xdp_adjust when XDP adjusted the head.
>
> Re-running the failing test with this commit applied causes the test to
> run successfully to completion.
>
> The other rx_skb_func implementations don't have this issue.
>
> Fixes: f6974b4c2d8e ("bnxt_en: Fix page pool logic for page size >= 64K")
> Signed-off-by: Joe Damato <joe@xxxxxxx>

Reviewed-by: Michael Chan <michael.chan@xxxxxxxxxxxx>

As a side note, struct bnxt_tpa_info is similar to struct
bnxt_sw_rx_bd, but it doesn't require the offset field because we
always allocate frags for TPA.  Thanks.

Attachment: smime.p7s
Description: S/MIME Cryptographic Signature