Re: [PATCH net 7/7] selftests/xsk: account invalid multi-buffer Tx descriptors

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Buffer leak in the generic (SKB) TX path when the first fragment of an invalid AF_XDP multi-buffer packet is rejected.
--

commit aba0a0e03b19503a4e6b7748da982e4082aebe45
Author: Maciej Fijalkowski <maciej.fijalkowski@intel.com>

selftests/xsk: account invalid multi-buffer Tx descriptors

This commit updates the af_xdp selftests to correctly account for invalid
middle fragments in multi-buffer tx packets. It ensures the whole invalid
packet is counted as outstanding in verbatim mode and appends explicit end
fragments to verify the kernel's drain logic.

> diff --git a/tools/testing/selftests/bpf/prog_tests/test_xsk.c b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> index de1e63c3fdf63..d8a1c0d40e5af 100644
> --- a/tools/testing/selftests/bpf/prog_tests/test_xsk.c
> +++ b/tools/testing/selftests/bpf/prog_tests/test_xsk.c
[ ... ]
> @@ -2104,13 +2107,16 @@ int testapp_invalid_desc_mb(struct test_spec *test)
>  		{0, 0, 0, false, 0},
>  		/* Invalid address in the second frame */
>  		{0, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
> -		{umem_sz, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
> +		{umem_sz * 2, XSK_UMEM__LARGE_FRAME_SIZE, 0, false, XDP_PKT_CONTD},
> +		{0, MIN_PKT_SIZE, 0, false, 0},

[Severity: High]
This isn't a regression introduced by this patch, but testing invalid
multi-buffer packets like this appears to expose a pre-existing buffer leak
in the generic (skb) tx path.

If an application sends a multi-buffer packet where the first fragment is
invalid (e.g. an out-of-bounds address), does __xsk_generic_xmit() in
net/xdp/xsk.c permanently leak the buffer?

In net/xdp/xsk.c:__xsk_generic_xmit():

        if (xskq_has_descs(xs->tx)) {
                bool reclaim_desc = xs->skb || xs->drain_cont;
                ...
                if (reclaim_desc)
                        xsk_cq_submit_addr_single_locked(xs->pool, &desc);

                xskq_cons_release(xs->tx);
                xs->drain_cont = xp_mb_desc(&desc);
        }

When the first fragment is invalid, xs->skb is null and xs->drain_cont is
false, making reclaim_desc evaluate to false.

The descriptor is then consumed from the tx ring via xskq_cons_release(), but
since it is never submitted to the completion queue, the buffer seems to be
permanently leaked, potentially leading to umem exhaustion.

[ ... ]

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