Re: [PATCH 04/11] fuse: add a notification to add new iomap devices

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

On Thu, May 22, 2025 at 06:46:14PM +0200, Amir Goldstein wrote:
> On Thu, May 22, 2025 at 2:03 AM Darrick J. Wong <djwong@kernel.org> wrote:
> >
> > From: Darrick J. Wong <djwong@kernel.org>
> >
> > Add a new notification so that fuse servers can add extra block devices
> > to use with iomap.
> >
> > Signed-off-by: "Darrick J. Wong" <djwong@kernel.org>

<snip>

> > +int fuse_iomap_add_device(struct fuse_conn *fc,
> > +                         const struct fuse_iomap_add_device_out *outarg)
> > +{
> > +       struct file *file;
> > +       int ret;
> > +
> > +       if (!fc->iomap)
> > +               return -EINVAL;
> > +
> > +       if (outarg->reserved)
> > +               return -EINVAL;
> > +
> > +       CLASS(fd, somefd)(outarg->fd);
> > +       if (fd_empty(somefd))
> > +               return -EBADF;
> > +       file = fd_file(somefd);
> > +
> > +       if (!S_ISBLK(file_inode(file)->i_mode))
> > +               return -ENODEV;
> > +
> > +       down_read(&fc->killsb);
> > +       ret = __fuse_iomap_add_device(fc, file);
> > +       up_read(&fc->killsb);
> > +       if (ret < 0)
> > +               return ret;
> > +
> > +       return put_user(ret, outarg->map_dev);
> > +}
> 
> This very much reminds of FUSE_DEV_IOC_BACKING_OPEN
> that gives kernel an fd to remember for later file operations.
> 
> FUSE_DEV_IOC_BACKING_OPEN was implemented as an ioctl
> because of security concerns of passing an fd to the kernel via write().
> 
> Speaking of security concerns, we need to consider if this requires some
> privileges to allow setting up direct access to blockdev.

Yeah, I was assuming that if the fuse server can open the bdev, then
that's enough.  But I suppose I at least need to check that it's opened
in write mode too.

> But also, apart from the fact that those are block device fds,
> what does iomap_conn.files[] differ from fc->backing_files_map?

Oh, so that's what that does!  Yes, I'd rather pile on to that than
introduce more ABI. :)

> Miklos had envisioned this (backing blockdev) use case as one of the
> private cases of fuse passthrough.
> 
> Instead of identity mapping to backing file created at open time
> it's extent mapping to backing blockdev created at data access time.
> 
> I am not saying that you need to reuse anything from fuse passthrough
> code, because the use cases probably do not overlap, but hopefully,
> you can avoid falling into the same pits that we have already managed to avoid.

<nod> The one downside is that fsiomap requires the file to point at
either a block device or (in theory) a dax device, so we'd have to check
that on every access.  But aside from that I think I could reuse this
piece.  Thanks for bringing that to my attention! :)

--D

> Thanks,
> Amir.