Re: [PATCH net v5 1/8] xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Unconditional rejection of `XDP_UMEM_TX_SW_CSUM` breaks valid native zero-copy configurations.
- [High] The root cause of the 'wedged TX ring' remains unaddressed, allowing trivial userspace triggers.
--

commit a1862dee263d88bc3b6ca0898f858a88e2e37fce
Author: Jason Xing <kerneljasonxing@gmail.com>

xsk: reject sw-csum UMEM binding to IFF_TX_SKB_NO_LINEAR devices

This commit attempts to prevent silent per-packet failures and wedged TX
rings when using sw-csum with drivers advertising IFF_TX_SKB_NO_LINEAR.
It does so by rejecting the bind operation with -EOPNOTSUPP during setup.

Closes: https://lore.kernel.org/all/20260419045822.843BFC2BCAF@smtp.kernel.org/#t
Fixes: 30c3055f9c0d ("xsk: wrap generic metadata handling onto separate function")
Acked-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Jason Xing <kernelxing@tencent.com>

> Without the patch, every descriptor carrying 'XDP_TX_METADATA |
> XDP_TXMD_FLAGS_CHECKSUM' produces:
> 1) a WARN_ONCE "offset (N) >= skb_headlen() (0)" from skb_checksum_help(),
> 2) sendmsg() returning -EINVAL without consuming the descriptor
>    (invalid_descs is not incremented),
> 3) a wedged TX ring: __xsk_generic_xmit() does not advance the
>     consumer on non-EOVERFLOW errors, so the next sendmsg() re-reads
>     the same descriptor and re-hits the same WARN until the socket
>     is closed.

Does this completely resolve the wedged TX ring issue? While this patch
prevents skb_checksum_help() from returning -EINVAL, xsk_skb_metadata()
can still return -EINVAL for other reasons, such as an out-of-bounds
csum_start or csum_offset provided by userspace.

If xsk_build_skb() returns -EINVAL, wouldn't __xsk_generic_xmit() still
evaluate if (err != -EOVERFLOW) goto out; and break its loop without
consuming the descriptor? Might the generic transmit path need to be
updated to consume the descriptor and drop the packet on terminal errors
like -EINVAL to fully prevent userspace from wedging the ring?

> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index cd7bc50872f6..d981cfdd8535 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -175,6 +175,9 @@ int xp_assign_dev(struct xsk_buff_pool *pool,
>  	if (force_zc && force_copy)
>  		return -EINVAL;
>  
> +	if (pool->tx_sw_csum && (netdev->priv_flags & IFF_TX_SKB_NO_LINEAR))
> +		return -EOPNOTSUPP;
> +

Will this check unconditionally reject valid native zero-copy configurations?

In native zero-copy mode (when force_zc is set), the transmission path
bypasses __xsk_generic_xmit() entirely and processes descriptors natively,
meaning it does not use xsk_build_skb_zerocopy() or invoke
skb_checksum_help().

If a UMEM legitimately has the XDP_UMEM_TX_SW_CSUM flag set (for example,
when shared across multiple devices), it appears this will now return
-EOPNOTSUPP. Should this check be restricted to only apply when using the
generic datapath?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260502200722.53960-1-kerneljasonxing@gmail.com?part=1