Thank you for your contribution! Sashiko AI review found 2 potential issue(s) 
to consider:
- [High] fuse: sleep in RCU read-side critical section during backing files 
cleanup
- [Medium] fuse: missing overflow validation on ext->addr and ext->length
--

commit 791e33c26af3a517c133a125042b17d8f5ff57ec
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 c852f04989619..e954391d696d0 100644
> --- a/fs/fuse/backing.c
> +++ b/fs/fuse/backing.c
[ ... ]
> @@ -306,12 +310,36 @@ static void fuse_backing_rht_free(void *p, void *data)
>  
>  void fuse_backing_files_free(struct fuse_conn *fc)
>  {
[ ... ]
> +     /*
> +      * extents are referencing other backings, put these refs before
> +      * destroying the backings themselves
> +      */
> +     rhashtable_walk_enter(&fc->backing_64_ht, &iter);
> +     rhashtable_walk_start(&iter);
> +     while ((fb = rhashtable_walk_next(&iter))) {
> +             if (IS_ERR(fb)) {
> +                     if (PTR_ERR(fb) == -EAGAIN)
> +                             continue;
> +                     break;
> +             }
> +             if (fb->type == FUSE_BACKING_EXTMAP) {
> +                     fuse_ext_map_destroy(&fb->extents);

[Severity: High]
Does this code sleep in an RCU read-side critical section?

rhashtable_walk_start() explicitly acquires the RCU read lock.
When this code drops the final reference to the underlying DAXDEV backing,
the call chain is:

fuse_ext_map_destroy()
  fuse_backing_put()
    fuse_backing_free()
      fs_put_dax()
        iput()

Since iput() can sleep, it appears this could trigger 'scheduling while
atomic' panics or lockdep warnings when unmounting a FUSE passthrough
connection.

> +                     fb->extents.rb_node = NULL;
> +             }
> +     }
> +     rhashtable_walk_stop(&iter);
> +     rhashtable_walk_exit(&iter);
> +
> +     rhashtable_free_and_destroy(&fc->backing_64_ht, fuse_backing_rht_free, 
> NULL);
>  }

> diff --git a/fs/fuse/ext_map.c b/fs/fuse/ext_map.c
> new file mode 100644
> index 0000000000000..ab01d678d08e5
> --- /dev/null
> +++ b/fs/fuse/ext_map.c
[ ... ]
> +static int fuse_add_extent(struct fuse_conn *fc, struct rb_root *extents,
> +                        struct fuse_extent *ext)
> +{
> +     struct fuse_iext *new_fie __free(kfree) = kzalloc_obj(*new_fie);
> +     struct rb_node *parent = NULL, **link = &extents->rb_node;
> +     loff_t end;
[ ... ]
> +     if (overflows_type(ext->offset, loff_t) ||
> +         check_add_overflow(ext->offset, ext->length, &end))
> +             return fuse_EIO("offset overflow");
> +
> +     new_fie->start = ext->offset;
> +     new_fie->end = end;
> +     new_fie->backing_offset = ext->addr;

[Severity: Medium]
Is it possible for the physical address calculation to wrap around?

The bounds check above validates ext->offset, but appears to omit checking
ext->addr + ext->length for overflow.

Could a user-space process supplying an ext->addr near U64_MAX cause an
integer overflow on the physical address range calculation, resulting in a
non-contiguous backing mapping?

> +
> +     while (*link) {

-- 
Sashiko AI review ยท 
https://sashiko.dev/#/patchset/[email protected]?part=6

Reply via email to