Re: [PATCH v2] fuse: fix uring race condition for null dereference of fc

[Bernd Schubert <bernd.schubert@xxxxxxxxxxx>]

On 3/14/25 21:50, Joanne Koong wrote:
> There is a race condition leading to a kernel crash from a null
> dereference when attemping to access fc->lock in
> fuse_uring_create_queue(). fc may be NULL in the case where another
> thread is creating the uring in fuse_uring_create() and has set
> fc->ring but has not yet set ring->fc when fuse_uring_create_queue()
> reads ring->fc.
> 
> This fix passes fc to fuse_uring_create_queue() instead of accessing it
> through ring->fc.
> 
> Signed-off-by: Joanne Koong <joannelkoong@xxxxxxxxx>
> Fixes: 24fe962c86f5 ("fuse: {io-uring} Handle SQEs - register commands")
> ---
>  fs/fuse/dev_uring.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/fs/fuse/dev_uring.c b/fs/fuse/dev_uring.c
> index ab8c26042aa8..64f1ae308dc4 100644
> --- a/fs/fuse/dev_uring.c
> +++ b/fs/fuse/dev_uring.c
> @@ -250,10 +250,10 @@ static struct fuse_ring *fuse_uring_create(struct fuse_conn *fc)
>  	return res;
>  }
>  
> -static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_ring *ring,
> +static struct fuse_ring_queue *fuse_uring_create_queue(struct fuse_conn *fc,
> +						       struct fuse_ring *ring,
>  						       int qid)
>  {
> -	struct fuse_conn *fc = ring->fc;
>  	struct fuse_ring_queue *queue;
>  	struct list_head *pq;
>  
> @@ -1088,7 +1088,7 @@ static int fuse_uring_register(struct io_uring_cmd *cmd,
>  
>  	queue = ring->queues[qid];
>  	if (!queue) {
> -		queue = fuse_uring_create_queue(ring, qid);
> +		queue = fuse_uring_create_queue(fc, ring, qid);
>  		if (!queue)
>  			return err;
>  	}

I wonder if we could get more issues, 
fuse_uring_register()
{
	struct fuse_ring *ring = smp_load_acquire(&fc->ring);
	...
	if (qid >= ring->nr_queues) {

...
}

In fuse_uring_create()
{
...
	fc->ring = ring;
	ring->nr_queues = nr_queues;
	ring->fc = fc;
	ring->max_payload_sz = max_payload_size;
...
}


I.e. we cannot trust any of the values assigned after fc->ring? 
Sorry about this, I should have noticed before :(


Thanks,
Bernd