linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/1] fuse: Add initial support for fs-verity
@ 2024-03-28 20:58 Richard Fung
  2024-03-28 20:58 ` [PATCH 1/1] " Richard Fung
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Richard Fung @ 2024-03-28 20:58 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, Richard Fung

Hi! I am trying to add fs-verity support for virtiofs.

The main complication is that fs-verity ioctls do not have constant
iovec lengths, so simply using _IOC_SIZE like with other ioctls does not
work.

Note this doesn't include support for FS_IOC_READ_VERITY_METADATA.  I
don't know of an existing use of it and I figured it would be better
not to include code that wouldn't be used and tested. However if you feel
like it should be added let me know.

(Also, apologies for any mistakes as this is my first Linux patch!)

Richard Fung (1):
  fuse: Add initial support for fs-verity

 fs/fuse/ioctl.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

-- 
2.44.0.478.gd926399ef9-goog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH 1/1] fuse: Add initial support for fs-verity
  2024-03-28 20:58 [PATCH 0/1] fuse: Add initial support for fs-verity Richard Fung
@ 2024-03-28 20:58 ` Richard Fung
  2024-04-02 16:16   ` Richard Fung
  2024-04-09 14:50   ` Miklos Szeredi
  2024-04-09 23:52 ` [PATCH 0/1] " Eric Biggers
  2024-04-16  0:16 ` [PATCH v2] " Richard Fung
  2 siblings, 2 replies; 14+ messages in thread
From: Richard Fung @ 2024-03-28 20:58 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, Richard Fung

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:
-- 
2.44.0.478.gd926399ef9-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/1] fuse: Add initial support for fs-verity
  2024-03-28 20:58 ` [PATCH 1/1] " Richard Fung
@ 2024-04-02 16:16   ` Richard Fung
  2024-04-09 14:50   ` Miklos Szeredi
  1 sibling, 0 replies; 14+ messages in thread
From: Richard Fung @ 2024-04-02 16:16 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, Tiffany Yang, fuse-devel

Realized I should have added fuse-devel@

Could you please have a look?

On Thu, Mar 28, 2024 at 1:58 PM 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:
> --
> 2.44.0.478.gd926399ef9-goog
>

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/1] fuse: Add initial support for fs-verity
  2024-03-28 20:58 ` [PATCH 1/1] " Richard Fung
  2024-04-02 16:16   ` Richard Fung
@ 2024-04-09 14:50   ` Miklos Szeredi
  2024-04-09 23:50     ` Eric Biggers
  1 sibling, 1 reply; 14+ messages in thread
From: Miklos Szeredi @ 2024-04-09 14:50 UTC (permalink / raw)
  To: Richard Fung; +Cc: linux-fsdevel, fsverity, Eric Biggers

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).

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/1] fuse: Add initial support for fs-verity
  2024-04-09 14:50   ` Miklos Szeredi
@ 2024-04-09 23:50     ` Eric Biggers
  2024-04-11  6:06       ` Miklos Szeredi
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2024-04-09 23:50 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: Richard Fung, linux-fsdevel, fsverity

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

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 0/1] fuse: Add initial support for fs-verity
  2024-03-28 20:58 [PATCH 0/1] fuse: Add initial support for fs-verity Richard Fung
  2024-03-28 20:58 ` [PATCH 1/1] " Richard Fung
@ 2024-04-09 23:52 ` Eric Biggers
  2024-04-16  0:16 ` [PATCH v2] " Richard Fung
  2 siblings, 0 replies; 14+ messages in thread
From: Eric Biggers @ 2024-04-09 23:52 UTC (permalink / raw)
  To: Richard Fung; +Cc: Miklos Szeredi, linux-fsdevel

On Thu, Mar 28, 2024 at 08:58:21PM +0000, Richard Fung wrote:
> Hi! I am trying to add fs-verity support for virtiofs.
> 
> The main complication is that fs-verity ioctls do not have constant
> iovec lengths, so simply using _IOC_SIZE like with other ioctls does not
> work.
> 
> Note this doesn't include support for FS_IOC_READ_VERITY_METADATA.  I
> don't know of an existing use of it and I figured it would be better
> not to include code that wouldn't be used and tested. However if you feel
> like it should be added let me know.
> 
> (Also, apologies for any mistakes as this is my first Linux patch!)
> 
> Richard Fung (1):
>   fuse: Add initial support for fs-verity
> 
>  fs/fuse/ioctl.c | 52 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 52 insertions(+)

Just a process note: single patches should not use a cover letter.  The
information should be in the patch instead, preferably in the actual commit
message but it's also possible to put text below the scissors line of the patch.

- Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/1] fuse: Add initial support for fs-verity
  2024-04-09 23:50     ` Eric Biggers
@ 2024-04-11  6:06       ` Miklos Szeredi
  2024-04-11 19:15         ` Richard Fung
  0 siblings, 1 reply; 14+ messages in thread
From: Miklos Szeredi @ 2024-04-11  6:06 UTC (permalink / raw)
  To: Eric Biggers; +Cc: Richard Fung, linux-fsdevel, fsverity

On Wed, 10 Apr 2024 at 01:50, Eric Biggers <ebiggers@kernel.org> wrote:

> 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.

Ideally I'd imagine something something similar to how we handle
FS_IOC_GETFLAGS/SETFLAGS.

Exceptions for those were also added in commit 31070f6ccec0 ("fuse:
Fix parameter for FS_IOC_{GET,SET}FLAGS").  But then infrastructure
was added to the vfs (commit 4c5b47997521 ("vfs: add fileattr ops"))
so that filesystems can handle these as normal callbacks instead of
dealing with ioctls directly.

In the fsverity case this is not such a clear cut case, since only
fuse (and possible network fs?) would actually implement the vfs
callback, others would just set the default handler from fsverity.  So
I don't insist on doing this, just saying that it would be the
cleanest outcome.

If we do add exceptions, the requirement from me is that it's split
out into a separate function from fuse_do_ioctl().

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/1] fuse: Add initial support for fs-verity
  2024-04-11  6:06       ` Miklos Szeredi
@ 2024-04-11 19:15         ` Richard Fung
  2024-04-12  8:25           ` Miklos Szeredi
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Fung @ 2024-04-11 19:15 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: Eric Biggers, linux-fsdevel, fsverity

On Wed, Apr 10, 2024 at 11:06 PM Miklos Szeredi <miklos@szeredi.hu> wrote:
> Ideally I'd imagine something something similar to how we handle
> FS_IOC_GETFLAGS/SETFLAGS.
>
> Exceptions for those were also added in commit 31070f6ccec0 ("fuse:
> Fix parameter for FS_IOC_{GET,SET}FLAGS").  But then infrastructure
> was added to the vfs (commit 4c5b47997521 ("vfs: add fileattr ops"))
> so that filesystems can handle these as normal callbacks instead of
> dealing with ioctls directly.
>
> In the fsverity case this is not such a clear cut case, since only
> fuse (and possible network fs?) would actually implement the vfs
> callback, others would just set the default handler from fsverity.  So
> I don't insist on doing this, just saying that it would be the
> cleanest outcome.
>
> If we do add exceptions, the requirement from me is that it's split
> out into a separate function from fuse_do_ioctl().
>
> Thanks,
> Miklos

Thank you all for the feedback and suggestions!

Would allowing FUSE_IOCTL_RETRY for these specific ioctls be
possible/preferable? From my limited understanding retrying is
designed to handle dynamically sized data. However it seems like
that's currently only allowed for CUSE.

If that's not a good idea then I'll try to split it into a separate
function if you don't feel strongly about the other approach.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH 1/1] fuse: Add initial support for fs-verity
  2024-04-11 19:15         ` Richard Fung
@ 2024-04-12  8:25           ` Miklos Szeredi
  0 siblings, 0 replies; 14+ messages in thread
From: Miklos Szeredi @ 2024-04-12  8:25 UTC (permalink / raw)
  To: Richard Fung; +Cc: Eric Biggers, linux-fsdevel, fsverity

On Thu, 11 Apr 2024 at 21:16, Richard Fung <richardfung@google.com> wrote:

> Would allowing FUSE_IOCTL_RETRY for these specific ioctls be
> possible/preferable? From my limited understanding retrying is
> designed to handle dynamically sized data. However it seems like
> that's currently only allowed for CUSE.
>
> If that's not a good idea then I'll try to split it into a separate
> function if you don't feel strongly about the other approach.

It's not a good idea, because it gives complete freedom to the fuse
server about where to gather data from, and that's just an invitation
to malicious behavior.

So yes, please split the current one off to a separate function and
let's see how that looks.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 14+ messages in thread

* [PATCH v2] fuse: Add initial support for fs-verity
  2024-03-28 20:58 [PATCH 0/1] fuse: Add initial support for fs-verity Richard Fung
  2024-03-28 20:58 ` [PATCH 1/1] " Richard Fung
  2024-04-09 23:52 ` [PATCH 0/1] " Eric Biggers
@ 2024-04-16  0:16 ` Richard Fung
  2024-04-19 17:05   ` Eric Biggers
  2 siblings, 1 reply; 14+ messages in thread
From: Richard Fung @ 2024-04-16  0:16 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: linux-fsdevel, ebiggers, fsverity, ynaffit, Richard Fung

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 | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 64 insertions(+)

diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
index 726640fa439e..01638784972a 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)
@@ -118,6 +119,63 @@ static int fuse_copy_ioctl_iovec(struct fuse_conn *fc, struct iovec *dst,
 }
 
 
+/* For fs-verity, determine iov lengths from input */
+static long fuse_setup_verity_ioctl(unsigned int cmd, unsigned long arg,
+				    struct iovec *iov, unsigned int *in_iovs)
+{
+	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;
+	}
+	return 0;
+}
+
+
 /*
  * For ioctls, there is no generic way to determine how much memory
  * needs to be read and/or written.  Furthermore, ioctls are allowed
@@ -227,6 +285,12 @@ long fuse_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg,
 			out_iov = iov;
 			out_iovs = 1;
 		}
+
+		if (cmd == FS_IOC_MEASURE_VERITY || cmd == FS_IOC_ENABLE_VERITY) {
+			err = fuse_setup_verity_ioctl(cmd, arg, iov, &in_iovs);
+			if (err)
+				goto out;
+		}
 	}
 
  retry:
-- 
2.44.0.683.g7961c838ac-goog


^ permalink raw reply related	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] fuse: Add initial support for fs-verity
  2024-04-16  0:16 ` [PATCH v2] " Richard Fung
@ 2024-04-19 17:05   ` Eric Biggers
  2024-04-22 16:31     ` Richard Fung
  0 siblings, 1 reply; 14+ messages in thread
From: Eric Biggers @ 2024-04-19 17:05 UTC (permalink / raw)
  To: Richard Fung; +Cc: Miklos Szeredi, linux-fsdevel, fsverity, ynaffit

Hi,

On Tue, Apr 16, 2024 at 12:16:39AM +0000, Richard Fung 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 | 64 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 64 insertions(+)
> 
> diff --git a/fs/fuse/ioctl.c b/fs/fuse/ioctl.c
> index 726640fa439e..01638784972a 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)
> @@ -118,6 +119,63 @@ static int fuse_copy_ioctl_iovec(struct fuse_conn *fc, struct iovec *dst,
>  }
>  
>  
> +/* For fs-verity, determine iov lengths from input */
> +static long fuse_setup_verity_ioctl(unsigned int cmd, unsigned long arg,
> +				    struct iovec *iov, unsigned int *in_iovs)
> +{
> +	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;
> +	}
> +	return 0;
> +}
> +
> +
>  /*
>   * For ioctls, there is no generic way to determine how much memory
>   * needs to be read and/or written.  Furthermore, ioctls are allowed
> @@ -227,6 +285,12 @@ long fuse_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg,
>  			out_iov = iov;
>  			out_iovs = 1;
>  		}
> +
> +		if (cmd == FS_IOC_MEASURE_VERITY || cmd == FS_IOC_ENABLE_VERITY) {
> +			err = fuse_setup_verity_ioctl(cmd, arg, iov, &in_iovs);
> +			if (err)
> +				goto out;
> +		}

This looks like it passes on the correct buffers for these two ioctls, so if the
FUSE developers agree that this works and is secure, consider this acked:

Acked-by: Eric Biggers <ebiggers@google.com>

It's a bit awkward that the ioctl number is checked twice, though.  Maybe rename
the new function to fuse_setup_special_ioctl() and call it unconditionally?

- Eric

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] fuse: Add initial support for fs-verity
  2024-04-19 17:05   ` Eric Biggers
@ 2024-04-22 16:31     ` Richard Fung
  2024-04-23  9:31       ` Miklos Szeredi
  0 siblings, 1 reply; 14+ messages in thread
From: Richard Fung @ 2024-04-22 16:31 UTC (permalink / raw)
  To: Eric Biggers; +Cc: Miklos Szeredi, linux-fsdevel, fsverity, ynaffit

On Fri, Apr 19, 2024 at 10:05 AM Eric Biggers <ebiggers@kernel.org> wrote:
> It's a bit awkward that the ioctl number is checked twice, though.  Maybe rename
> the new function to fuse_setup_special_ioctl() and call it unconditionally?
>
> - Eric

I'm happy to change it but the benefit of the current iteration is
that it's much more obvious to someone unfamiliar with the code that
code path is only used for fs-verity. Admittedly, I may be
overindexing on the importance of that as someone who's trying to
learn the codebase myself.

Let me know if you still prefer I change it

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] fuse: Add initial support for fs-verity
  2024-04-22 16:31     ` Richard Fung
@ 2024-04-23  9:31       ` Miklos Szeredi
  2024-04-23 18:41         ` Richard Fung
  0 siblings, 1 reply; 14+ messages in thread
From: Miklos Szeredi @ 2024-04-23  9:31 UTC (permalink / raw)
  To: Richard Fung; +Cc: Eric Biggers, linux-fsdevel, fsverity, ynaffit

On Mon, 22 Apr 2024 at 18:31, Richard Fung <richardfung@google.com> wrote:
>
> On Fri, Apr 19, 2024 at 10:05 AM Eric Biggers <ebiggers@kernel.org> wrote:
> > It's a bit awkward that the ioctl number is checked twice, though.  Maybe rename
> > the new function to fuse_setup_special_ioctl() and call it unconditionally?
> >
> > - Eric
>
> I'm happy to change it but the benefit of the current iteration is
> that it's much more obvious to someone unfamiliar with the code that
> code path is only used for fs-verity. Admittedly, I may be
> overindexing on the importance of that as someone who's trying to
> learn the codebase myself.
>
> Let me know if you still prefer I change it

Or move the switch out to the caller and split
fuse_setup_verity_ioctl() into two separate functions, since there's
no commonality between them (except that both are verity related).

I did that transformation and pushed it to

  git://git.kernel.org/pub/scm/linux/kernel/git/mszeredi/fuse.git#for-next

Please verify that I didn't mess anything up.

Thanks,
Miklos

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [PATCH v2] fuse: Add initial support for fs-verity
  2024-04-23  9:31       ` Miklos Szeredi
@ 2024-04-23 18:41         ` Richard Fung
  0 siblings, 0 replies; 14+ messages in thread
From: Richard Fung @ 2024-04-23 18:41 UTC (permalink / raw)
  To: Miklos Szeredi; +Cc: Eric Biggers, linux-fsdevel, fsverity, ynaffit

> Please verify that I didn't mess anything up.

Tested it and it works! Thanks for updating the patch

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, other threads:[~2024-04-23 18:41 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-28 20:58 [PATCH 0/1] fuse: Add initial support for fs-verity Richard Fung
2024-03-28 20:58 ` [PATCH 1/1] " Richard Fung
2024-04-02 16:16   ` Richard Fung
2024-04-09 14:50   ` Miklos Szeredi
2024-04-09 23:50     ` Eric Biggers
2024-04-11  6:06       ` Miklos Szeredi
2024-04-11 19:15         ` Richard Fung
2024-04-12  8:25           ` Miklos Szeredi
2024-04-09 23:52 ` [PATCH 0/1] " Eric Biggers
2024-04-16  0:16 ` [PATCH v2] " Richard Fung
2024-04-19 17:05   ` Eric Biggers
2024-04-22 16:31     ` Richard Fung
2024-04-23  9:31       ` Miklos Szeredi
2024-04-23 18:41         ` Richard Fung

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).