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

sashiko-bot
2026-04-15T08:51:38Z

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