[PATCH net v2] xsk: reject undersized tx_metadata at runtime validation

[PATCH net v2] xsk: reject undersized tx_metadata at runtime validation

[Cen Zhang (Microsoft)]

There is no metadata length check before accessing struct xsk_tx_metadata
fields in both xdp_umem_reg() (registration) and
xsk_buff_valid_tx_metadata() (runtime).  Thus, an invalid small
tx_metadata_len (e.g. 8 bytes) passes registration and later causes an
out-of-bounds read when xsk_skb_metadata() accesses fields beyond the
declared metadata region.

KASAN reports this as:

  BUG: KASAN: vmalloc-out-of-bounds in xsk_skb_metadata+0x4b2/0x500
  Read of size 8 at addr ffffc90000f11000 by task exploit/148

  xsk_skb_metadata (net/xdp/xsk.c:837)
  xsk_build_skb (net/xdp/xsk.c)
  __xsk_generic_xmit (net/xdp/xsk.c)
  xsk_sendmsg (net/xdp/xsk.c)

Fixing at registration time would break backward compatibility with older
userspace applications, so apply a runtime length check in
xsk_buff_valid_tx_metadata() that validates tx_metadata_len against the
fields required by the enabled flags.

Fixes: 341ac980eab9 ("xsk: Support tx_metadata_len")
Reported-by: AutonomousCodeSecurity@microsoft.com
Link: https://lore.kernel.org/all/20260720155210.34229-1-blbllhy@gmail.com/
Suggested-by: Stanislav Fomichev <sdf@fomichev.me>
Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
---
V2:
 - Change to runtime per-flag length check as suggested by Stanislav.

 include/net/xdp_sock_drv.h | 22 ++++++++++++++++++----
 net/xdp/xsk.c              |  2 +-
 2 files changed, 19 insertions(+), 5 deletions(-)

diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
index 46797645a0c2..5c54c7462c45 100644
--- a/include/net/xdp_sock_drv.h
+++ b/include/net/xdp_sock_drv.h
@@ -260,9 +260,22 @@ xsk_buff_raw_get_ctx(const struct xsk_buff_pool *pool, u64 addr)
 	0)
 
 static inline bool
-xsk_buff_valid_tx_metadata(const struct xsk_tx_metadata *meta)
+xsk_buff_valid_tx_metadata(const struct xsk_buff_pool *pool,
+			   const struct xsk_tx_metadata *meta)
 {
-	return !(meta->flags & ~XDP_TXMD_FLAGS_VALID);
+	/* covers flags, XDP_TXMD_FLAGS_CHECKSUM & XDP_TXMD_FLAGS_TIMESTAMP */
+	if (unlikely(pool->tx_metadata_len < 16))
+		return false;
+
+	if (unlikely(meta->flags & ~XDP_TXMD_FLAGS_VALID))
+		return false;
+
+	if (meta->flags & XDP_TXMD_FLAGS_LAUNCH_TIME)
+		if (unlikely(pool->tx_metadata_len <
+			     offsetofend(struct xsk_tx_metadata, request.launch_time)))
+			return false;
+
+	return true;
 }
 
 static inline struct xsk_tx_metadata *
@@ -274,7 +287,7 @@ __xsk_buff_get_metadata(const struct xsk_buff_pool *pool, void *data)
 		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)))
 		return NULL; /* no way to signal the error to the user */
 
 	return meta;
@@ -469,7 +482,8 @@ xsk_buff_raw_get_ctx(const struct xsk_buff_pool *pool, u64 addr)
 	return (struct xdp_desc_ctx){ };
 }
 
-static inline bool xsk_buff_valid_tx_metadata(struct xsk_tx_metadata *meta)
+static inline bool xsk_buff_valid_tx_metadata(const struct xsk_buff_pool *pool,
+					      struct xsk_tx_metadata *meta)
 {
 	return false;
 }
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index b970f30ea9b9..75b2c97e41e4 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -809,7 +809,7 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
 		return -EINVAL;
 
 	meta = buffer - pool->tx_metadata_len;
-	if (unlikely(!xsk_buff_valid_tx_metadata(meta)))
+	if (unlikely(!xsk_buff_valid_tx_metadata(pool, meta)))
 		return -EINVAL;
 
 	if (meta->flags & XDP_TXMD_FLAGS_CHECKSUM) {
-- 
2.53.0

Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation

[Stanislav Fomichev]

On 07/22, Cen Zhang (Microsoft) wrote:
> There is no metadata length check before accessing struct xsk_tx_metadata
> fields in both xdp_umem_reg() (registration) and
> xsk_buff_valid_tx_metadata() (runtime).  Thus, an invalid small
> tx_metadata_len (e.g. 8 bytes) passes registration and later causes an
> out-of-bounds read when xsk_skb_metadata() accesses fields beyond the
> declared metadata region.
> 
> KASAN reports this as:
> 
>   BUG: KASAN: vmalloc-out-of-bounds in xsk_skb_metadata+0x4b2/0x500
>   Read of size 8 at addr ffffc90000f11000 by task exploit/148
> 
>   xsk_skb_metadata (net/xdp/xsk.c:837)
>   xsk_build_skb (net/xdp/xsk.c)
>   __xsk_generic_xmit (net/xdp/xsk.c)
>   xsk_sendmsg (net/xdp/xsk.c)
> 
> Fixing at registration time would break backward compatibility with older
> userspace applications, so apply a runtime length check in
> xsk_buff_valid_tx_metadata() that validates tx_metadata_len against the
> fields required by the enabled flags.
> 
> Fixes: 341ac980eab9 ("xsk: Support tx_metadata_len")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Link: https://lore.kernel.org/all/20260720155210.34229-1-blbllhy@gmail.com/
> Suggested-by: Stanislav Fomichev <sdf@fomichev.me>
> Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
> ---
> V2:
>  - Change to runtime per-flag length check as suggested by Stanislav.

Hmm, the AI review points out another set of TOCTOU issues :-/ I wonder
whether want to go all in and rewrite the users to have a copy
of metadata? Something like the following below, build tested only,
any issues with that?

(xsk_tx_metadata_to_compl (!meta) check is still buggy? not sure)

diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
index 17f213cc93e4..94f2a32917d2 100644
--- a/drivers/net/ethernet/intel/igc/igc.h
+++ b/drivers/net/ethernet/intel/igc/igc.h
@@ -604,7 +604,7 @@ struct igc_xdp_buff {
 
 struct igc_metadata_request {
 	struct igc_tx_buffer *tx_buffer;
-	struct xsk_tx_metadata *meta;
+	struct xsk_tx_metadata meta;
 	struct igc_ring *tx_ring;
 	u32 cmd_type;
 	u16 used_desc;
diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
index e6e9441fc3d4..ec51a5b1051d 100644
--- a/drivers/net/ethernet/intel/igc/igc_main.c
+++ b/drivers/net/ethernet/intel/igc/igc_main.c
@@ -2961,7 +2961,7 @@ static void igc_xsk_request_timestamp(void *_priv)
 		 * metadata area. It is the location to store the value of
 		 * tx hardware timestamp.
 		 */
-		xsk_tx_metadata_to_compl(meta_req->meta, &tstamp->xsk_meta);
+		xsk_tx_metadata_to_compl(&meta_req->meta, &tstamp->xsk_meta);
 
 		/* Set timestamp bit based on the _TSTAMP(_X) bit. */
 		tx_flags |= tstamp->flags;
@@ -3059,7 +3059,7 @@ static void igc_xdp_xmit_zc(struct igc_ring *ring)
 	 */
 	while (budget >= 4 && xsk_tx_peek_desc(pool, &xdp_desc)) {
 		struct igc_metadata_request meta_req;
-		struct xsk_tx_metadata *meta = NULL;
+		struct xsk_tx_metadata meta = {};
 		struct igc_tx_buffer *bi;
 		u32 olinfo_status;
 		dma_addr_t dma;
@@ -3071,7 +3071,7 @@ static void igc_xdp_xmit_zc(struct igc_ring *ring)
 		olinfo_status = xdp_desc.len << IGC_ADVTXD_PAYLEN_SHIFT;
 
 		dma = xsk_buff_raw_get_dma(pool, xdp_desc.addr);
-		meta = xsk_buff_get_metadata(pool, xdp_desc.addr);
+		xsk_buff_read_metadata(pool, xdp_desc.addr, &meta);
 		xsk_buff_raw_dma_sync_for_device(pool, dma, xdp_desc.len);
 		bi = &ring->tx_buffer_info[ntu];
 
@@ -3079,7 +3079,7 @@ static void igc_xdp_xmit_zc(struct igc_ring *ring)
 		meta_req.tx_buffer = bi;
 		meta_req.meta = meta;
 		meta_req.used_desc = 0;
-		xsk_tx_metadata_request(meta, &igc_xsk_tx_metadata_ops,
+		xsk_tx_metadata_request(&meta, &igc_xsk_tx_metadata_ops,
 					&meta_req);
 
 		/* xsk_tx_metadata_request() may have updated next_to_use */
diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
index 8aeab4b21035..1721a0a9220a 100644
--- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
+++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
@@ -66,7 +66,7 @@ static void mlx5e_xsk_tx_post_err(struct mlx5e_xdpsq *sq,
 bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget)
 {
 	struct xsk_buff_pool *pool = sq->xsk_pool;
-	struct xsk_tx_metadata *meta = NULL;
+	struct xsk_tx_metadata meta = {};
 	union mlx5e_xdp_info xdpi;
 	bool work_done = true;
 	bool flush = false;
@@ -99,13 +99,13 @@ bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget)
 		xdptxd.dma_addr = xsk_buff_raw_get_dma(pool, desc.addr);
 		xdptxd.data = xsk_buff_raw_get_data(pool, desc.addr);
 		xdptxd.len = desc.len;
-		meta = xsk_buff_get_metadata(pool, desc.addr);
+		xsk_buff_read_metadata(pool, desc.addr, &meta);
 
 		xsk_buff_raw_dma_sync_for_device(pool, xdptxd.dma_addr, xdptxd.len);
 
 		ret = INDIRECT_CALL_2(sq->xmit_xdp_frame, mlx5e_xmit_xdp_frame_mpwqe,
 				      mlx5e_xmit_xdp_frame, sq, &xdptxd,
-				      check_result, meta);
+				      check_result, &meta);
 		if (unlikely(!ret)) {
 			if (sq->mpwqe.wqe)
 				mlx5e_xdp_mpwqe_complete(sq);
@@ -116,7 +116,7 @@ bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget)
 			if (xp_tx_metadata_enabled(sq->xsk_pool)) {
 				struct xsk_tx_metadata_compl compl;
 
-				xsk_tx_metadata_to_compl(meta, &compl);
+				xsk_tx_metadata_to_compl(&meta, &compl);
 				XSK_TX_COMPL_FITS(void *);
 
 				mlx5e_xdpi_fifo_push(&sq->db.xdpi_fifo,
diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
index 562d20830b94..217134559d41 100644
--- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
+++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
@@ -2707,7 +2707,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
 
 	for (; budget > 0; budget--) {
 		struct stmmac_metadata_request meta_req;
-		struct xsk_tx_metadata *meta = NULL;
+		struct xsk_tx_metadata meta = {};
 		dma_addr_t dma_addr;
 		bool set_ic;
 
@@ -2732,7 +2732,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
 
 		tx_desc = stmmac_get_tx_desc(priv, tx_q, entry);
 		dma_addr = xsk_buff_raw_get_dma(pool, xdp_desc.addr);
-		meta = xsk_buff_get_metadata(pool, xdp_desc.addr);
+		xsk_buff_read_metadata(pool, xdp_desc.addr, &meta);
 		xsk_buff_raw_dma_sync_for_device(pool, dma_addr, xdp_desc.len);
 
 		/* To return XDP buffer to XSK pool, we simple call
@@ -2761,7 +2761,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
 		meta_req.set_ic = &set_ic;
 		meta_req.tbs = tx_q->tbs;
 		meta_req.edesc = &tx_q->dma_entx[entry];
-		xsk_tx_metadata_request(meta, &stmmac_xsk_tx_metadata_ops,
+		xsk_tx_metadata_request(&meta, &stmmac_xsk_tx_metadata_ops,
 					&meta_req);
 		if (set_ic) {
 			tx_q->tx_count_frames = 0;
@@ -2775,7 +2775,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
 
 		stmmac_enable_dma_transmission(priv, priv->ioaddr, queue);
 
-		xsk_tx_metadata_to_compl(meta,
+		xsk_tx_metadata_to_compl(&meta,
 					 &tx_q->tx_skbuff_dma[entry].xsk_meta);
 
 		tx_q->cur_tx = STMMAC_NEXT_ENTRY(tx_q->cur_tx, priv->dma_conf.dma_tx_size);
diff --git a/include/net/libeth/xsk.h b/include/net/libeth/xsk.h
index 82b5d21aae87..e2fa6bf6b1b3 100644
--- a/include/net/libeth/xsk.h
+++ b/include/net/libeth/xsk.h
@@ -205,7 +205,7 @@ __libeth_xsk_xmit_fill_buf_md(const struct xdp_desc *xdesc,
 	BUILD_BUG_ON(!__builtin_constant_p(tmo == libeth_xsktmo));
 	tmo = tmo == libeth_xsktmo ? &__libeth_xsktmo : tmo;
 
-	xsk_tx_metadata_request(ctx.meta, tmo, &desc);
+	xsk_tx_metadata_request(&ctx.meta, tmo, &desc);
 
 	return desc;
 }
diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
index ebac60a3d8a1..eb2645a88934 100644
--- a/include/net/xdp_sock.h
+++ b/include/net/xdp_sock.h
@@ -163,7 +163,7 @@ static inline void xsk_tx_metadata_request(const struct xsk_tx_metadata *meta,
 					   const struct xsk_tx_metadata_ops *ops,
 					   void *priv)
 {
-	if (!meta)
+	if (!meta || !meta->flags)
 		return;
 
 	if (ops->tmo_request_launch_time)
diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
index 46797645a0c2..925c25bad57c 100644
--- a/include/net/xdp_sock_drv.h
+++ b/include/net/xdp_sock_drv.h
@@ -259,31 +259,42 @@ xsk_buff_raw_get_ctx(const struct xsk_buff_pool *pool, u64 addr)
 		XDP_TXMD_FLAGS_LAUNCH_TIME | \
 	0)
 
-static inline bool
-xsk_buff_valid_tx_metadata(const struct xsk_tx_metadata *meta)
+static inline int
+xsk_buff_read_tx_metadata(const struct xsk_buff_pool *pool,
+			  void *data,
+			  struct xsk_tx_metadata *meta)
 {
-	return !(meta->flags & ~XDP_TXMD_FLAGS_VALID);
-}
-
-static inline struct xsk_tx_metadata *
-__xsk_buff_get_metadata(const struct xsk_buff_pool *pool, void *data)
-{
-	struct xsk_tx_metadata *meta;
+	struct xsk_tx_metadata *p;
+	u64 flags = 0;
 
 	if (!pool->tx_metadata_len)
-		return NULL;
+		return 0;
+
+	/* covers flags, XDP_TXMD_FLAGS_CHECKSUM & XDP_TXMD_FLAGS_TIMESTAMP */
+	if (unlikely(pool->tx_metadata_len < 16))
+		return -EINVAL;
+
+	p = data - pool->tx_metadata_len;
+	flags = READ_ONCE(p->flags);
+
+	if (flags & ~XDP_TXMD_FLAGS_VALID)
+		return -EINVAL;
 
-	meta = data - pool->tx_metadata_len;
-	if (unlikely(!xsk_buff_valid_tx_metadata(meta)))
-		return NULL; /* no way to signal the error to the user */
+	if (flags & XDP_TXMD_FLAGS_LAUNCH_TIME)
+		if (pool->tx_metadata_len < offsetofend(struct xsk_tx_metadata, request.launch_time))
+			return -EINVAL;
 
-	return meta;
+	memcpy(meta, p, min(pool->tx_metadata_len, sizeof(struct xsk_tx_metadata)));
+	meta->flags = flags;
+	return 0;
 }
 
-static inline struct xsk_tx_metadata *
-xsk_buff_get_metadata(struct xsk_buff_pool *pool, u64 addr)
+static inline void xsk_buff_read_metadata(const struct xsk_buff_pool *pool,
+					  u64 addr,
+					  struct xsk_tx_metadata *meta)
 {
-	return __xsk_buff_get_metadata(pool, xp_raw_get_data(pool, addr));
+	if (xsk_buff_read_tx_metadata(pool, xp_raw_get_data(pool, addr), meta) < 0)
+		meta->flags = 0;
 }
 
 static inline void xsk_buff_dma_sync_for_cpu(struct xdp_buff *xdp)
@@ -469,21 +480,18 @@ xsk_buff_raw_get_ctx(const struct xsk_buff_pool *pool, u64 addr)
 	return (struct xdp_desc_ctx){ };
 }
 
-static inline bool xsk_buff_valid_tx_metadata(struct xsk_tx_metadata *meta)
+static inline int
+xsk_buff_read_tx_metadata(const struct xsk_buff_pool *pool,
+			  void *data,
+			  struct xsk_tx_metadata *meta)
 {
 	return false;
 }
 
-static inline struct xsk_tx_metadata *
-__xsk_buff_get_metadata(const struct xsk_buff_pool *pool, void *data)
-{
-	return NULL;
-}
-
-static inline struct xsk_tx_metadata *
-xsk_buff_get_metadata(struct xsk_buff_pool *pool, u64 addr)
+static inline void xsk_buff_read_metadata(const struct xsk_buff_pool *pool,
+					  u64 addr,
+					  struct xsk_tx_metadata *meta)
 {
-	return NULL;
 }
 
 static inline void xsk_buff_dma_sync_for_cpu(struct xdp_buff *xdp)
diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
index ccb3b350001f..71992b9e680d 100644
--- a/include/net/xsk_buff_pool.h
+++ b/include/net/xsk_buff_pool.h
@@ -141,12 +141,12 @@ void xp_dma_unmap(struct xsk_buff_pool *pool, unsigned long attrs);
 struct xdp_buff *xp_alloc(struct xsk_buff_pool *pool);
 u32 xp_alloc_batch(struct xsk_buff_pool *pool, struct xdp_buff **xdp, u32 max);
 bool xp_can_alloc(struct xsk_buff_pool *pool, u32 count);
-void *xp_raw_get_data(struct xsk_buff_pool *pool, u64 addr);
+void *xp_raw_get_data(const struct xsk_buff_pool *pool, u64 addr);
 dma_addr_t xp_raw_get_dma(struct xsk_buff_pool *pool, u64 addr);
 
 struct xdp_desc_ctx {
 	dma_addr_t dma;
-	struct xsk_tx_metadata *meta;
+	struct xsk_tx_metadata meta;
 };
 
 struct xdp_desc_ctx xp_raw_get_ctx(const struct xsk_buff_pool *pool, u64 addr);
diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
index b970f30ea9b9..dbe4ddae30e8 100644
--- a/net/xdp/xsk.c
+++ b/net/xdp/xsk.c
@@ -802,19 +802,17 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
 			    struct xdp_desc *desc, struct xsk_buff_pool *pool,
 			    u32 hr)
 {
-	struct xsk_tx_metadata *meta = NULL;
+	struct xsk_tx_metadata meta = {};
 	u16 csum_start, csum_offset;
+	int ret;
 
-	if (unlikely(pool->tx_metadata_len == 0))
-		return -EINVAL;
-
-	meta = buffer - pool->tx_metadata_len;
-	if (unlikely(!xsk_buff_valid_tx_metadata(meta)))
-		return -EINVAL;
+	ret = xsk_buff_read_tx_metadata(pool, buffer, &meta);
+	if (ret < 0)
+		return ret;
 
-	if (meta->flags & XDP_TXMD_FLAGS_CHECKSUM) {
-		csum_start = READ_ONCE(meta->request.csum_start);
-		csum_offset = READ_ONCE(meta->request.csum_offset);
+	if (meta.flags & XDP_TXMD_FLAGS_CHECKSUM) {
+		csum_start = READ_ONCE(meta.request.csum_start);
+		csum_offset = READ_ONCE(meta.request.csum_offset);
 
 		if (unlikely(csum_start + csum_offset +
 			     sizeof(__sum16) > desc->len))
@@ -833,9 +831,9 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
 		}
 	}
 
-	if (meta->flags & XDP_TXMD_FLAGS_LAUNCH_TIME)
-		skb->skb_mstamp_ns = meta->request.launch_time;
-	xsk_tx_metadata_to_compl(meta, &skb_shinfo(skb)->xsk_meta);
+	if (meta.flags & XDP_TXMD_FLAGS_LAUNCH_TIME)
+		skb->skb_mstamp_ns = meta.request.launch_time;
+	xsk_tx_metadata_to_compl(&meta, &skb_shinfo(skb)->xsk_meta);
 
 	return 0;
 }
diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
index 1f28a9641571..c9f05835e433 100644
--- a/net/xdp/xsk_buff_pool.c
+++ b/net/xdp/xsk_buff_pool.c
@@ -735,7 +735,7 @@ static void *__xp_raw_get_data(const struct xsk_buff_pool *pool, u64 addr)
 	return pool->addrs + addr;
 }
 
-void *xp_raw_get_data(struct xsk_buff_pool *pool, u64 addr)
+void *xp_raw_get_data(const struct xsk_buff_pool *pool, u64 addr)
 {
 	return __xp_raw_get_data(pool, __xp_raw_get_addr(pool, addr));
 }
@@ -773,7 +773,7 @@ struct xdp_desc_ctx xp_raw_get_ctx(const struct xsk_buff_pool *pool, u64 addr)
 	addr = __xp_raw_get_addr(pool, addr);
 
 	ret.dma = __xp_raw_get_dma(pool, addr);
-	ret.meta = __xsk_buff_get_metadata(pool, __xp_raw_get_data(pool, addr));
+	xsk_buff_read_metadata(pool, addr, &ret.meta);
 
 	return ret;
 }

Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation

[Maciej Fijalkowski]

On Wed, Jul 22, 2026 at 10:55:17AM -0700, Stanislav Fomichev wrote:
> On 07/22, Cen Zhang (Microsoft) wrote:
> > There is no metadata length check before accessing struct xsk_tx_metadata
> > fields in both xdp_umem_reg() (registration) and
> > xsk_buff_valid_tx_metadata() (runtime).  Thus, an invalid small
> > tx_metadata_len (e.g. 8 bytes) passes registration and later causes an
> > out-of-bounds read when xsk_skb_metadata() accesses fields beyond the
> > declared metadata region.
> > 
> > KASAN reports this as:
> > 
> >   BUG: KASAN: vmalloc-out-of-bounds in xsk_skb_metadata+0x4b2/0x500
> >   Read of size 8 at addr ffffc90000f11000 by task exploit/148
> > 
> >   xsk_skb_metadata (net/xdp/xsk.c:837)
> >   xsk_build_skb (net/xdp/xsk.c)
> >   __xsk_generic_xmit (net/xdp/xsk.c)
> >   xsk_sendmsg (net/xdp/xsk.c)
> > 
> > Fixing at registration time would break backward compatibility with older
> > userspace applications, so apply a runtime length check in
> > xsk_buff_valid_tx_metadata() that validates tx_metadata_len against the
> > fields required by the enabled flags.
> > 
> > Fixes: 341ac980eab9 ("xsk: Support tx_metadata_len")
> > Reported-by: AutonomousCodeSecurity@microsoft.com
> > Link: https://lore.kernel.org/all/20260720155210.34229-1-blbllhy@gmail.com/
> > Suggested-by: Stanislav Fomichev <sdf@fomichev.me>
> > Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
> > ---
> > V2:
> >  - Change to runtime per-flag length check as suggested by Stanislav.
> 
> Hmm, the AI review points out another set of TOCTOU issues :-/ I wonder
> whether want to go all in and rewrite the users to have a copy
> of metadata? Something like the following below, build tested only,
> any issues with that?

Maybe you want to take ownership of this patch and run the proposed
resolution against internal AI review?

> 
> (xsk_tx_metadata_to_compl (!meta) check is still buggy? not sure)
> 
> diff --git a/drivers/net/ethernet/intel/igc/igc.h b/drivers/net/ethernet/intel/igc/igc.h
> index 17f213cc93e4..94f2a32917d2 100644
> --- a/drivers/net/ethernet/intel/igc/igc.h
> +++ b/drivers/net/ethernet/intel/igc/igc.h
> @@ -604,7 +604,7 @@ struct igc_xdp_buff {
>  
>  struct igc_metadata_request {
>  	struct igc_tx_buffer *tx_buffer;
> -	struct xsk_tx_metadata *meta;
> +	struct xsk_tx_metadata meta;
>  	struct igc_ring *tx_ring;
>  	u32 cmd_type;
>  	u16 used_desc;
> diff --git a/drivers/net/ethernet/intel/igc/igc_main.c b/drivers/net/ethernet/intel/igc/igc_main.c
> index e6e9441fc3d4..ec51a5b1051d 100644
> --- a/drivers/net/ethernet/intel/igc/igc_main.c
> +++ b/drivers/net/ethernet/intel/igc/igc_main.c
> @@ -2961,7 +2961,7 @@ static void igc_xsk_request_timestamp(void *_priv)
>  		 * metadata area. It is the location to store the value of
>  		 * tx hardware timestamp.
>  		 */
> -		xsk_tx_metadata_to_compl(meta_req->meta, &tstamp->xsk_meta);
> +		xsk_tx_metadata_to_compl(&meta_req->meta, &tstamp->xsk_meta);
>  
>  		/* Set timestamp bit based on the _TSTAMP(_X) bit. */
>  		tx_flags |= tstamp->flags;
> @@ -3059,7 +3059,7 @@ static void igc_xdp_xmit_zc(struct igc_ring *ring)
>  	 */
>  	while (budget >= 4 && xsk_tx_peek_desc(pool, &xdp_desc)) {
>  		struct igc_metadata_request meta_req;
> -		struct xsk_tx_metadata *meta = NULL;
> +		struct xsk_tx_metadata meta = {};
>  		struct igc_tx_buffer *bi;
>  		u32 olinfo_status;
>  		dma_addr_t dma;
> @@ -3071,7 +3071,7 @@ static void igc_xdp_xmit_zc(struct igc_ring *ring)
>  		olinfo_status = xdp_desc.len << IGC_ADVTXD_PAYLEN_SHIFT;
>  
>  		dma = xsk_buff_raw_get_dma(pool, xdp_desc.addr);
> -		meta = xsk_buff_get_metadata(pool, xdp_desc.addr);
> +		xsk_buff_read_metadata(pool, xdp_desc.addr, &meta);
>  		xsk_buff_raw_dma_sync_for_device(pool, dma, xdp_desc.len);
>  		bi = &ring->tx_buffer_info[ntu];
>  
> @@ -3079,7 +3079,7 @@ static void igc_xdp_xmit_zc(struct igc_ring *ring)
>  		meta_req.tx_buffer = bi;
>  		meta_req.meta = meta;
>  		meta_req.used_desc = 0;
> -		xsk_tx_metadata_request(meta, &igc_xsk_tx_metadata_ops,
> +		xsk_tx_metadata_request(&meta, &igc_xsk_tx_metadata_ops,
>  					&meta_req);
>  
>  		/* xsk_tx_metadata_request() may have updated next_to_use */
> diff --git a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
> index 8aeab4b21035..1721a0a9220a 100644
> --- a/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en/xsk/tx.c
> @@ -66,7 +66,7 @@ static void mlx5e_xsk_tx_post_err(struct mlx5e_xdpsq *sq,
>  bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget)
>  {
>  	struct xsk_buff_pool *pool = sq->xsk_pool;
> -	struct xsk_tx_metadata *meta = NULL;
> +	struct xsk_tx_metadata meta = {};
>  	union mlx5e_xdp_info xdpi;
>  	bool work_done = true;
>  	bool flush = false;
> @@ -99,13 +99,13 @@ bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget)
>  		xdptxd.dma_addr = xsk_buff_raw_get_dma(pool, desc.addr);
>  		xdptxd.data = xsk_buff_raw_get_data(pool, desc.addr);
>  		xdptxd.len = desc.len;
> -		meta = xsk_buff_get_metadata(pool, desc.addr);
> +		xsk_buff_read_metadata(pool, desc.addr, &meta);
>  
>  		xsk_buff_raw_dma_sync_for_device(pool, xdptxd.dma_addr, xdptxd.len);
>  
>  		ret = INDIRECT_CALL_2(sq->xmit_xdp_frame, mlx5e_xmit_xdp_frame_mpwqe,
>  				      mlx5e_xmit_xdp_frame, sq, &xdptxd,
> -				      check_result, meta);
> +				      check_result, &meta);
>  		if (unlikely(!ret)) {
>  			if (sq->mpwqe.wqe)
>  				mlx5e_xdp_mpwqe_complete(sq);
> @@ -116,7 +116,7 @@ bool mlx5e_xsk_tx(struct mlx5e_xdpsq *sq, unsigned int budget)
>  			if (xp_tx_metadata_enabled(sq->xsk_pool)) {
>  				struct xsk_tx_metadata_compl compl;
>  
> -				xsk_tx_metadata_to_compl(meta, &compl);
> +				xsk_tx_metadata_to_compl(&meta, &compl);
>  				XSK_TX_COMPL_FITS(void *);
>  
>  				mlx5e_xdpi_fifo_push(&sq->db.xdpi_fifo,
> diff --git a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> index 562d20830b94..217134559d41 100644
> --- a/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> +++ b/drivers/net/ethernet/stmicro/stmmac/stmmac_main.c
> @@ -2707,7 +2707,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
>  
>  	for (; budget > 0; budget--) {
>  		struct stmmac_metadata_request meta_req;
> -		struct xsk_tx_metadata *meta = NULL;
> +		struct xsk_tx_metadata meta = {};
>  		dma_addr_t dma_addr;
>  		bool set_ic;
>  
> @@ -2732,7 +2732,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
>  
>  		tx_desc = stmmac_get_tx_desc(priv, tx_q, entry);
>  		dma_addr = xsk_buff_raw_get_dma(pool, xdp_desc.addr);
> -		meta = xsk_buff_get_metadata(pool, xdp_desc.addr);
> +		xsk_buff_read_metadata(pool, xdp_desc.addr, &meta);
>  		xsk_buff_raw_dma_sync_for_device(pool, dma_addr, xdp_desc.len);
>  
>  		/* To return XDP buffer to XSK pool, we simple call
> @@ -2761,7 +2761,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
>  		meta_req.set_ic = &set_ic;
>  		meta_req.tbs = tx_q->tbs;
>  		meta_req.edesc = &tx_q->dma_entx[entry];
> -		xsk_tx_metadata_request(meta, &stmmac_xsk_tx_metadata_ops,
> +		xsk_tx_metadata_request(&meta, &stmmac_xsk_tx_metadata_ops,
>  					&meta_req);
>  		if (set_ic) {
>  			tx_q->tx_count_frames = 0;
> @@ -2775,7 +2775,7 @@ static bool stmmac_xdp_xmit_zc(struct stmmac_priv *priv, u32 queue, u32 budget)
>  
>  		stmmac_enable_dma_transmission(priv, priv->ioaddr, queue);
>  
> -		xsk_tx_metadata_to_compl(meta,
> +		xsk_tx_metadata_to_compl(&meta,
>  					 &tx_q->tx_skbuff_dma[entry].xsk_meta);
>  
>  		tx_q->cur_tx = STMMAC_NEXT_ENTRY(tx_q->cur_tx, priv->dma_conf.dma_tx_size);
> diff --git a/include/net/libeth/xsk.h b/include/net/libeth/xsk.h
> index 82b5d21aae87..e2fa6bf6b1b3 100644
> --- a/include/net/libeth/xsk.h
> +++ b/include/net/libeth/xsk.h
> @@ -205,7 +205,7 @@ __libeth_xsk_xmit_fill_buf_md(const struct xdp_desc *xdesc,
>  	BUILD_BUG_ON(!__builtin_constant_p(tmo == libeth_xsktmo));
>  	tmo = tmo == libeth_xsktmo ? &__libeth_xsktmo : tmo;
>  
> -	xsk_tx_metadata_request(ctx.meta, tmo, &desc);
> +	xsk_tx_metadata_request(&ctx.meta, tmo, &desc);
>  
>  	return desc;
>  }
> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index ebac60a3d8a1..eb2645a88934 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -163,7 +163,7 @@ static inline void xsk_tx_metadata_request(const struct xsk_tx_metadata *meta,
>  					   const struct xsk_tx_metadata_ops *ops,
>  					   void *priv)
>  {
> -	if (!meta)
> +	if (!meta || !meta->flags)
>  		return;
>  
>  	if (ops->tmo_request_launch_time)
> diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
> index 46797645a0c2..925c25bad57c 100644
> --- a/include/net/xdp_sock_drv.h
> +++ b/include/net/xdp_sock_drv.h
> @@ -259,31 +259,42 @@ xsk_buff_raw_get_ctx(const struct xsk_buff_pool *pool, u64 addr)
>  		XDP_TXMD_FLAGS_LAUNCH_TIME | \
>  	0)
>  
> -static inline bool
> -xsk_buff_valid_tx_metadata(const struct xsk_tx_metadata *meta)
> +static inline int
> +xsk_buff_read_tx_metadata(const struct xsk_buff_pool *pool,
> +			  void *data,
> +			  struct xsk_tx_metadata *meta)
>  {
> -	return !(meta->flags & ~XDP_TXMD_FLAGS_VALID);
> -}
> -
> -static inline struct xsk_tx_metadata *
> -__xsk_buff_get_metadata(const struct xsk_buff_pool *pool, void *data)
> -{
> -	struct xsk_tx_metadata *meta;
> +	struct xsk_tx_metadata *p;
> +	u64 flags = 0;
>  
>  	if (!pool->tx_metadata_len)
> -		return NULL;
> +		return 0;
> +
> +	/* covers flags, XDP_TXMD_FLAGS_CHECKSUM & XDP_TXMD_FLAGS_TIMESTAMP */
> +	if (unlikely(pool->tx_metadata_len < 16))
> +		return -EINVAL;
> +
> +	p = data - pool->tx_metadata_len;
> +	flags = READ_ONCE(p->flags);
> +
> +	if (flags & ~XDP_TXMD_FLAGS_VALID)
> +		return -EINVAL;
>  
> -	meta = data - pool->tx_metadata_len;
> -	if (unlikely(!xsk_buff_valid_tx_metadata(meta)))
> -		return NULL; /* no way to signal the error to the user */
> +	if (flags & XDP_TXMD_FLAGS_LAUNCH_TIME)
> +		if (pool->tx_metadata_len < offsetofend(struct xsk_tx_metadata, request.launch_time))
> +			return -EINVAL;
>  
> -	return meta;
> +	memcpy(meta, p, min(pool->tx_metadata_len, sizeof(struct xsk_tx_metadata)));
> +	meta->flags = flags;
> +	return 0;
>  }
>  
> -static inline struct xsk_tx_metadata *
> -xsk_buff_get_metadata(struct xsk_buff_pool *pool, u64 addr)
> +static inline void xsk_buff_read_metadata(const struct xsk_buff_pool *pool,
> +					  u64 addr,
> +					  struct xsk_tx_metadata *meta)
>  {
> -	return __xsk_buff_get_metadata(pool, xp_raw_get_data(pool, addr));
> +	if (xsk_buff_read_tx_metadata(pool, xp_raw_get_data(pool, addr), meta) < 0)
> +		meta->flags = 0;
>  }
>  
>  static inline void xsk_buff_dma_sync_for_cpu(struct xdp_buff *xdp)
> @@ -469,21 +480,18 @@ xsk_buff_raw_get_ctx(const struct xsk_buff_pool *pool, u64 addr)
>  	return (struct xdp_desc_ctx){ };
>  }
>  
> -static inline bool xsk_buff_valid_tx_metadata(struct xsk_tx_metadata *meta)
> +static inline int
> +xsk_buff_read_tx_metadata(const struct xsk_buff_pool *pool,
> +			  void *data,
> +			  struct xsk_tx_metadata *meta)
>  {
>  	return false;
>  }
>  
> -static inline struct xsk_tx_metadata *
> -__xsk_buff_get_metadata(const struct xsk_buff_pool *pool, void *data)
> -{
> -	return NULL;
> -}
> -
> -static inline struct xsk_tx_metadata *
> -xsk_buff_get_metadata(struct xsk_buff_pool *pool, u64 addr)
> +static inline void xsk_buff_read_metadata(const struct xsk_buff_pool *pool,
> +					  u64 addr,
> +					  struct xsk_tx_metadata *meta)
>  {
> -	return NULL;
>  }
>  
>  static inline void xsk_buff_dma_sync_for_cpu(struct xdp_buff *xdp)
> diff --git a/include/net/xsk_buff_pool.h b/include/net/xsk_buff_pool.h
> index ccb3b350001f..71992b9e680d 100644
> --- a/include/net/xsk_buff_pool.h
> +++ b/include/net/xsk_buff_pool.h
> @@ -141,12 +141,12 @@ void xp_dma_unmap(struct xsk_buff_pool *pool, unsigned long attrs);
>  struct xdp_buff *xp_alloc(struct xsk_buff_pool *pool);
>  u32 xp_alloc_batch(struct xsk_buff_pool *pool, struct xdp_buff **xdp, u32 max);
>  bool xp_can_alloc(struct xsk_buff_pool *pool, u32 count);
> -void *xp_raw_get_data(struct xsk_buff_pool *pool, u64 addr);
> +void *xp_raw_get_data(const struct xsk_buff_pool *pool, u64 addr);
>  dma_addr_t xp_raw_get_dma(struct xsk_buff_pool *pool, u64 addr);
>  
>  struct xdp_desc_ctx {
>  	dma_addr_t dma;
> -	struct xsk_tx_metadata *meta;
> +	struct xsk_tx_metadata meta;
>  };
>  
>  struct xdp_desc_ctx xp_raw_get_ctx(const struct xsk_buff_pool *pool, u64 addr);
> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index b970f30ea9b9..dbe4ddae30e8 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -802,19 +802,17 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
>  			    struct xdp_desc *desc, struct xsk_buff_pool *pool,
>  			    u32 hr)
>  {
> -	struct xsk_tx_metadata *meta = NULL;
> +	struct xsk_tx_metadata meta = {};
>  	u16 csum_start, csum_offset;
> +	int ret;
>  
> -	if (unlikely(pool->tx_metadata_len == 0))
> -		return -EINVAL;
> -
> -	meta = buffer - pool->tx_metadata_len;
> -	if (unlikely(!xsk_buff_valid_tx_metadata(meta)))
> -		return -EINVAL;
> +	ret = xsk_buff_read_tx_metadata(pool, buffer, &meta);
> +	if (ret < 0)
> +		return ret;
>  
> -	if (meta->flags & XDP_TXMD_FLAGS_CHECKSUM) {
> -		csum_start = READ_ONCE(meta->request.csum_start);
> -		csum_offset = READ_ONCE(meta->request.csum_offset);
> +	if (meta.flags & XDP_TXMD_FLAGS_CHECKSUM) {
> +		csum_start = READ_ONCE(meta.request.csum_start);
> +		csum_offset = READ_ONCE(meta.request.csum_offset);
>  
>  		if (unlikely(csum_start + csum_offset +
>  			     sizeof(__sum16) > desc->len))
> @@ -833,9 +831,9 @@ static int xsk_skb_metadata(struct sk_buff *skb, void *buffer,
>  		}
>  	}
>  
> -	if (meta->flags & XDP_TXMD_FLAGS_LAUNCH_TIME)
> -		skb->skb_mstamp_ns = meta->request.launch_time;
> -	xsk_tx_metadata_to_compl(meta, &skb_shinfo(skb)->xsk_meta);
> +	if (meta.flags & XDP_TXMD_FLAGS_LAUNCH_TIME)
> +		skb->skb_mstamp_ns = meta.request.launch_time;
> +	xsk_tx_metadata_to_compl(&meta, &skb_shinfo(skb)->xsk_meta);
>  
>  	return 0;
>  }
> diff --git a/net/xdp/xsk_buff_pool.c b/net/xdp/xsk_buff_pool.c
> index 1f28a9641571..c9f05835e433 100644
> --- a/net/xdp/xsk_buff_pool.c
> +++ b/net/xdp/xsk_buff_pool.c
> @@ -735,7 +735,7 @@ static void *__xp_raw_get_data(const struct xsk_buff_pool *pool, u64 addr)
>  	return pool->addrs + addr;
>  }
>  
> -void *xp_raw_get_data(struct xsk_buff_pool *pool, u64 addr)
> +void *xp_raw_get_data(const struct xsk_buff_pool *pool, u64 addr)
>  {
>  	return __xp_raw_get_data(pool, __xp_raw_get_addr(pool, addr));
>  }
> @@ -773,7 +773,7 @@ struct xdp_desc_ctx xp_raw_get_ctx(const struct xsk_buff_pool *pool, u64 addr)
>  	addr = __xp_raw_get_addr(pool, addr);
>  
>  	ret.dma = __xp_raw_get_dma(pool, addr);
> -	ret.meta = __xsk_buff_get_metadata(pool, __xp_raw_get_data(pool, addr));
> +	xsk_buff_read_metadata(pool, addr, &ret.meta);
>  
>  	return ret;
>  }

Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation

[Cen Zhang]

On Wed, Jul 22, 2026 at 10:55:17AM -0700, Stanislav Fomichev wrote:
> Hmm, the AI review points out another set of TOCTOU issues :-/ I wonder
> whether want to go all in and rewrite the users to have a copy
> of metadata? Something like the following below, build tested only,
> any issues with that?

I applied your proposed patch and tested it against my local reproducer
-- the bug is gone.

> Maybe you want to take ownership of this patch and run the proposed
> resolution against internal AI review?

Please go ahead and carry this forward. Happy to help test further
if needed.

Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation

[Maciej Fijalkowski]

On Thu, Jul 23, 2026 at 08:06:10AM -0400, Cen Zhang wrote:
> On Wed, Jul 22, 2026 at 10:55:17AM -0700, Stanislav Fomichev wrote:
> > Hmm, the AI review points out another set of TOCTOU issues :-/ I wonder
> > whether want to go all in and rewrite the users to have a copy
> > of metadata? Something like the following below, build tested only,
> > any issues with that?
> 
> I applied your proposed patch and tested it against my local reproducer
> -- the bug is gone.
> 
> > Maybe you want to take ownership of this patch and run the proposed
> > resolution against internal AI review?
> 
> Please go ahead and carry this forward. Happy to help test further
> if needed.

Let's see whether Stan is ok with that, you can get credit via
Reported-by: tag or you can continue working on this, that was only a
suggestion, so I hope you did not find it offensive.

Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation

[Cen Zhang]

Not at all! Thanks for both your suggestion and Stanislav's rewrite!

Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation

[Alexander Lobakin]

From: Stanislav Fomichev <sdf.kernel@gmail.com>
Date: Wed, 22 Jul 2026 10:55:17 -0700

> On 07/22, Cen Zhang (Microsoft) wrote:
>> There is no metadata length check before accessing struct xsk_tx_metadata
>> fields in both xdp_umem_reg() (registration) and
>> xsk_buff_valid_tx_metadata() (runtime).  Thus, an invalid small
>> tx_metadata_len (e.g. 8 bytes) passes registration and later causes an
>> out-of-bounds read when xsk_skb_metadata() accesses fields beyond the
>> declared metadata region.

[...]

> @@ -773,7 +773,7 @@ struct xdp_desc_ctx xp_raw_get_ctx(const struct xsk_buff_pool *pool, u64 addr)
>  	addr = __xp_raw_get_addr(pool, addr);
>  
>  	ret.dma = __xp_raw_get_dma(pool, addr);
> -	ret.meta = __xsk_buff_get_metadata(pool, __xp_raw_get_data(pool, addr));
> +	xsk_buff_read_metadata(pool, addr, &ret.meta);

That effectively disables some micro-optimizations that were the reason
I created this helper -- __xp_raw_get_data() is inline, while
xsk_buff_read_metadata() uses the external version, probably some other
stuff (note the underscored __xsk_buff_get_metadata()) =\ This function
expanded to have no external calls on clang -O2, with this patch I'm not
sure it would.

>  
>  	return ret;
>  }
Thanks,
Olek