diff mbox series

[v2,03/20] fscrypt: add fscrypt_have_same_policy() to check inode compatibility

Message ID 59fe9cb2916e7bad3bc57abc211a35f0a85990bc.1662420176.git.sweettea-kernel@dorminy.me (mailing list archive)
State Superseded
Headers show
Series btrfs: add fscrypt integration | expand

Commit Message

Sweet Tea Dorminy Sept. 6, 2022, 12:35 a.m. UTC
From: Omar Sandoval <osandov@osandov.com>

Btrfs will need to check whether inode policies are identical for
various purposes: if two inodes want to share an extent, they must have
the same policy, including key identifier; symlinks must not span the
encrypted/unencrypted border; and certain encryption policies will allow
btrfs to store one fscrypt_context for multiple objects. Therefore, add
a function which allows checking the encryption policies of two inodes
to ensure they are identical.

Signed-off-by: Omar Sandoval <osandov@osandov.com>
Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
---
 fs/crypto/policy.c      | 26 ++++++++++++++++++++++++++
 include/linux/fscrypt.h |  1 +
 2 files changed, 27 insertions(+)

Comments

Josef Bacik Sept. 8, 2022, 1:53 p.m. UTC | #1
On Mon, Sep 05, 2022 at 08:35:18PM -0400, Sweet Tea Dorminy wrote:
> From: Omar Sandoval <osandov@osandov.com>
> 
> Btrfs will need to check whether inode policies are identical for
> various purposes: if two inodes want to share an extent, they must have
> the same policy, including key identifier; symlinks must not span the
> encrypted/unencrypted border; and certain encryption policies will allow
> btrfs to store one fscrypt_context for multiple objects. Therefore, add
> a function which allows checking the encryption policies of two inodes
> to ensure they are identical.
> 
> Signed-off-by: Omar Sandoval <osandov@osandov.com>
> Signed-off-by: Sweet Tea Dorminy <sweettea-kernel@dorminy.me>
> ---
>  fs/crypto/policy.c      | 26 ++++++++++++++++++++++++++
>  include/linux/fscrypt.h |  1 +
>  2 files changed, 27 insertions(+)
> 
> diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
> index 80b8ca0f340b..ed8b7b6531e5 100644
> --- a/fs/crypto/policy.c
> +++ b/fs/crypto/policy.c
> @@ -415,6 +415,32 @@ static int fscrypt_get_policy(struct inode *inode, union fscrypt_policy *policy)
>  	return fscrypt_policy_from_context(policy, &ctx, ret);
>  }
>  
> +/**
> + * fscrypt_have_same_policy() - check whether two inodes have the same policy
> + * @inode1: the first inode
> + * @inode2: the second inode
> + *
> + * Return: %true if equal, else %false
> + */
> +int fscrypt_have_same_policy(struct inode *inode1, struct inode *inode2)
> +{
> +	union fscrypt_policy policy1, policy2;
> +	int err;
> +
> +	if (!IS_ENCRYPTED(inode1) && !IS_ENCRYPTED(inode2))
> +		return true;
> +	else if (!IS_ENCRYPTED(inode1) || !IS_ENCRYPTED(inode2))
> +		return false;
> +	err = fscrypt_get_policy(inode1, &policy1);
> +	if (err)
> +		return err;
> +	err = fscrypt_get_policy(inode2, &policy2);
> +	if (err)
> +		return err;

These things can return random errors, so you're mixing bool with errnos, and
then you're using this function as if it only returns bools.  I'm not sure what
the best thing to do is here for consistency, maybe return 0 for no match, 1 for
match, and then err and handle it that way.  At the very least the callers need
to be updated to handle the errors.  But I really don't like the mixing of bool
returns with ERRNO returns.  An alternative would be to have a helper do the
getting of the policies which will give you the errno's appropriately, and then
do the call to fscrypt_policies_equal with the policies you grab.  Thanks,

Josef
diff mbox series

Patch

diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index 80b8ca0f340b..ed8b7b6531e5 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -415,6 +415,32 @@  static int fscrypt_get_policy(struct inode *inode, union fscrypt_policy *policy)
 	return fscrypt_policy_from_context(policy, &ctx, ret);
 }
 
+/**
+ * fscrypt_have_same_policy() - check whether two inodes have the same policy
+ * @inode1: the first inode
+ * @inode2: the second inode
+ *
+ * Return: %true if equal, else %false
+ */
+int fscrypt_have_same_policy(struct inode *inode1, struct inode *inode2)
+{
+	union fscrypt_policy policy1, policy2;
+	int err;
+
+	if (!IS_ENCRYPTED(inode1) && !IS_ENCRYPTED(inode2))
+		return true;
+	else if (!IS_ENCRYPTED(inode1) || !IS_ENCRYPTED(inode2))
+		return false;
+	err = fscrypt_get_policy(inode1, &policy1);
+	if (err)
+		return err;
+	err = fscrypt_get_policy(inode2, &policy2);
+	if (err)
+		return err;
+	return fscrypt_policies_equal(&policy1, &policy2);
+}
+EXPORT_SYMBOL(fscrypt_have_same_policy);
+
 static int set_encryption_policy(struct inode *inode,
 				 const union fscrypt_policy *policy)
 {
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index a4e00314c91b..16d2252d5562 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -318,6 +318,7 @@  static inline struct page *fscrypt_pagecache_page(struct page *bounce_page)
 void fscrypt_free_bounce_page(struct page *bounce_page);
 
 /* policy.c */
+int fscrypt_have_same_policy(struct inode *inode1, struct inode *inode2);
 int fscrypt_ioctl_set_policy(struct file *filp, const void __user *arg);
 int fscrypt_ioctl_get_policy(struct file *filp, void __user *arg);
 int fscrypt_ioctl_get_policy_ex(struct file *filp, void __user *arg);