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

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 the
> 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_queue(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_ring *ring)
>  	struct fuse_ring_ent *ent;
>  
>  	for (qid = 0; qid < ring->nr_queues; qid++) {
> -		struct fuse_ring_queue *queue = ring->queues[qid];
> +		struct fuse_ring_queue *queue = 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 >= ring->nr_queues)
>  		return -EINVAL;
>  
> -	queue = ring->queues[qid];
> +	queue = READ_ONCE(ring->queues[qid]);
>  	if (!queue)
>  		return err;
>  	fpq = &queue->fpq;
> @@ -1035,7 +1037,7 @@ static bool is_ring_ready(struct fuse_ring *ring, int current_qid)
>  		if (current_qid == qid)
>  			continue;
>  
> -		queue = ring->queues[qid];
> +		queue = READ_ONCE(ring->queues[qid]);
>  		if (!queue) {
>  			ready = false;
>  			break;
> @@ -1191,7 +1193,7 @@ static int fuse_uring_register(struct io_uring_cmd *cmd,
>  		return -EINVAL;
>  	}
>  
> -	queue = ring->queues[qid];
> +	queue = READ_ONCE(ring->queues[qid]);
>  	if (!queue) {
>  		queue = fuse_uring_create_queue(ring, qid);
>  		if (!queue)


For consistency, maybe READ_ONCE/WRITE_ONCE in fuse_uring_destruct?
Otherwise,

Reviewed-by: Bernd Schubert <[email protected]>