* [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation
@ 2026-07-22 4:13 Cen Zhang (Microsoft)
2026-07-22 17:55 ` Stanislav Fomichev
0 siblings, 1 reply; 9+ messages in thread
From: Cen Zhang (Microsoft) @ 2026-07-22 4:13 UTC (permalink / raw)
To: magnus.karlsson, maciej.fijalkowski, davem, edumazet, kuba,
pabeni
Cc: sdf, horms, netdev, bpf, linux-kernel, AutonomousCodeSecurity,
tgopinath, kys, blbllhy
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
^ permalink raw reply related [flat|nested] 9+ messages in thread* Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation 2026-07-22 4:13 [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation Cen Zhang (Microsoft) @ 2026-07-22 17:55 ` Stanislav Fomichev 2026-07-23 10:34 ` Maciej Fijalkowski 2026-07-23 15:16 ` Alexander Lobakin 0 siblings, 2 replies; 9+ messages in thread From: Stanislav Fomichev @ 2026-07-22 17:55 UTC (permalink / raw) To: Cen Zhang (Microsoft) Cc: magnus.karlsson, maciej.fijalkowski, davem, edumazet, kuba, pabeni, sdf, horms, netdev, bpf, linux-kernel, AutonomousCodeSecurity, tgopinath, kys 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; } ^ permalink raw reply related [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation 2026-07-22 17:55 ` Stanislav Fomichev @ 2026-07-23 10:34 ` Maciej Fijalkowski 2026-07-23 12:06 ` Cen Zhang 2026-07-23 15:16 ` Alexander Lobakin 1 sibling, 1 reply; 9+ messages in thread From: Maciej Fijalkowski @ 2026-07-23 10:34 UTC (permalink / raw) To: Stanislav Fomichev Cc: Cen Zhang (Microsoft), magnus.karlsson, davem, edumazet, kuba, pabeni, sdf, horms, netdev, bpf, linux-kernel, AutonomousCodeSecurity, tgopinath, kys 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; > } ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation 2026-07-23 10:34 ` Maciej Fijalkowski @ 2026-07-23 12:06 ` Cen Zhang 2026-07-23 12:18 ` Maciej Fijalkowski 0 siblings, 1 reply; 9+ messages in thread From: Cen Zhang @ 2026-07-23 12:06 UTC (permalink / raw) To: Maciej Fijalkowski, Stanislav Fomichev Cc: magnus.karlsson, davem, edumazet, kuba, pabeni, sdf, horms, netdev, bpf, linux-kernel, AutonomousCodeSecurity, tgopinath, kys 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation 2026-07-23 12:06 ` Cen Zhang @ 2026-07-23 12:18 ` Maciej Fijalkowski 2026-07-23 12:26 ` Cen Zhang 0 siblings, 1 reply; 9+ messages in thread From: Maciej Fijalkowski @ 2026-07-23 12:18 UTC (permalink / raw) To: Cen Zhang Cc: Stanislav Fomichev, magnus.karlsson, davem, edumazet, kuba, pabeni, sdf, horms, netdev, bpf, linux-kernel, AutonomousCodeSecurity, tgopinath, kys 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. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation 2026-07-23 12:18 ` Maciej Fijalkowski @ 2026-07-23 12:26 ` Cen Zhang 2026-07-23 17:14 ` Stanislav Fomichev 0 siblings, 1 reply; 9+ messages in thread From: Cen Zhang @ 2026-07-23 12:26 UTC (permalink / raw) To: Maciej Fijalkowski Cc: Stanislav Fomichev, magnus.karlsson, davem, edumazet, kuba, pabeni, sdf, horms, netdev, bpf, linux-kernel, AutonomousCodeSecurity, tgopinath, kys Not at all! Thanks for both your suggestion and Stanislav's rewrite! ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation 2026-07-23 12:26 ` Cen Zhang @ 2026-07-23 17:14 ` Stanislav Fomichev 0 siblings, 0 replies; 9+ messages in thread From: Stanislav Fomichev @ 2026-07-23 17:14 UTC (permalink / raw) To: Cen Zhang Cc: Maciej Fijalkowski, magnus.karlsson, davem, edumazet, kuba, pabeni, sdf, horms, netdev, bpf, linux-kernel, AutonomousCodeSecurity, tgopinath, kys On 07/23, Cen Zhang wrote: > Not at all! Thanks for both your suggestion and Stanislav's rewrite! SG, I can take it over.. ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation 2026-07-22 17:55 ` Stanislav Fomichev 2026-07-23 10:34 ` Maciej Fijalkowski @ 2026-07-23 15:16 ` Alexander Lobakin 2026-07-23 17:15 ` Stanislav Fomichev 1 sibling, 1 reply; 9+ messages in thread From: Alexander Lobakin @ 2026-07-23 15:16 UTC (permalink / raw) To: Stanislav Fomichev Cc: Cen Zhang (Microsoft), magnus.karlsson, maciej.fijalkowski, davem, edumazet, kuba, pabeni, sdf, horms, netdev, bpf, linux-kernel, AutonomousCodeSecurity, tgopinath, kys 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 ^ permalink raw reply [flat|nested] 9+ messages in thread
* Re: [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation 2026-07-23 15:16 ` Alexander Lobakin @ 2026-07-23 17:15 ` Stanislav Fomichev 0 siblings, 0 replies; 9+ messages in thread From: Stanislav Fomichev @ 2026-07-23 17:15 UTC (permalink / raw) To: Alexander Lobakin Cc: Cen Zhang (Microsoft), magnus.karlsson, maciej.fijalkowski, davem, edumazet, kuba, pabeni, sdf, horms, netdev, bpf, linux-kernel, AutonomousCodeSecurity, tgopinath, kys On 07/23, Alexander Lobakin wrote: > 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. Any better ideas on how to solve it? I can convert all non libeth stuff to these new xsk_buff_read_metadata helpers and keep xp_raw_get_ctx to you? Will that work? ^ permalink raw reply [flat|nested] 9+ messages in thread
end of thread, other threads:[~2026-07-23 17:15 UTC | newest] Thread overview: 9+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-22 4:13 [PATCH net v2] xsk: reject undersized tx_metadata at runtime validation Cen Zhang (Microsoft) 2026-07-22 17:55 ` Stanislav Fomichev 2026-07-23 10:34 ` Maciej Fijalkowski 2026-07-23 12:06 ` Cen Zhang 2026-07-23 12:18 ` Maciej Fijalkowski 2026-07-23 12:26 ` Cen Zhang 2026-07-23 17:14 ` Stanislav Fomichev 2026-07-23 15:16 ` Alexander Lobakin 2026-07-23 17:15 ` Stanislav Fomichev
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox