Re: [PATCH RFC net-next v4 01/14] xsk: introduce XDP_GENERIC_XMIT_BATCH setsockopt

[sashiko-bot@kernel.org]

Sashiko AI review found 1 potential issue(s):
- [High] Unsafe array allocation using kmalloc for a potentially large, user-controlled size
--

commit 16fdf26210767cd884154d2717d7894d96a2ee54
Author: Jason Xing <kernelxing@tencent.com>
xsk: introduce XDP_GENERIC_XMIT_BATCH setsockopt

This commit introduces the XDP_GENERIC_XMIT_BATCH setsockopt to allow
batch processing in copy mode, attempting to improve throughput by 
allocating and initializing multiple skbs at one time.

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 3b46bc635c432..1194268ae8676 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
[ ... ]
> @@ -1582,6 +1593,42 @@ static int xsk_setsockopt(struct socket *sock, int level, int optname,
>  		WRITE_ONCE(xs->max_tx_budget, budget);
>  		return 0;
>  	}
> +	case XDP_GENERIC_XMIT_BATCH:
> +	{
> +		struct xsk_buff_pool *pool = xs->pool;
> +		struct xsk_batch *batch = &xs->batch;
> +		struct xdp_desc *descs;
> +		struct sk_buff **skbs;
> +		unsigned int size;
> +		int ret = 0;
> +
> +		if (optlen != sizeof(size))
> +			return -EINVAL;
> +		if (copy_from_sockptr(&size, optval, sizeof(size)))
> +			return -EFAULT;
> +		if (size == batch->generic_xmit_batch)
> +			return 0;
> +		if (!size || size > xs->max_tx_budget || !pool)
> +			return -EACCES;
> +
> +		mutex_lock(&xs->mutex);
> +		skbs = kmalloc(size * sizeof(struct sk_buff *), GFP_KERNEL);

Can the requested size here become large enough to trigger high-order
allocation failures under memory fragmentation?

Since size is bounded by xs->max_tx_budget, which follows the TX ring
entries, it looks like this could reach up to 65536 on some configurations.
A 512KB contiguous allocation via kmalloc might fail on long-running
systems and trigger page allocation warnings in the kernel log.

Also, since size is a user-provided value bounded only by the maximum
budget, could the open-coded multiplication overflow on 32-bit platforms?

Would it be safer to use kvmalloc_array() here to prevent both integer
overflows and the dependency on physically contiguous memory, similar to
how kvcalloc() is used for the descs array right below it?

> +		if (!skbs) {
> +			ret = -ENOMEM;
> +			goto out;
> +		}
> +		descs = kvcalloc(size, sizeof(struct xdp_desc), GFP_KERNEL);
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260415082654.21026-1-kerneljasonxing@gmail.com?part=1