Re: [PATCH AUTOSEL 6.18] xsk: tighten UMEM headroom validation to account for tailroom and min frame

[sashiko-bot@kernel.org]

Sashiko AI review found 2 potential issue(s):
- [High] The commit introduces a UAPI backward compatibility breakage and makes UMEM headroom limits unpredictable for userspace.
- [High] Permanent `locked_vm` accounting leak during short pins in `xdp_umem_pin_pages()`.
--

commit 39b8ee9e55bd1dabb902c8f19155e836abf3b229
Author: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

xsk: tighten UMEM headroom validation to account for tailroom and min frame

This commit modifies the headroom validation in xdp_umem_reg() to reserve
128 bytes for the minimum ethernet frame size and enough tailroom for
skb_shared_info. This prevents potential corruption at the end of
multi-buffer XSK frames.

Link: https://patch.msgid.link/20260402154958.562179-2-maciej.fijalkowski@intel.com

> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index 9f76ca591d54f..9ec7bd948acc7 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c

[ ... ]

> @@ -202,7 +202,8 @@ static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
>  	if (!unaligned_chunks && chunks_rem)
>  		return -EINVAL;
>  
> -	if (headroom >= chunk_size - XDP_PACKET_HEADROOM)
> +	if (headroom > chunk_size - XDP_PACKET_HEADROOM -
> +		       SKB_DATA_ALIGN(sizeof(struct skb_shared_info)) - 128)
>  		return -EINVAL;
>  
>  	if (mr->flags & XDP_UMEM_TX_METADATA_LEN) {

Does this change make the maximum allowed headroom unpredictable for userspace?

Since sizeof(struct skb_shared_info) and SKB_DATA_ALIGN depend on kernel
internal structures and architecture-specific cache line sizes, userspace
applications do not have access to these values to deterministically
calculate the limit.

Could this also break existing userspace applications that legitimately
configured larger headrooms but do not enable multi-buffer later on?

While looking at the surrounding code in xdp_umem_reg(), is there an existing
locked_vm accounting leak in the error paths?

When xdp_umem_reg() attempts to pin pages, it first accounts them:

	err = xdp_umem_account_pages(umem);
	if (err)
		return err;

	err = xdp_umem_pin_pages(umem, (unsigned long)addr);
	if (err)
		goto out_account;

If pin_user_pages() successfully pins some, but not all pages,
xdp_umem_pin_pages() updates umem->npgs to the smaller count before returning
an error:

	npgs = pin_user_pages(address, umem->npgs,
			      gup_flags | FOLL_LONGTERM, &umem->pgs[0]);
	...
	if (npgs != umem->npgs) {
		if (npgs >= 0) {
			umem->npgs = npgs;
			err = -ENOMEM;
			goto out_pin;
		}

Then xdp_umem_reg() jumps to out_account and calls
xdp_umem_unaccount_pages(umem). This subtracts the newly modified, smaller
umem->npgs from umem->user->locked_vm, which appears to permanently leak the
difference in the user's locked_vm counter.

Can this be repeatedly triggered by passing ranges with unmapped holes to
exhaust the memory lock limit?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260420132314.1023554-143-sashal@kernel.org?part=1