netdev - Re: [PATCH bpf-next v2 1/2] xsk: introduce local_cq for each af

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <0956384a0b29a47199ebc6f2d1e88c6e2da6eec21b145df1846e56e7097534a5@mail.kernel.org>
Date: Tue, 16 Dec 2025 05:46:24 +0000 (UTC)
From: bot+bpf-ci@...nel.org
To: kerneljasonxing@...il.com,davem@...emloft.net,edumazet@...gle.com,kuba@...nel.org,pabeni@...hat.com,bjorn@...nel.org,magnus.karlsson@...el.com,maciej.fijalkowski@...el.com,jonathan.lemon@...il.com,sdf@...ichev.me,ast@...nel.org,daniel@...earbox.net,hawk@...nel.org,john.fastabend@...il.com
Cc: bpf@...r.kernel.org,netdev@...r.kernel.org,kernelxing@...cent.com,ast@...nel.org,andrii@...nel.org,daniel@...earbox.net,martin.lau@...nel.org,eddyz87@...il.com,yonghong.song@...ux.dev,clm@...a.com,ihor.solodrai@...ux.dev
Subject: Re: [PATCH bpf-next v2 1/2] xsk: introduce local_cq for each af_xdp socket

> diff --git a/include/net/xdp_sock.h b/include/net/xdp_sock.h
> index 23e8861e8b25..c53ab2609d8c 100644
> --- a/include/net/xdp_sock.h
> +++ b/include/net/xdp_sock.h
> @@ -45,6 +45,12 @@ struct xsk_map {
>  	struct xdp_sock __rcu *xsk_map[];
>  };
>
> +struct local_cq {
> +	u32 prod ____cacheline_aligned_in_smp;
> +	u32 ring_mask ____cacheline_aligned_in_smp;
> +	u64 desc[] ____cacheline_aligned_in_smp;
> +};
> +
>  struct xdp_sock {
>  	/* struct sock must be the first member of struct xdp_sock */
>  	struct sock sk;
> @@ -89,6 +95,8 @@ struct xdp_sock {
>  	struct mutex mutex;
>  	struct xsk_queue *fq_tmp; /* Only as tmp storage before bind */
>  	struct xsk_queue *cq_tmp; /* Only as tmp storage before bind */
> +	/* Maintain addr(s) of descriptors locally */
> +	struct local_cq *lcq;
>  };

[ ... ]

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index f093c3453f64..9b637d5e4528 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c
> @@ -1212,6 +1212,30 @@ static void xsk_delete_from_maps(struct xdp_sock *xs)
>  	}
>  }
>
> +/* Initialize local compeletion queue for each xsk */
> +static int xsk_init_local_cq(struct xdp_sock *xs)
> +{
> +	struct xsk_queue *cq = xs->pool->cq;
> +	size_t size;
> +
> +	if (!cq || !cq->nentries)
> +		return -EINVAL;
> +
> +	size = struct_size_t(struct local_cq, desc, cq->nentries);
> +	xs->lcq = vmalloc(size);
> +	if (!xs->lcq)
> +		return -ENOMEM;
> +	xs->lcq->ring_mask = cq->nentries - 1;
> +	xs->lcq->prod = 0;
> +
> +	return 0;
> +}
> +
> +static void xsk_clear_local_cq(struct xdp_sock *xs)
> +{
> +	vfree(xs->lcq);

Can this lead to a double-free? When xsk_bind() fails after
xsk_init_local_cq() succeeds, xsk_clear_local_cq() is called in the
error path, freeing xs->lcq. However, xs->lcq is not set to NULL after
the vfree(). Later when the socket is closed, xsk_release() calls
xsk_clear_local_cq() again, causing vfree() to be called on the
already-freed pointer.

The path would be: xsk_bind()->xsk_init_local_cq() succeeds, allocating
xs->lcq. Then xp_assign_dev_shared() fails, triggering the error path
that calls xsk_clear_local_cq(). The bind fails and returns to userspace.
User closes the socket, triggering xsk_release()->xsk_clear_local_cq(),
which calls vfree() on the dangling xs->lcq pointer.

Should xs->lcq be set to NULL after the vfree to prevent this double-free?

> +}
> +
>  static int xsk_release(struct socket *sock)
>  {
>  	struct sock *sk = sock->sk;
> @@ -1241,6 +1265,7 @@ static int xsk_release(struct socket *sock)
>  	xskq_destroy(xs->tx);
>  	xskq_destroy(xs->fq_tmp);
>  	xskq_destroy(xs->cq_tmp);
> +	xsk_clear_local_cq(xs);
>
>  	sock_orphan(sk);
>  	sock->sk = NULL;

[ ... ]

> @@ -1360,9 +1385,18 @@ static int xsk_bind(struct socket *sock, struct sockaddr_unsized *addr, int addr
>  				goto out_unlock;
>  			}
>
> +			err = xsk_init_local_cq(xs);
> +			if (err) {
> +				xp_destroy(xs->pool);
> +				xs->pool = NULL;
> +				sockfd_put(sock);
> +				goto out_unlock;
> +			}
> +
>  			err = xp_assign_dev_shared(xs->pool, umem_xs, dev,
>  						   qid);
>  			if (err) {
> +				xsk_clear_local_cq(xs);
>  				xp_destroy(xs->pool);
>  				xs->pool = NULL;
>  				sockfd_put(sock);

[ ... ]


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/20257706446