Re: [PATCH v2 3/8] fuse: support 64 bit, server allocated backing ID

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider:
- [High] fuse: unchecked rhashtable_init() return leads to NULL dereference
- [High] fuse: race between process_init_reply() and ioctl corrupts IDR/rhashtable union
- [Medium] fuse: duplicate 64-bit backing IDs lead to memory leaks
--

commit 4b1027b3a4203f8caa807855de45ddb14b02bbb9
Author: Miklos Szeredi <mszeredi@redhat.com>

fuse: support 64 bit, server allocated backing ID

This commit adds support for server allocated 64-bit backing IDs
alongside the existing kernel allocated 32-bit IDs. It introduces
FUSE_DEV_IOC_BACKING_CREATE and FUSE_NOTIFY_BACKING_REMOVE for
managing the 64-bit backing IDs.

> diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> index 433fa3098d71f..3c879df7989c9 100644
> --- a/fs/fuse/backing.c
> +++ b/fs/fuse/backing.c
[ ... ]
> +static int fuse_backing_add_64(struct fuse_conn *fc, struct fuse_backing *fb)
> +{
> +	return rhashtable_insert_fast(&fc->backing_64_ht, &fb->hash_node, fuse_backing_prm);
>  }

[Severity: Medium]
Does this allow duplicate backing IDs, leading to a memory leak?

rhashtable_insert_fast() does not check for duplicate keys. If a user
invokes FUSE_DEV_IOC_BACKING_CREATE multiple times with the exact same
64-bit map->backing_id, they will all be inserted.

When removing via fuse_backing_close_64(), rhashtable_remove_fast() will
only retrieve and delete one instance, leaving the duplicates orphaned and
leaking the struct fuse_backing objects.

[ ... ]
> +void fuse_backing_files_init_64(struct fuse_conn *fc)
> +{
> +	rhashtable_init(&fc->backing_64_ht, &fuse_backing_prm);
> +	fc->backing_id_64 = true;
>  }

[Severity: High]
What happens if rhashtable_init() fails here due to memory pressure?

Since the return value is ignored, fc->backing_id_64 = true is set
unconditionally. If rhashtable_init() returns an error, the hash table
remains uninitialized.

A subsequent ioctl triggering fuse_backing_open_64() would call
rhashtable_insert_fast(), which dereferences the NULL ht->tbl, resulting
in a kernel panic.

> diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> index 2a1aadc08e2a8..9b65543c6cc79 100644
> --- a/fs/fuse/fuse_i.h
> +++ b/fs/fuse/fuse_i.h
[ ... ]
> @@ -770,8 +775,14 @@ struct fuse_conn {
>  	struct fuse_sync_bucket __rcu *curr_bucket;
>  
>  #ifdef CONFIG_FUSE_PASSTHROUGH
> -	/** @backing_files_map: IDR for backing files ids */
> -	struct idr backing_files_map;
> +	/* Selected by backing_id_64 */
> +	union {
> +		/** @backing_files_map: IDR for backing files ids */
> +		struct idr backing_files_map;
> +
> +		/** @backing_64_ht: 64 bit ID lookup hash table */
> +		struct rhashtable backing_64_ht;
> +	};
>  #endif
>  };
>  
> diff --git a/fs/fuse/inode.c b/fs/fuse/inode.c
> index cbb10e19e7e86..bb76bddfc2417 100644
> --- a/fs/fuse/inode.c
> +++ b/fs/fuse/inode.c
[ ... ]
> @@ -1408,13 +1408,15 @@ static void process_init_reply(struct fuse_args *args, int error)
>  			 * them together.
>  			 */
>  			if (IS_ENABLED(CONFIG_FUSE_PASSTHROUGH) &&
> -			    (flags & FUSE_PASSTHROUGH) &&
> +			    (flags & (FUSE_PASSTHROUGH | FUSE_PASSTHROUGH_V2)) &&
>  			    arg->max_stack_depth > 0 &&
>  			    arg->max_stack_depth <= FILESYSTEM_MAX_STACK_DEPTH &&
>  			    !(flags & FUSE_WRITEBACK_CACHE))  {
>  				fc->passthrough = 1;
>  				fc->max_stack_depth = arg->max_stack_depth;
>  				fm->sb->s_stack_depth = arg->max_stack_depth;
> +				if (flags & FUSE_PASSTHROUGH_V2)
> +					fuse_backing_files_init_64(fc);

[Severity: High]
Can this sequence cause memory corruption in the union?

If fc->passthrough = 1 is set locklessly before fuse_backing_files_init_64(fc)
completes, a concurrent FUSE_DEV_IOC_BACKING_OPEN ioctl in fuse_backing_open()
could see fc->passthrough == 1 and fc->backing_id_64 == 0.

It would proceed to call fuse_backing_id_alloc(), modifying the 32-bit
backing_files_map IDR. Meanwhile, fuse_backing_files_init_64() executes
rhashtable_init(&fc->backing_64_ht), which would blindly overwrite the very
same memory union, corrupting the IDR state.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001150935.655979-1-mszeredi@redhat.com?part=3