Re: [PATCH v3] xsk: fix memory corruptions in net/core/xdp.c

[Björn Töpel]

Maciej Fijalkowski <maciej.fijalkowski@xxxxxxxxx> writes:

> On Fri, Oct 16, 2026 at 01:19:35PM +0200, Björn Töpel wrote:
>> Waking a dead thread: RISE FROM YOUR GRAVE!
>> 
>> Clement Lecigne <clecigne@xxxxxxxxxx> writes:
>> 
>> > From: Clément Lecigne <clecigne@xxxxxxxxxx>
>> >
>> > Commit 560d958c6c68 ("xsk: add generic XSk &xdp_buff -> skb conversion")
>> > introduced a vulnerability in the handling of XDP_PASS for AF_XDP zero-copy
>> > frames.
>> 
>> Hmm, I acually hit this in my WIP for UMEM page pool provider hack.
>> 
>> > Note: Currently, this specific AF_XDP zero-copy conversion path is only
>> > reachable from the drivers/net/ethernet/intel/ice and
>> > drivers/net/ethernet/intel/idpf drivers.
>> >
>> > When building an skb, xdp_build_skb_from_zc() uses the chunk size
>> > (xdp->frame_sz) for the allocation. However, napi_build_skb() automatically
>> > reserves space at the end of the allocation for the skb_shared_info
>> > structure. 
>> >
>> > Most high performance UMEM applications use 4K chunks, where the
>> > corruption cannot happen.
>> 
>> I think this can also happen with 4K UMEM chunks.
>> 
>> For example, on ice, use MTU 3800, 4K chunks, and no extra UMEM
>> headroom. Attach an xdp.frags program that returns XDP_PASS, and bind
>> the socket without XDP_USE_SG.
>> 
>> A 3814-byte Ethernet frame fits in the UMEM chunk:
>> 
>>   256 headroom + 3814 packet = 4070 bytes
>> 
>> But the copied skb also needs space for skb_shared_info. Using
>> the 320-byte size from your example:
>> 
>>   256 headroom + 3814 packet + 320 shared info = 4390 bytes
>> 
>> The old code allocates only 4096 bytes, so the packet copy
>> overwrites skb_shared_info.
>> 
>> Am I missing something?
>
> I was pushing back on relying on scenario above as we respect the tailroom
> now since some time:
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=a315e022a72d95ef5f1d4e58e903cb492b0ad931
>
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=1ee1605138fc94cc8f8f273321dd2471c64977f9

Hmm, isn't that only with XDP_USE_SG? My example binds without it.

Then xsk_pool_get_rx_frame_size() returns 3840. XDP_PASS still builds an
skb with skb_shared_info, even if it's linear. So we only have 4096 -
256 - 320 = 3520 bytes for the packet.

Or am I missing another check?