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 <[email protected]> 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/[email protected]?part=3
