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

On Wed, Apr 15, 2026 at 4:51 PM <[email protected]> wrote:
>
> 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 <[email protected]>
> 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?

kvmalloc_array() indeed makes more sense. Will update it.

Thanks,
Jason