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] [day] [month] [year] [list]

Message-ID: <CAL+tcoB9Lw24NCc-7ZDD2Qs9WUPs6G6Fwb8=47n2T=2Gwwf9AQ@mail.gmail.com>
Date: Tue, 16 Dec 2025 14:18:47 +0800
From: Jason Xing <kerneljasonxing@...il.com>
To: bot+bpf-ci@...nel.org
Cc: 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, bpf@...r.kernel.org, netdev@...r.kernel.org, 
	kernelxing@...cent.com, andrii@...nel.org, 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

On Tue, Dec 16, 2025 at 1:46 PM <bot+bpf-ci@...nel.org> wrote:
>
> > 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?

Exactly. Thanks. Will fix it.

Now it's time, I think, to run the AI reviewer locally before posting
patches since it's quite useful to spot some corner cases...

Thanks,
Jason