diff mbox series

[v2] fuse: Add initial support for fs-verity

Message ID 20240416001639.359059-1-richardfung@google.com (mailing list archive)
State Accepted
Headers show
Series [v2] fuse: Add initial support for fs-verity | expand

Commit Message

Richard Fung April 16, 2024, 12:16 a.m. UTC
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(+)

Comments

Eric Biggers April 19, 2024, 5:05 p.m. UTC | #1
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
Richard Fung April 22, 2024, 4:31 p.m. UTC | #2
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
Miklos Szeredi April 23, 2024, 9:31 a.m. UTC | #3
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
Richard Fung April 23, 2024, 6:41 p.m. UTC | #4
> Please verify that I didn't mess anything up.

Tested it and it works! Thanks for updating the patch
diff mbox series

Patch

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: