Thank you for your contribution! Sashiko AI review found 3 potential issue(s) to consider: - [High] fuse: NULL pointer dereference in fuse_notify_map() due to missing backing_id_64 check - [High] fuse: type confusion and wild pointer dereference in fuse_ext_map_is_dax() - [Medium] fuse: duplicate backing_id insertion due to rhashtable_insert_fast usage --
commit 7c30614b0de0feab07ac16cfbaac90b3eedab05c Author: Miklos Szeredi <[email protected]> fuse: add extent map data structure Add support for creating and managing extent maps that map file regions to dax device regions. Introduce FUSE_NOTIFY_BACKING_MAP for populating extent maps from userspace. > diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c > index 0ed850ddf8cde..b7ebc951dc313 100644 > --- a/fs/fuse/backing.c > +++ b/fs/fuse/backing.c [ ... ] > @@ -84,7 +88,7 @@ static const struct rhashtable_params fuse_backing_prm = { > .key_len = sizeof_field(struct fuse_backing, backing_id), > }; > > -static int fuse_backing_add_64(struct fuse_conn *fc, struct fuse_backing *fb) > +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] Will this blindly insert duplicate backing IDs into the hash table? Calling rhashtable_insert_fast() passes NULL as the key to the internal __rhashtable_insert_fast(), which short-circuits the duplicate comparison logic. Since the newly added FUSE_NOTIFY_BACKING_MAP accepts user-provided backing IDs and inserts them here, could an untrusted FUSE server send multiple requests with the same backing ID and cause inconsistent state or unbounded memory accumulation? Should this use rhashtable_lookup_insert_fast() instead to properly check for uniqueness? > diff --git a/fs/fuse/ext_map.c b/fs/fuse/ext_map.c > new file mode 100644 > index 0000000000000..39d7bb521873c > --- /dev/null > +++ b/fs/fuse/ext_map.c [ ... ] > +bool fuse_ext_map_is_dax(struct fuse_backing *fb) > +{ > + struct fuse_iext *fie; > + > + if (WARN_ON(RB_EMPTY_ROOT(&fb->extents))) > + return false; > + > + fie = rb_entry(fb->extents.rb_node, typeof(*fie), rb); > + return fie->backing->type == FUSE_BACKING_DAXDEV; > +} [Severity: High] Does this safely handle other backing types passed in via the fb pointer? The extents field is part of an anonymous union overlaying dax_dev. If this function is called on a FUSE_BACKING_DAXDEV backing, it will treat the dax_dev pointer as an rb_root. If dax_dev is non-NULL, RB_EMPTY_ROOT() returns false, and the code then casts the pointer via rb_entry() and dereferences it. Can this cause an out-of-bounds read or a wild pointer dereference? Should there be a check to ensure fb->type == FUSE_BACKING_EXTMAP before attempting to access fb->extents? > diff --git a/fs/fuse/notify.c b/fs/fuse/notify.c > index 93e916a16ac93..7c427f88bbc52 100644 > --- a/fs/fuse/notify.c > +++ b/fs/fuse/notify.c [ ... ] > +static int fuse_notify_map(struct fuse_conn *fc, unsigned int size, > + struct fuse_copy_state *cs) > +{ > + struct fuse_notify_backing_map_out outarg; > + struct fuse_extent *ext __free(kvfree) = NULL; > + int err; [ ... ] > + if (outarg.flags & ~FUSE_BACKING_MAP_CREATE) > + return -EINVAL; > + > + if (!IS_ENABLED(CONFIG_FUSE_PASSTHROUGH)) > + return -EOPNOTSUPP; > + > + ext = kvmalloc_objs(*ext, outarg.num_extents); > + if (!ext) > + return -ENOMEM; > + > + err = fuse_copy_one(cs, ext, size); > + if (err) > + return err; > + > + return fuse_ext_map_populate(fc, &outarg, ext); > +} [Severity: High] Is a check for fc->backing_id_64 missing here before attempting to populate the extent map? If the backing_id_64 feature was not negotiated with the FUSE server during connection initialization, fc->backing_64_ht will remain uninitialized (zero-filled) because fuse_backing_files_init_64() is never called. When fuse_ext_map_populate() subsequently calls fuse_backing_add_64(), it will use rhashtable_insert_fast() on an uninitialized hash table. Could this result in a guaranteed NULL pointer dereference on ht->tbl, allowing a buggy or malicious FUSE server to crash the kernel? -- Sashiko AI review ยท https://sashiko.dev/#/patchset/[email protected]?part=7
