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

On 07/29, Maciej Fijalkowski wrote:

On Mon, Jul 27, 2026 at 09:19:53AM -0700, Stanislav Fomichev wrote:

quoted

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.

quoted

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