Re: [PATCH net] xsk: reject tx_metadata_len smaller than struct xsk_tx_metadata

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] UAPI Backward Compatibility Breakage

Pre-existing issues:
- [High] UAPI ABI Compatibility Mismatch in struct xsk_tx_metadata
--

--- Patch [1]: [PATCH net] xsk: reject tx_metadata_len smaller than struct xsk_tx_metadata ---
commit a89522120661c494f685c71309affa610fc7ca08
Author: Cen Zhang (Microsoft) <blbllhy@gmail.com>

xsk: reject tx_metadata_len smaller than struct xsk_tx_metadata

xdp_umem_reg() validates tx_metadata_len for upper bound (<256) and
alignment (%8) but not a lower bound. xsk_skb_metadata() computes
meta = buffer - pool->tx_metadata_len then unconditionally accesses
the full 24-byte struct xsk_tx_metadata, so any value less than
sizeof(struct xsk_tx_metadata) allows an out-of-bounds read. This adds
a lower-bound check in xdp_umem_reg() to reject tx_metadata_len
values that cannot cover the full metadata struct.

> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index 58da2f4f4397a..d16ad9d8f9197 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -208,7 +208,8 @@ static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
>  		return -EINVAL;
>  
>  	if (mr->flags & XDP_UMEM_TX_METADATA_LEN) {
> -		if (mr->tx_metadata_len >= 256 || mr->tx_metadata_len % 8)
> +		if (mr->tx_metadata_len < sizeof(struct xsk_tx_metadata) ||
> +		    mr->tx_metadata_len >= 256 || mr->tx_metadata_len % 8)

[Severity: Critical]
Does this strict size check in xdp_umem_reg() break backward compatibility
with older userspace applications?

Older applications compiled against earlier kernel headers (where
struct xsk_tx_metadata was 16 bytes, prior to the addition of the
launch_time field) will legitimately request a tx_metadata_len of 16.
This check evaluates to 16 < 24 and unconditionally rejects these
valid requests with -EINVAL, causing registration to fail.

To safely fix the underlying KASAN out-of-bounds read, should the kernel
instead dynamically validate that pool->tx_metadata_len is large enough to
cover the fields actually requested by meta->flags at runtime in
xsk_skb_metadata(), rather than enforcing a global minimum size here?

>  			return -EINVAL;
>  		umem->tx_metadata_len = mr->tx_metadata_len;
>  	}

[Severity: High]
This is a pre-existing issue, but does struct xsk_tx_metadata have an ABI
compatibility mismatch between 32-bit and 64-bit architectures?

Because __u64 has a 4-byte alignment requirement on 32-bit x86 but an 8-byte
alignment requirement on 64-bit x86, the compiler introduces 4 bytes of
implicit padding on 64-bit architectures before the launch_time field.

If a 32-bit userspace program runs on a 64-bit kernel in compat mode, it
writes launch_time to offset 12, but the 64-bit kernel reads it from offset
16 in xsk_skb_metadata(), resulting in garbage values being used for packet
transmission scheduling.

include/uapi/linux/if_xdp.h:struct xsk_tx_metadata {
        ...
        struct {
                ...
                __u16 csum_offset;

                /* XDP_TXMD_FLAGS_LAUNCH_TIME */
                /* Launch time in nanosecond against the PTP HW Clock */
                __u64 launch_time;
        } request;
}

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