Re: [PATCH] fuse: Support FS_IOC_GET_ENCRYPTION_POLICY_EX

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Please Cc linux-fscrypt@xxxxxxxxxxxxxxx on all fscrypt-related patches.

On Mon, Dec 07, 2020 at 01:03:03PM +0900, Chirantan Ekbote wrote:
> This is a dynamically sized ioctl so we need to check the user-provided
> parameter for the actual length.
> 
> Signed-off-by: Chirantan Ekbote <chirantan@xxxxxxxxxxxx>

Could you add something here about why this ioctl in particular needs to be
passed through FUSE?  This isn't the only dynamically-sized ioctl.

> @@ -2808,6 +2809,21 @@ long fuse_do_ioctl(struct file *file, unsigned int cmd, unsigned long arg,
>  		case FS_IOC_SETFLAGS:
>  			iov->iov_len = sizeof(int);
>  			break;
> +		case FS_IOC_GET_ENCRYPTION_POLICY_EX: {

This is in the middle of a 200 lines function.  It would be easier to understand
if you refactored this to use a helper function that that takes in the ioctl
number and user buffer and returns the size.

> +			struct fscrypt_get_policy_ex_arg policy;

'__u64 policy_size' would be sufficient, since only that part of the struct is
used.

> +			unsigned long size_ptr =
> +				arg + offsetof(struct fscrypt_get_policy_ex_arg,
> +					       policy_size);

Doing pointer arithmetic on unsigned long is unusual.  It would be easier to
understand if you did:

	struct fscrypt_get_policy_ex_arg __user *uarg =
		(struct fscrypt_get_policy_ex_arg __user *)arg;

Then pass &uarg->policy_size to copy_from_user().

> +
> +			if (copy_from_user(&policy.policy_size,
> +					   (void __user *)size_ptr,
> +					   sizeof(policy.policy_size)))
> +				return -EFAULT;
> +
> +			iov->iov_len =
> +				sizeof(policy.policy_size) + policy.policy_size;
> +			break;

This may overflow SIZE_MAX, as policy_size is a __u64 directly from userspace.
Wouldn't FUSE need to limit the size to a smaller value?

- Eric



[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux