Commit e0f22902 authored by Jason Xing's avatar Jason Xing Committed by Jakub Kicinski
Browse files

xsk: fix xsk_addrs slab leak on multi-buffer error path

When xsk_build_skb() / xsk_build_skb_zerocopy() sees the first
continuation descriptor, it promotes destructor_arg from an inlined
address to a freshly allocated xsk_addrs (num_descs = 1). The counter
is bumped to >= 2 only at the very end of a successful build (by calling
xsk_inc_num_desc()).

If the build fails in between (e.g. alloc_page() returns NULL with
-EAGAIN, or the MAX_SKB_FRAGS overflow hits), we jump to free_err, skip
calling xsk_inc_num_desc() to increment num_descs and leave the half-built
skb attached to xs->skb for the app to retry. The skb now has
1) destructor_arg = a real xsk_addrs pointer,
2) num_descs = 1

If the app never retries and just close()s the socket, xsk_release()
calls xsk_drop_skb() -> xsk_consume_skb(), which decides whether to
free xsk_addrs by testing num_descs > 1:

    if (unlikely(num_descs > 1))
        kmem_cache_free(xsk_tx_generic_cache, destructor_arg);

Because num_descs is exactly 1 the branch is skipped and the
xsk_addrs object is leaked to the xsk_tx_generic_cache slab.

Fix it by directly testing if destructor_arg is still addr. Or else it
is modified and used to store the newly allocated memory from
xsk_tx_generic_cache regardless of increment of num_desc, which we
need to handle.

Closes: https://lore.kernel.org/all/20260419045824.D9E5EC2BCAF@smtp.kernel.org/


Fixes: 0ebc27a4 ("xsk: avoid data corruption on cq descriptor number")
Acked-by: default avatarStanislav Fomichev <sdf@fomichev.me>
Signed-off-by: default avatarJason Xing <kernelxing@tencent.com>
Reviewed-by: default avatarAlexander Lobakin <aleksander.lobakin@intel.com>
Link: https://patch.msgid.link/20260502200722.53960-8-kerneljasonxing@gmail.com


Signed-off-by: default avatarJakub Kicinski <kuba@kernel.org>
parent 8c2cff50
Loading
Loading
Loading
Loading
+2 −2
Original line number Diff line number Diff line
@@ -685,7 +685,7 @@ static void xsk_cq_submit_addr_locked(struct xsk_buff_pool *pool,
	spin_lock_irqsave(&pool->cq_prod_lock, flags);
	idx = xskq_get_prod(pool->cq);

	if (unlikely(num_descs > 1)) {
	if (unlikely(!xsk_skb_destructor_is_addr(skb))) {
		xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;

		for (i = 0; i < num_descs; i++) {
@@ -740,7 +740,7 @@ static void xsk_consume_skb(struct sk_buff *skb)
	u32 num_descs = xsk_get_num_desc(skb);
	struct xsk_addrs *xsk_addr;

	if (unlikely(num_descs > 1)) {
	if (unlikely(!xsk_skb_destructor_is_addr(skb))) {
		xsk_addr = (struct xsk_addrs *)skb_shinfo(skb)->destructor_arg;
		kmem_cache_free(xsk_tx_generic_cache, xsk_addr);
	}