Re: [PATCH v2 3/3] fuse: publish io-uring queues with release semantics

Joanne Koong <[email protected]> Thu, 16 Jul 2026 11:21:10 -0700
Newsgroups dev.linux.lists.fuse-devel,org.kernel.vger.stable
Message-ID <CAJnrk1YCSmK=7UwCsm8KF5kvndk+zM655whcHt85oKbpse8XYw@mail.gmail.com>
On Wed, Jul 15, 2026 at 3:54=E2=80=AFPM Bernd Schubert <[email protected]> =
wrote:
>
>
>
> On 7/15/26 19:43, Joanne Koong wrote:
> > fuse_uring_create_queue() initializes a fuse_ring_queue and then
> > publishes the pointer into ring->queues[qid] with WRITE_ONCE() under th=
e
> > fch->lock. There are several readers that may concurrently be fetching
> > that pointer locklessly and then deferencing it.
> >
> > WRITE_ONCE() doesn't ensure ordering of the queue's field
> > initialization before the ring->queues[qid] pointer assignment. The
> > queue must be published with smp_store_release() so the field
> > initialization is guaranteed to happen before.
> >
> > Readers in paths where the read may happen concurrently with the store
> > need to use READ_ONCE() because any race involving a plain access is
> > undefined.
> >
> > Fixes: 24fe962c86f5 ("fuse: {io-uring} Handle SQEs - register commands"=
)
> > Cc: [email protected]
> > Signed-off-by: Joanne Koong <[email protected]>
> > ---
> >  fs/fuse/dev_uring.c | 14 ++++++++------
> >  1 file changed, 8 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> > index 51f985154aa1..bf9d51f2a508 100644
> > --- a/fs/fuse/dev_uring.c
> > +++ b/fs/fuse/dev_uring.c
> > @@ -321,9 +321,11 @@ static struct fuse_ring_queue *fuse_uring_create_q=
ueue(struct fuse_ring *ring,
> >       }
> >
> >       /*
> > -      * write_once and lock as the caller mostly doesn't take the lock=
 at all
> > +      * fch->lock serializes concurrent creators for this qid.
> > +      * smp_store_release() are for the lockless readers who must see =
a
> > +      * fully initialized queue after &ring->queues[qid] is set
> >        */
> > -     WRITE_ONCE(ring->queues[qid], queue);
> > +     smp_store_release(&ring->queues[qid], queue);
> >       spin_unlock(&fch->lock);
> >
> >       return queue;
> > @@ -434,7 +436,7 @@ static void fuse_uring_log_ent_state(struct fuse_ri=
ng *ring)
> >       struct fuse_ring_ent *ent;
> >
> >       for (qid =3D 0; qid < ring->nr_queues; qid++) {
> > -             struct fuse_ring_queue *queue =3D ring->queues[qid];
> > +             struct fuse_ring_queue *queue =3D READ_ONCE(ring->queues[=
qid]);
> >
> >               if (!queue)
> >                       continue;
> > @@ -967,7 +969,7 @@ static int fuse_uring_commit_fetch(struct io_uring_=
cmd *cmd, int issue_flags,
> >       if (qid >=3D ring->nr_queues)
> >               return -EINVAL;
> >
> > -     queue =3D ring->queues[qid];
> > +     queue =3D READ_ONCE(ring->queues[qid]);
> >       if (!queue)
> >               return err;
> >       fpq =3D &queue->fpq;
> > @@ -1035,7 +1037,7 @@ static bool is_ring_ready(struct fuse_ring *ring,=
 int current_qid)
> >               if (current_qid =3D=3D qid)
> >                       continue;
> >
> > -             queue =3D ring->queues[qid];
> > +             queue =3D READ_ONCE(ring->queues[qid]);
> >               if (!queue) {
> >                       ready =3D false;
> >                       break;
> > @@ -1191,7 +1193,7 @@ static int fuse_uring_register(struct io_uring_cm=
d *cmd,
> >               return -EINVAL;
> >       }
> >
> > -     queue =3D ring->queues[qid];
> > +     queue =3D READ_ONCE(ring->queues[qid]);
> >       if (!queue) {
> >               queue =3D fuse_uring_create_queue(ring, qid);
> >               if (!queue)
>
>
> For consistency, maybe READ_ONCE/WRITE_ONCE in fuse_uring_destruct?

I didn't add it because I don't think it's necessary (because
fuse_uring_destruct can only run after the queue has been created),
but I can add it to make things more uniform, if you have a preference
for one way or the other. I'll send out v3 with this change.

Thanks,
Joanne