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

["Björn Töpel" <bjorn@kernel.org>]

Waking a dead thread: RISE FROM YOUR GRAVE!

Clement Lecigne <clecigne@google.com> writes:

> From: Clément Lecigne <clecigne@google.com>
>
> 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?


Björn