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

[Stanislav Fomichev <sdf.kernel@gmail.com>]

On 07/29, Maciej Fijalkowski wrote:
> 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.

Went through the issues, I don't think it found anything interesting.
Everything is pre-existing, but still worth addressing, will send
those fixes separately.

> >  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?

For this, yes, it is explained in the commit message:

  Note that only xsk_skb_metadata is properly using the flags,
  __xsk_buff_get_metadata ignores them. Next commits address that.

And eventually addressed in "[PATCH net 6/6] xsk: validate metadata when
processing requests". Couldn't find a less confusing way to split the
patches..