netdev - Re: [PATCH bpf-next v1 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: <3b8d7e918263a668f37e7c991de87d9473d01604ae5e61c5a40310ef00ffc017@mail.kernel.org>
Date: Tue,  9 Dec 2025 09:28:58 +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 v1 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;
> +};

The prod field is not initialized in xsk_init_local_cq(). Since vmalloc()
does not zero memory, prod will contain garbage values when first used.
The next commit in this series (722af5ef5295 "xsk: introduce a dedicated
local completion queue for each xsk") uses prod with lcq->prod++ before
any initialization occurs.

Should prod be initialized to 0 in xsk_init_local_cq()?

> +
>  struct xdp_sock {
>  	/* struct sock must be the first member of struct xdp_sock */
>  	struct sock sk;

[ ... ]

> diff --git a/net/xdp/xsk.c b/net/xdp/xsk.c
> index f093c3453f64..ce165d093105 100644
> --- a/net/xdp/xsk.c
> +++ b/net/xdp/xsk.c

[ ... ]

> +static int xsk_init_local_cq(struct xdp_sock *xs)
> +{
> +	u32 nentries = xs->pool->cq->nentries;
> +	size_t size = struct_size_t(struct local_cq, desc, nentries);
> +
> +	xs->lcq = vmalloc(size);
> +	if (!xs->lcq)
> +		return -ENOMEM;
> +	xs->lcq->ring_mask = nentries - 1;
> +
> +	return 0;
> +}

Can xs->pool->cq be NULL here? In the XDP_SHARED_UMEM path with
different queue_id or device (xsk_bind()->xp_create_and_assign_umem()->
xsk_init_local_cq()), xp_create_and_assign_umem() sets pool->cq =
xs->cq_tmp, but there's no validation that xs->cq_tmp is non-NULL
before calling xsk_init_local_cq().

For comparison, the non-shared umem path validates cq_tmp via
xsk_validate_queues() before reaching xsk_init_local_cq(). Should the
XDP_SHARED_UMEM path with different qid/dev also validate that cq_tmp
is set before calling xp_create_and_assign_umem()?

> +
> +static void xsk_clear_local_cq(struct xdp_sock *xs)
> +{
> +	vfree(xs->lcq);
> +}

[ ... ]

---
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/20058064407