Re: [PATCH v4 net 3/6] xsk: provide sufficient space in pool->tx_descs

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [Critical] Heap Buffer Overflow in batched Tx descriptor API when a shared XDP socket has a larger Tx ring.
--

--- Patch [3]: [PATCH v4 net 3/6] xsk: provide sufficient space in pool->tx_descs ---
commit 16fadec5330bdab35c81e060269bbbba543bb7cd
Author: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

xsk: provide sufficient space in pool->tx_descs

The temporary Tx descriptor array in an XSK buffer pool is sized from the Tx
ring of the socket that creates the pool. This commit sizes the temporary
descriptor array to the larger of the first Tx ring and the device's
xdp_zc_max_segs capability to prevent stalls in batch parsing for shared-UMEM.

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index 12a845d012f6f..091792d1d82de 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
[ ... ]
> @@ -1557,7 +1558,8 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
>  			 * utilizes
>  			 */
>  			if (xs->tx && !xs->pool->tx_descs) {
> -				err = xp_alloc_tx_descs(xs->pool, xs);
> +				err = xp_alloc_tx_descs(xs->pool, xs,
> +							dev->xdp_zc_max_segs);

[Severity: Critical]
This is a pre-existing issue, but does this logic leave the system vulnerable
to a heap buffer overflow?

When a primary socket is configured with a small Tx ring, pool->tx_descs is
allocated with a small capacity. If a secondary shared-UMEM socket is then
bound with a much larger Tx ring, this check skips reallocating or resizing
pool->tx_descs.

Later, during batch reading, xsk_tx_peek_release_desc_batch() can overflow
this array:

net/xdp/xsk.c:xsk_tx_peek_release_desc_batch()
    ...
    nb_pkts = xskq_cons_read_desc_batch(xs->tx, pool, nb_pkts);
    ...

Since nb_pkts is not clamped to pool->tx_descs_nentries, the batch size
can exceed the allocated capacity. This leads to an out-of-bounds write
when xskq_cons_read_desc_batch() copies descriptors:

net/xdp/xsk_queue.h:xskq_cons_read_desc_batch()
    ...
    descs[nb_entries] = ring->desc[idx];
    ...

The same out-of-bounds write can occur in the fallback path:

net/xdp/xsk.c:xsk_tx_peek_release_fallback()
    ...
    while (nb_pkts < max_entries && xsk_tx_peek_desc(pool, &descs[nb_pkts]))
        nb_pkts++;
    ...

Since max_entries isn't clamped to the actual allocated capacity, couldn't
this corrupt adjacent kernel memory?

>  				if (err) {
>  					xp_put_pool(xs->pool);
>  					xs->pool = NULL;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260719135609.147823-1-maciej.fijalkowski@intel.com?part=3