Re: [PATCH 04/28] fuse: adapt FUSE_DEV_IOC_BACKING_{OPEN,CLOSE} to add new iomap devices

["Darrick J. Wong" <djwong@kernel.org>]

On Thu, Sep 18, 2025 at 08:42:08PM +0200, Amir Goldstein wrote:
> On Thu, Sep 18, 2025 at 8:17 PM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > On Wed, Sep 17, 2025 at 05:09:14AM +0200, Amir Goldstein wrote:
> > > On Tue, Sep 16, 2025 at 2:30 AM Darrick J. Wong <djwong@kernel.org> wrote:
> > > >
> > > > From: Darrick J. Wong <djwong@kernel.org>
> > > >
> > > > Enable the use of the backing file open/close ioctls so that fuse
> > > > servers can register block devices for use with iomap.
> > > >
> > > > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>
> > > > ---
> > > >  fs/fuse/fuse_i.h          |    5 ++
> > > >  include/uapi/linux/fuse.h |    3 +
> > > >  fs/fuse/Kconfig           |    1
> > > >  fs/fuse/backing.c         |   12 +++++
> > > >  fs/fuse/file_iomap.c      |   99 +++++++++++++++++++++++++++++++++++++++++----
> > > >  fs/fuse/trace.c           |    1
> > > >  6 files changed, 111 insertions(+), 10 deletions(-)
> > > >
> > > >
> > > > diff --git a/fs/fuse/fuse_i.h b/fs/fuse/fuse_i.h
> > > > index 389b123f0bf144..791f210c13a876 100644
> > > > --- a/fs/fuse/fuse_i.h
> > > > +++ b/fs/fuse/fuse_i.h
> > > > @@ -97,12 +97,14 @@ struct fuse_submount_lookup {
> > > >  };
> > > >
> > > >  struct fuse_conn;
> > > > +struct fuse_backing;
> > > >
> > > >  /** Operations for subsystems that want to use a backing file */
> > > >  struct fuse_backing_ops {
> > > >         int (*may_admin)(struct fuse_conn *fc, uint32_t flags);
> > > >         int (*may_open)(struct fuse_conn *fc, struct file *file);
> > > >         int (*may_close)(struct fuse_conn *fc, struct file *file);
> > > > +       int (*post_open)(struct fuse_conn *fc, struct fuse_backing *fb);
> > > >         unsigned int type;
> > > >  };
> > > >
> > > > @@ -110,6 +112,7 @@ struct fuse_backing_ops {
> > > >  struct fuse_backing {
> > > >         struct file *file;
> > > >         struct cred *cred;
> > > > +       struct block_device *bdev;
> > > >         const struct fuse_backing_ops *ops;
> > > >
> > > >         /** refcount */
> > > > @@ -1704,6 +1707,8 @@ static inline bool fuse_has_iomap(const struct inode *inode)
> > > >  {
> > > >         return get_fuse_conn_c(inode)->iomap;
> > > >  }
> > > > +
> > > > +extern const struct fuse_backing_ops fuse_iomap_backing_ops;
> > > >  #else
> > > >  # define fuse_iomap_enabled(...)               (false)
> > > >  # define fuse_has_iomap(...)                   (false)
> > > > diff --git a/include/uapi/linux/fuse.h b/include/uapi/linux/fuse.h
> > > > index 3634cbe602cd9c..3a367f387795ff 100644
> > > > --- a/include/uapi/linux/fuse.h
> > > > +++ b/include/uapi/linux/fuse.h
> > > > @@ -1124,7 +1124,8 @@ struct fuse_notify_retrieve_in {
> > > >
> > > >  #define FUSE_BACKING_TYPE_MASK         (0xFF)
> > > >  #define FUSE_BACKING_TYPE_PASSTHROUGH  (0)
> > > > -#define FUSE_BACKING_MAX_TYPE          (FUSE_BACKING_TYPE_PASSTHROUGH)
> > > > +#define FUSE_BACKING_TYPE_IOMAP                (1)
> > > > +#define FUSE_BACKING_MAX_TYPE          (FUSE_BACKING_TYPE_IOMAP)
> > > >
> > > >  #define FUSE_BACKING_FLAGS_ALL         (FUSE_BACKING_TYPE_MASK)
> > > >
> > > > diff --git a/fs/fuse/Kconfig b/fs/fuse/Kconfig
> > > > index 52e1a04183e760..baa38cf0f295ff 100644
> > > > --- a/fs/fuse/Kconfig
> > > > +++ b/fs/fuse/Kconfig
> > > > @@ -75,6 +75,7 @@ config FUSE_IOMAP
> > > >         depends on FUSE_FS
> > > >         depends on BLOCK
> > > >         select FS_IOMAP
> > > > +       select FUSE_BACKING
> > > >         help
> > > >           Enable fuse servers to operate the regular file I/O path through
> > > >           the fs-iomap library in the kernel.  This enables higher performance
> > > > diff --git a/fs/fuse/backing.c b/fs/fuse/backing.c
> > > > index 229c101ab46b0e..fc58636ac78eaa 100644
> > > > --- a/fs/fuse/backing.c
> > > > +++ b/fs/fuse/backing.c
> > > > @@ -89,6 +89,10 @@ fuse_backing_ops_from_map(const struct fuse_backing_map *map)
> > > >  #ifdef CONFIG_FUSE_PASSTHROUGH
> > > >         case FUSE_BACKING_TYPE_PASSTHROUGH:
> > > >                 return &fuse_passthrough_backing_ops;
> > > > +#endif
> > > > +#ifdef CONFIG_FUSE_IOMAP
> > > > +       case FUSE_BACKING_TYPE_IOMAP:
> > > > +               return &fuse_iomap_backing_ops;
> > > >  #endif
> > > >         default:
> > > >                 break;
> > > > @@ -137,8 +141,16 @@ int fuse_backing_open(struct fuse_conn *fc, struct fuse_backing_map *map)
> > > >         fb->file = file;
> > > >         fb->cred = prepare_creds();
> > > >         fb->ops = ops;
> > > > +       fb->bdev = NULL;
> > > >         refcount_set(&fb->count, 1);
> > > >
> > > > +       res = ops->post_open ? ops->post_open(fc, fb) : 0;
> > > > +       if (res) {
> > > > +               fuse_backing_free(fb);
> > > > +               fb = NULL;
> > > > +               goto out;
> > > > +       }
> > > > +
> > > >         res = fuse_backing_id_alloc(fc, fb);
> > > >         if (res < 0) {
> > > >                 fuse_backing_free(fb);
> > > > diff --git a/fs/fuse/file_iomap.c b/fs/fuse/file_iomap.c
> > > > index e7d19e2aee4541..3a4161633add0e 100644
> > > > --- a/fs/fuse/file_iomap.c
> > > > +++ b/fs/fuse/file_iomap.c
> > > > @@ -319,10 +319,6 @@ static inline bool fuse_iomap_check_mapping(const struct inode *inode,
> > > >                 return false;
> > > >         }
> > > >
> > > > -       /* XXX: we don't support devices yet */
> > > > -       if (BAD_DATA(map->dev != FUSE_IOMAP_DEV_NULL))
> > > > -               return false;
> > > > -
> > > >         /* No overflows in the device range, if supplied */
> > > >         if (map->addr != FUSE_IOMAP_NULL_ADDR &&
> > > >             BAD_DATA(check_add_overflow(map->addr, map->length, &end)))
> > > > @@ -334,6 +330,7 @@ static inline bool fuse_iomap_check_mapping(const struct inode *inode,
> > > >  /* Convert a mapping from the server into something the kernel can use */
> > > >  static inline void fuse_iomap_from_server(struct inode *inode,
> > > >                                           struct iomap *iomap,
> > > > +                                         const struct fuse_backing *fb,
> > > >                                           const struct fuse_iomap_io *fmap)
> > > >  {
> > > >         iomap->addr = fmap->addr;
> > > > @@ -341,7 +338,9 @@ static inline void fuse_iomap_from_server(struct inode *inode,
> > > >         iomap->length = fmap->length;
> > > >         iomap->type = fuse_iomap_type_from_server(fmap->type);
> > > >         iomap->flags = fuse_iomap_flags_from_server(fmap->flags);
> > > > -       iomap->bdev = inode->i_sb->s_bdev; /* XXX */
> > > > +
> > > > +       iomap->bdev = fb ? fb->bdev : NULL;
> > > > +       iomap->dax_dev = NULL;
> > > >  }
> > > >
> > > >  /* Convert a mapping from the kernel into something the server can use */
> > > > @@ -392,6 +391,27 @@ static inline bool fuse_is_iomap_file_write(unsigned int opflags)
> > > >         return opflags & (IOMAP_WRITE | IOMAP_ZERO | IOMAP_UNSHARE);
> > > >  }
> > > >
> > > > +static inline struct fuse_backing *
> > > > +fuse_iomap_find_dev(struct fuse_conn *fc, const struct fuse_iomap_io *map)
> > > > +{
> > > > +       struct fuse_backing *ret = NULL;
> > > > +
> > > > +       if (map->dev != FUSE_IOMAP_DEV_NULL && map->dev < INT_MAX)
> > > > +               ret = fuse_backing_lookup(fc, &fuse_iomap_backing_ops,
> > > > +                                         map->dev);
> > > > +
> > > > +       switch (map->type) {
> > > > +       case FUSE_IOMAP_TYPE_MAPPED:
> > > > +       case FUSE_IOMAP_TYPE_UNWRITTEN:
> > > > +               /* Mappings backed by space must have a device/addr */
> > > > +               if (BAD_DATA(ret == NULL))
> > > > +                       return ERR_PTR(-EFSCORRUPTED);
> > > > +               break;
> > > > +       }
> > > > +
> > > > +       return ret;
> > > > +}
> > > > +
> > > >  static int fuse_iomap_begin(struct inode *inode, loff_t pos, loff_t count,
> > > >                             unsigned opflags, struct iomap *iomap,
> > > >                             struct iomap *srcmap)
> > > > @@ -405,6 +425,8 @@ static int fuse_iomap_begin(struct inode *inode, loff_t pos, loff_t count,
> > > >         };
> > > >         struct fuse_iomap_begin_out outarg = { };
> > > >         struct fuse_mount *fm = get_fuse_mount(inode);
> > > > +       struct fuse_backing *read_dev = NULL;
> > > > +       struct fuse_backing *write_dev = NULL;
> > > >         FUSE_ARGS(args);
> > > >         int err;
> > > >
> > > > @@ -431,24 +453,44 @@ static int fuse_iomap_begin(struct inode *inode, loff_t pos, loff_t count,
> > > >         if (err)
> > > >                 return err;
> > > >
> > > > +       read_dev = fuse_iomap_find_dev(fm->fc, &outarg.read);
> > > > +       if (IS_ERR(read_dev))
> > > > +               return PTR_ERR(read_dev);
> > > > +
> > > >         if (fuse_is_iomap_file_write(opflags) &&
> > > >             outarg.write.type != FUSE_IOMAP_TYPE_PURE_OVERWRITE) {
> > > > +               /* open the write device */
> > > > +               write_dev = fuse_iomap_find_dev(fm->fc, &outarg.write);
> > > > +               if (IS_ERR(write_dev)) {
> > > > +                       err = PTR_ERR(write_dev);
> > > > +                       goto out_read_dev;
> > > > +               }
> > > > +
> > > >                 /*
> > > >                  * For an out of place write, we must supply the write mapping
> > > >                  * via @iomap, and the read mapping via @srcmap.
> > > >                  */
> > > > -               fuse_iomap_from_server(inode, iomap, &outarg.write);
> > > > -               fuse_iomap_from_server(inode, srcmap, &outarg.read);
> > > > +               fuse_iomap_from_server(inode, iomap, write_dev, &outarg.write);
> > > > +               fuse_iomap_from_server(inode, srcmap, read_dev, &outarg.read);
> > > >         } else {
> > > >                 /*
> > > >                  * For everything else (reads, reporting, and pure overwrites),
> > > >                  * we can return the sole mapping through @iomap and leave
> > > >                  * @srcmap unchanged from its default (HOLE).
> > > >                  */
> > > > -               fuse_iomap_from_server(inode, iomap, &outarg.read);
> > > > +               fuse_iomap_from_server(inode, iomap, read_dev, &outarg.read);
> > > >         }
> > > >
> > > > -       return 0;
> > > > +       /*
> > > > +        * XXX: if we ever want to support closing devices, we need a way to
> > > > +        * track the fuse_backing refcount all the way through bio endios.
> > > > +        * For now we put the refcount here because you can't remove an iomap
> > > > +        * device until unmount time.
> > > > +        */
> > > > +       fuse_backing_put(write_dev);
> > > > +out_read_dev:
> > > > +       fuse_backing_put(read_dev);
> > > > +       return err;
> > > >  }
> > > >
> > > >  /* Decide if we send FUSE_IOMAP_END to the fuse server */
> > > > @@ -523,3 +565,42 @@ const struct iomap_ops fuse_iomap_ops = {
> > > >         .iomap_begin            = fuse_iomap_begin,
> > > >         .iomap_end              = fuse_iomap_end,
> > > >  };
> > > > +
> > > > +static int fuse_iomap_may_admin(struct fuse_conn *fc, unsigned int flags)
> > > > +{
> > > > +       if (!fc->iomap)
> > > > +               return -EPERM;
> > > > +
> > >
> > > IIRC, on RFC I asked why is iomap exempt from CAP_SYS_ADMIN
> > > check. If there was a good reason, I forgot it.
> >
> > CAP_SYS_ADMIN means that the fuse server (or the fuservicemount helper)
> > can make quite a lot of other changes to the system that are not at all
> > related to being a filesystem.  I'd rather not use that one.
> >
> > Instead I require CAP_SYS_RAWIO to enable fc->iomap, so that the fuse
> > server has to have *some* privilege, but only enough to write to raw
> > block devices since that's what iomap does.
> >
> > > The problem is that while fuse-iomap fs is only expected to open
> > > a handful of backing devs, we would like to prevent abuse of this ioctl
> > > by a buggy or malicious user.
> > >
> > > I think that if you want to avoid CAP_SYS_ADMIN here you should
> > > enforce a limit on the number of backing bdevs.
> > >
> > > If you accept my suggestion to mutually exclude passthrough and
> > > iomap features per fs, then you'd just need to keep track on numbers
> > > of fuse_backing ids and place a limit for iomap fs.
> > >
> > > BTW, I think it is enough keep track of the number of backing ids
> > > and no need to keep track of the number of fuse_backing objects
> > > (which can outlive a backing id), because an "anonymous" fuse_backing
> > > object is always associated with an open fuse file - that's the same as
> > > an overlayfs backing file, which is not accounted for in ulimit.
> >
> > How about restricting the backing ids to RLIMIT_NOFILE?  The @end param
> > to idr_alloc_cyclic constrains them in exactly that way.
> 
> IDK. My impression was that Miklos didn't like having a large number
> of unaccounted files, but it's up to him.
> 
> Do you have an estimate on the worst case number of backing blockdev
> for fuse iomap?

It's the upper limit on the number of block devices that you can attach
to a multi-device filesystem for use with files.  For ext4 it's 1, for
XFS it would be 2, for btrfs I have no idea.

--D

> Thanks,
> Amir.