Re: [Intel-wired-lan] [PATCH net 0/6] xsk: harden TX metadata validation against races

On Mon, Jul 27, 2026 at 09:19:53AM -0700, Stanislav Fomichev wrote:
> Cen Zhang reported a KASAN out-of-bounds read when AF_XDP is configured
> with a TX metadata area smaller than struct xsk_tx_metadata. The metadata
> is also shared with user space, so reading its flags more than once can
> produce inconsistent validation and processing decisions.
> 
> Require enough space for the flags and one request field, validate the
> launch-time field against the configured metadata length, and use one
> snapshot of the flags while processing each request. Carry the validated
> decision through completion handling so later user-space changes cannot
> enable an unrequested completion timestamp.

Stan,

I'm taking week off and I see one sashiko-reported issue which is not a
pre-existing one. Hope you could clarify it and unless no one picks the
review of this set i'll get back at it next week.

>  static inline struct xsk_tx_metadata *
>  __xsk_buff_get_metadata(const struct xsk_buff_pool *pool, void *data)
>  {
>  	struct xsk_tx_metadata *meta;
> +	u64 flags;
>  
>  	if (!pool->tx_metadata_len)
>  		return NULL;
>  
>  	meta = data - pool->tx_metadata_len;
> -	if (unlikely(!xsk_buff_valid_tx_metadata(meta)))
> +	if (unlikely(!xsk_buff_valid_tx_metadata(pool, meta, &flags)))
>  		return NULL; /* no way to signal the error to the user */
>  
>  	return meta;
The snapshotted flags are validated for size compliance in
xsk_buff_valid_tx_metadata() but then discarded, returning the
un-snapshotted user memory pointer (meta) to the driver.
Later in the zero-copy driver path, xsk_tx_metadata_request() re-reads
meta->flags directly from user memory:
include/net/xdp_sock.h:xsk_tx_metadata_request() {
    ...
    if (meta->flags & XDP_TXMD_FLAGS_LAUNCH_TIME)
        ops->tmo_request_launch_time(meta->request.launch_time, priv);
    ...
}
Does this create a Time-of-Check to Time-of-Use (TOCTOU) race condition in
the zero-copy TX metadata validation where userspace can concurrently enable
launch time after the size validation?

> 
> Reported-by: [email protected]
> Reported-by: Cen Zhang (Microsoft) <[email protected]>
> Link: https://lore.kernel.org/netdev/[email protected]/
> 
> Stanislav Fomichev (6):
>   xsk: require at least 16 bytes of TX metadata
>   xsk: pass TX metadata pointer by reference
>   xsk: clear metadata pointer when no timestamp is requested
>   xsk: validate launch-time metadata size
>   xsk: move xsk_tx_metadata_request() to xdp_sock_drv.h
>   xsk: validate metadata when processing requests
> 
>  drivers/net/ethernet/intel/igc/igc_main.c     |  2 +-
>  drivers/net/ethernet/mellanox/mlx5/core/en.h  |  2 +-
>  .../net/ethernet/mellanox/mlx5/core/en/xdp.c  | 15 ++--
>  .../net/ethernet/mellanox/mlx5/core/en/xdp.h  |  4 +-
>  .../ethernet/mellanox/mlx5/core/en/xsk/tx.c   |  2 +-
>  .../net/ethernet/stmicro/stmmac/stmmac_main.c |  4 +-
>  include/net/libeth/xsk.h                      |  2 +-
>  include/net/xdp_sock.h                        | 45 ++---------
>  include/net/xdp_sock_drv.h                    | 77 ++++++++++++++++---
>  net/xdp/xdp_umem.c                            |  2 +
>  net/xdp/xsk.c                                 | 11 ++-
>  net/xdp/xsk_buff_pool.c                       |  6 +-
>  12 files changed, 101 insertions(+), 71 deletions(-)
> 
> -- 
> 2.53.0-Meta
>