Re: [PATCH net 5/7] xsk: reclaim invalid multi-buffer Tx descs in ZC path

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Heap Buffer Overflow (OOB Write) in `xsk_tx_peek_release_fallback()` due to array size mismatch in shared UMEM configuration.
--

commit 6ddf04bf3e0869241f35a9093870aefb3a5e86f2
Author: Maciej Fijalkowski <maciej.fijalkowski@intel.com>
Subject: xsk: reclaim invalid multi-buffer Tx descs in ZC path

This commit addresses an issue in the zero-copy Tx batching path where
multi-buffer packets with an invalid descriptor could leave earlier
descriptors consumed but unreturned to userspace. It handles invalid
multi-buffer packets as a unit, queuing reclaim-only descriptors to the
completion ring, and gates the batched path while a same-pool bind waits
for pre-existing readers.

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -576,9 +588,20 @@ static u32 xsk_tx_peek_release_fallback(struct xsk_buff_pool *pool, u32 max_entr
>  
>  u32 xsk_tx_peek_release_desc_batch(struct xsk_buff_pool *pool, u32 nb_pkts)
>  {
> +	struct xsk_tx_batch batch = {};
>  	struct xdp_sock *xs;
> +	u32 cq_cached_prod;
>  
>  	rcu_read_lock();
> +
> +	/* Pairs with the release stores in xp_prepare_xsk_tx_share() and
> +	 * xp_finish_xsk_tx_share(). If bind is converting a singular Tx pool
> +	 * to shared, do not enter the singular batched path.
> +	 */
> +	if (smp_load_acquire(&pool->tx_share_pending))
> +		goto out;
> +	if (unlikely(pool->reclaim_descs))
> +		goto out;
>  	if (!list_is_singular(&pool->xsk_tx_list)) {
>  		/* Fallback to the non-batched version */
>  		rcu_read_unlock();

[Severity: High]
This is a pre-existing issue, but can the fallback path overwrite the
pool->tx_descs array?

In xsk_bind(), when a shared pool is created, pool->tx_descs is allocated
using xp_alloc_tx_descs() based on the first socket's nentries. If the first
socket is configured with a very small TX ring (for example, 2 entries),
pool->tx_descs is sized to match.

If a second socket with a larger TX ring is then bound to the same shared
pool, pool->tx_descs is not reallocated to accommodate the larger capacity.

When the batched path sees multiple sockets and falls back to the non-batched
version:

static u32 xsk_tx_peek_release_fallback(struct xsk_buff_pool *pool, u32 max_entries)
{
	struct xdp_desc *descs = pool->tx_descs;
	u32 nb_pkts = 0;

	while (nb_pkts < max_entries && xsk_tx_peek_desc(pool, &descs[nb_pkts]))
		nb_pkts++;
    ...
}

Does this loop write up to max_entries (the driver's budget, typically 64)
into the descs array, causing an out-of-bounds write past the initial small
allocation limit?

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