* Re: [PATCH net] xsk: reject tx_metadata_len smaller than struct xsk_tx_metadata
2026-07-20 15:52 [PATCH net] xsk: reject tx_metadata_len smaller than struct xsk_tx_metadata Cen Zhang (Microsoft)
@ 2026-07-20 20:24 ` Stanislav Fomichev
0 siblings, 0 replies; 2+ messages in thread
From: Stanislav Fomichev @ 2026-07-20 20:24 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/20, Cen Zhang (Microsoft) wrote:
> xdp_umem_reg() validates tx_metadata_len for upper bound (<256) and
> alignment (%8) but not a lower bound. xsk_skb_metadata() computes
> meta = buffer - pool->tx_metadata_len then unconditionally accesses
> the full 24-byte struct xsk_tx_metadata, so any value less than
> sizeof(struct xsk_tx_metadata) allows an out-of-bounds read.
>
> 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)
>
> Add a lower-bound check in xdp_umem_reg() to reject tx_metadata_len
> values that cannot cover the full metadata struct.
>
> Fixes: 341ac980eab9 ("xsk: Support tx_metadata_len")
> Reported-by: AutonomousCodeSecurity@microsoft.com
> Signed-off-by: Cen Zhang (Microsoft) <blbllhy@gmail.com>
> ---
> net/xdp/xdp_umem.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/net/xdp/xdp_umem.c b/net/xdp/xdp_umem.c
> index 58da2f4f4397..d16ad9d8f919 100644
> --- a/net/xdp/xdp_umem.c
> +++ b/net/xdp/xdp_umem.c
> @@ -208,7 +208,8 @@ static int xdp_umem_reg(struct xdp_umem *umem, struct xdp_umem_reg *mr)
> return -EINVAL;
>
> if (mr->flags & XDP_UMEM_TX_METADATA_LEN) {
> - if (mr->tx_metadata_len >= 256 || mr->tx_metadata_len % 8)
> + if (mr->tx_metadata_len < sizeof(struct xsk_tx_metadata) ||
> + mr->tx_metadata_len >= 256 || mr->tx_metadata_len % 8)
> return -EINVAL;
> umem->tx_metadata_len = mr->tx_metadata_len;
> }
> --
> 2.53.0
>
This will make adding new tx metadata types harder (and will require all
userspace to be updated whenever we do so), will the following be
a bit nicer? (but, obviously, paying more per-packet at runtime)
(untested)
diff --git a/include/net/xdp_sock_drv.h b/include/net/xdp_sock_drv.h
index 46797645a0c2..b55b878949d5 100644
--- a/include/net/xdp_sock_drv.h
+++ b/include/net/xdp_sock_drv.h
@@ -260,9 +260,21 @@ 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, launch_time)))
+ return false;
+
+ return true;
}
static inline struct xsk_tx_metadata *
@@ -274,7 +286,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 +481,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) {
^ permalink raw reply related [flat|nested] 2+ messages in thread