Re: [PATCH 1/1] fuse: Add initial support for fs-verity

From: Eric Biggers <ebiggers@kernel.org>
To: Miklos Szeredi <miklos@szeredi.hu>
Cc: Richard Fung <richardfung@google.com>,
	linux-fsdevel@vger.kernel.org, fsverity@lists.linux.dev
Subject: Re: [PATCH 1/1] fuse: Add initial support for fs-verity
Date: Tue, 9 Apr 2024 19:50:18 -0400	[thread overview]
Message-ID: <20240409235018.GC1609@quark.localdomain> (raw)
In-Reply-To: <CAJfpegvtUywhs8vse1rZ6E=hnxUS6uo_eii-oHDmWd0hb35jjA@mail.gmail.com>

On Tue, Apr 09, 2024 at 04:50:10PM +0200, Miklos Szeredi wrote:
> On Thu, 28 Mar 2024 at 21:58, Richard Fung <richardfung@google.com> wrote:
> >
> > This adds support for the FS_IOC_ENABLE_VERITY and FS_IOC_MEASURE_VERITY
> > ioctls. The FS_IOC_READ_VERITY_METADATA is missing but from the
> > documentation, "This is a fairly specialized use case, and most fs-verity
> > users won’t need this ioctl."
> >
> > Signed-off-by: Richard Fung <richardfung@google.com>
> > ---
> >  fs/fuse/ioctl.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 52 insertions(+)
> >
> > diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
> > index 726640fa439e..a0e86c3de48f 100644
> > --- a/fs/fuse/ioctl.c
> > +++ b/fs/fuse/ioctl.c
> > @@ -8,6 +8,7 @@
> >  #include <linux/uio.h>
> >  #include <linux/compat.h>
> >  #include <linux/fileattr.h>
> > +#include <linux/fsverity.h>
> >
> >  static ssize_t fuse_send_ioctl(struct fuse_mount *fm, struct fuse_args *args,
> >                                struct fuse_ioctl_out *outarg)
> > @@ -227,6 +228,57 @@ long fuse_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg,
> >                         out_iov = iov;
> >                         out_iovs = 1;
> >                 }
> > +
> > +               /* For fs-verity, determine iov lengths from input */
> > +               switch (cmd) {
> > +               case FS_IOC_MEASURE_VERITY: {
> > +                       __u16 digest_size;
> > +                       struct fsverity_digest __user *uarg =
> > +               (struct fsverity_digest __user *)arg;
> > +
> > +                       if (copy_from_user(&digest_size, &uarg->digest_size,
> > +                                                sizeof(digest_size)))
> > +                               return -EFAULT;
> > +
> > +                       if (digest_size > SIZE_MAX - sizeof(struct fsverity_digest))
> > +                               return -EINVAL;
> > +
> > +                       iov->iov_len = sizeof(struct fsverity_digest) + digest_size;
> > +                       break;
> > +               }
> > +               case FS_IOC_ENABLE_VERITY: {
> > +                       struct fsverity_enable_arg enable;
> > +                       struct fsverity_enable_arg __user *uarg =
> > +               (struct fsverity_enable_arg __user *)arg;
> > +                       const __u32 max_buffer_len = FUSE_MAX_MAX_PAGES * PAGE_SIZE;
> > +
> > +                       if (copy_from_user(&enable, uarg, sizeof(enable)))
> > +                               return -EFAULT;
> > +
> > +                       if (enable.salt_size > max_buffer_len ||
> > +               enable.sig_size > max_buffer_len)
> > +                               return -ENOMEM;
> > +
> > +                       if (enable.salt_size > 0) {
> > +                               iov++;
> > +                               in_iovs++;
> > +
> > +                               iov->iov_base = u64_to_user_ptr(enable.salt_ptr);
> > +                               iov->iov_len = enable.salt_size;
> > +                       }
> > +
> > +                       if (enable.sig_size > 0) {
> > +                               iov++;
> > +                               in_iovs++;
> > +
> > +                               iov->iov_base = u64_to_user_ptr(enable.sig_ptr);
> > +                               iov->iov_len = enable.sig_size;
> > +                       }
> > +                       break;
> > +               }
> > +               default:
> > +                       break;
> > +               }
> >         }
> >
> >   retry:
> 
> I'm not thrilled by having ioctl specific handling added to the
> generic fuse ioctl code.
> 
> But more important is what  the fsverity folks think (CC's added).
> 

I am fine with having FUSE support passing through FS_IOC_MEASURE_VERITY and
FS_IOC_ENABLE_VERITY.

As you may have noticed, these ioctls are a bit more complex than the simple
ones that FUSE allows already.  The argument to FS_IOC_MEASURE_VERITY has a
variable-length trailing array, and the argument to FS_IOC_ENABLE_VERITY has up
to two pointers to other buffers.

I am hoping the FUSE folks have thoughts on what is the best way to support
ioctls like these.  I suspect that this patch (with the special handling in
FUSE) may be the only feasible approach, but I haven't properly investigated it.

- Eric