* [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-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 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-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