[PATCH 0/9] Allow deleting files with unsupported encryption policy

[PATCH 0/9] Allow deleting files with unsupported encryption policy

[Eric Biggers]

Currently it's impossible to delete files that use an unsupported
encryption policy, as the kernel will just return an error when
performing any operation on the top-level encrypted directory, even just
a path lookup into the directory or opening the directory for readdir.

It's desirable to return errors for most operations on files that use an
unsupported encryption policy, but the current behavior is too strict.
We need to allow enough to delete files, so that people can't be stuck
with undeletable files when downgrading kernel versions.  That includes
allowing directories to be listed and allowing dentries to be looked up.

This series fixes this (on ext4, f2fs, and ubifs) by treating an
unsupported encryption policy in the same way as "key unavailable" in
the cases that are required for a recursive delete to work.

The actual fix is in patch 9, so see that for more details.

Patches 1-8 are cleanups that prepare for the actual fix by removing
direct use of fscrypt_get_encryption_info() by filesystems.

This patchset applies to branch "master" (commit 4a4b8721f1a5) of
https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git.

Eric Biggers (9):
  ext4: remove ext4_dir_open()
  f2fs: remove f2fs_dir_open()
  ubifs: remove ubifs_dir_open()
  ext4: don't call fscrypt_get_encryption_info() from dx_show_leaf()
  fscrypt: introduce fscrypt_prepare_readdir()
  fscrypt: move body of fscrypt_prepare_setattr() out-of-line
  fscrypt: move fscrypt_require_key() to fscrypt_private.h
  fscrypt: unexport fscrypt_get_encryption_info()
  fscrypt: allow deleting files with unsupported encryption policy

 fs/crypto/fname.c           |  8 +++-
 fs/crypto/fscrypt_private.h | 28 ++++++++++++++
 fs/crypto/hooks.c           | 16 +++++++-
 fs/crypto/keysetup.c        | 20 ++++++++--
 fs/crypto/policy.c          | 22 +++++++----
 fs/ext4/dir.c               | 16 ++------
 fs/ext4/namei.c             | 10 +----
 fs/f2fs/dir.c               | 10 +----
 fs/ubifs/dir.c              | 11 +-----
 include/linux/fscrypt.h     | 75 +++++++++++++++++++------------------
 10 files changed, 126 insertions(+), 90 deletions(-)


base-commit: 4a4b8721f1a5e4b01e45b3153c68d5a1014b25de
-- 
2.29.2

[PATCH 1/9] ext4: remove ext4_dir_open()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Since encrypted directories can be opened without their encryption key
being available, and each readdir tries to set up the key, trying to set
up the key in ->open() too isn't really useful.

Just remove it so that directories don't need an ->open() method
anymore, and so that we eliminate a use of fscrypt_get_encryption_info()
(which I'd like to stop exporting to filesystems).

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/dir.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index ca50c90adc4c..16bfbdd5007c 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -616,13 +616,6 @@ static int ext4_dx_readdir(struct file *file, struct dir_context *ctx)
 	return 0;
 }
 
-static int ext4_dir_open(struct inode * inode, struct file * filp)
-{
-	if (IS_ENCRYPTED(inode))
-		return fscrypt_get_encryption_info(inode) ? -EACCES : 0;
-	return 0;
-}
-
 static int ext4_release_dir(struct inode *inode, struct file *filp)
 {
 	if (filp->private_data)
@@ -664,7 +657,6 @@ const struct file_operations ext4_dir_operations = {
 	.compat_ioctl	= ext4_compat_ioctl,
 #endif
 	.fsync		= ext4_sync_file,
-	.open		= ext4_dir_open,
 	.release	= ext4_release_dir,
 };
 
-- 
2.29.2

Re: [PATCH 1/9] ext4: remove ext4_dir_open()

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1543 bytes --]

On Nov 24, 2020, at 5:23 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> 
> From: Eric Biggers <ebiggers@google.com>
> 
> Since encrypted directories can be opened without their encryption key
> being available, and each readdir tries to set up the key, trying to set
> up the key in ->open() too isn't really useful.
> 
> Just remove it so that directories don't need an ->open() method
> anymore, and so that we eliminate a use of fscrypt_get_encryption_info()
> (which I'd like to stop exporting to filesystems).
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/ext4/dir.c | 8 --------
> 1 file changed, 8 deletions(-)
> 
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index ca50c90adc4c..16bfbdd5007c 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -616,13 +616,6 @@ static int ext4_dx_readdir(struct file *file, struct dir_context *ctx)
> 	return 0;
> }
> 
> -static int ext4_dir_open(struct inode * inode, struct file * filp)
> -{
> -	if (IS_ENCRYPTED(inode))
> -		return fscrypt_get_encryption_info(inode) ? -EACCES : 0;
> -	return 0;
> -}
> -
> static int ext4_release_dir(struct inode *inode, struct file *filp)
> {
> 	if (filp->private_data)
> @@ -664,7 +657,6 @@ const struct file_operations ext4_dir_operations = {
> 	.compat_ioctl	= ext4_compat_ioctl,
> #endif
> 	.fsync		= ext4_sync_file,
> -	.open		= ext4_dir_open,
> 	.release	= ext4_release_dir,
> };
> 
> --
> 2.29.2
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

[PATCH 2/9] f2fs: remove f2fs_dir_open()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Since encrypted directories can be opened without their encryption key
being available, and each readdir tries to set up the key, trying to set
up the key in ->open() too isn't really useful.

Just remove it so that directories don't need an ->open() method
anymore, and so that we eliminate a use of fscrypt_get_encryption_info()
(which I'd like to stop exporting to filesystems).

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/f2fs/dir.c | 8 --------
 1 file changed, 8 deletions(-)

diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 4b9ef8bbfa4a..47bee953fc8d 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -1081,19 +1081,11 @@ static int f2fs_readdir(struct file *file, struct dir_context *ctx)
 	return err < 0 ? err : 0;
 }
 
-static int f2fs_dir_open(struct inode *inode, struct file *filp)
-{
-	if (IS_ENCRYPTED(inode))
-		return fscrypt_get_encryption_info(inode) ? -EACCES : 0;
-	return 0;
-}
-
 const struct file_operations f2fs_dir_operations = {
 	.llseek		= generic_file_llseek,
 	.read		= generic_read_dir,
 	.iterate_shared	= f2fs_readdir,
 	.fsync		= f2fs_sync_file,
-	.open		= f2fs_dir_open,
 	.unlocked_ioctl	= f2fs_ioctl,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl   = f2fs_compat_ioctl,
-- 
2.29.2

Re: [PATCH 2/9] f2fs: remove f2fs_dir_open()

[Chao Yu]

On 2020/11/25 8:23, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Since encrypted directories can be opened without their encryption key
> being available, and each readdir tries to set up the key, trying to set

readdir -> readdir/lookup?

> up the key in ->open() too isn't really useful.
> 
> Just remove it so that directories don't need an ->open() method
> anymore, and so that we eliminate a use of fscrypt_get_encryption_info()
> (which I'd like to stop exporting to filesystems).
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Chao Yu <yuchao0@huawei.com>

Thanks,

Re: [PATCH 2/9] f2fs: remove f2fs_dir_open()

[Eric Biggers]

On Thu, Nov 26, 2020 at 03:04:55PM +0800, Chao Yu wrote:
> On 2020/11/25 8:23, Eric Biggers wrote:
> > From: Eric Biggers <ebiggers@google.com>
> > 
> > Since encrypted directories can be opened without their encryption key
> > being available, and each readdir tries to set up the key, trying to set
> 
> readdir -> readdir/lookup?

Yes, ->lookup() tries to set up the key too.  It's different because ->lookup()
doesn't require that the directory be open.  But I suppose that's another reason
why setting up the directory's key in ->open() isn't useful.

I'll add something about that.

- Eric

> 
> > up the key in ->open() too isn't really useful.
> > 
> > Just remove it so that directories don't need an ->open() method
> > anymore, and so that we eliminate a use of fscrypt_get_encryption_info()
> > (which I'd like to stop exporting to filesystems).
> > 
> > Signed-off-by: Eric Biggers <ebiggers@google.com>
> 
> Reviewed-by: Chao Yu <yuchao0@huawei.com>
> 
> Thanks,

[PATCH 3/9] ubifs: remove ubifs_dir_open()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Since encrypted directories can be opened without their encryption key
being available, and each readdir tries to set up the key, trying to set
up the key in ->open() too isn't really useful.

Just remove it so that directories don't need an ->open() method
anymore, and so that we eliminate a use of fscrypt_get_encryption_info()
(which I'd like to stop exporting to filesystems).

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ubifs/dir.c | 9 ---------
 1 file changed, 9 deletions(-)

diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 08fde777c324..009fbf844d3e 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -1619,14 +1619,6 @@ int ubifs_getattr(const struct path *path, struct kstat *stat,
 	return 0;
 }
 
-static int ubifs_dir_open(struct inode *dir, struct file *file)
-{
-	if (IS_ENCRYPTED(dir))
-		return fscrypt_get_encryption_info(dir) ? -EACCES : 0;
-
-	return 0;
-}
-
 const struct inode_operations ubifs_dir_inode_operations = {
 	.lookup      = ubifs_lookup,
 	.create      = ubifs_create,
@@ -1653,7 +1645,6 @@ const struct file_operations ubifs_dir_operations = {
 	.iterate_shared = ubifs_readdir,
 	.fsync          = ubifs_fsync,
 	.unlocked_ioctl = ubifs_ioctl,
-	.open		= ubifs_dir_open,
 #ifdef CONFIG_COMPAT
 	.compat_ioctl   = ubifs_compat_ioctl,
 #endif
-- 
2.29.2

[PATCH 4/9] ext4: don't call fscrypt_get_encryption_info() from dx_show_leaf()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

The call to fscrypt_get_encryption_info() in dx_show_leaf() is too low
in the call tree; fscrypt_get_encryption_info() should have already been
called when starting the directory operation.  And indeed, it already
is.  Moreover, the encryption key is guaranteed to already be available
because dx_show_leaf() is only called when adding a new directory entry.

And even if the key wasn't available, dx_show_leaf() uses
fscrypt_fname_disk_to_usr() which knows how to create a no-key name.

So for the above reasons, and because it would be desirable to stop
exporting fscrypt_get_encryption_info() directly to filesystems, remove
the call to fscrypt_get_encryption_info() from dx_show_leaf().

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/ext4/namei.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 793fc7db9d28..7b31aea3e025 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -643,13 +643,7 @@ static struct stats dx_show_leaf(struct inode *dir,
 
 				name  = de->name;
 				len = de->name_len;
-				if (IS_ENCRYPTED(dir))
-					res = fscrypt_get_encryption_info(dir);
-				if (res) {
-					printk(KERN_WARNING "Error setting up"
-					       " fname crypto: %d\n", res);
-				}
-				if (!fscrypt_has_encryption_key(dir)) {
+				if (!IS_ENCRYPTED(dir)) {
 					/* Directory is not encrypted */
 					ext4fs_dirhash(dir, de->name,
 						de->name_len, &h);
-- 
2.29.2

Re: [PATCH 4/9] ext4: don't call fscrypt_get_encryption_info() from dx_show_leaf()

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]

On Nov 24, 2020, at 5:23 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> 
> From: Eric Biggers <ebiggers@google.com>
> 
> The call to fscrypt_get_encryption_info() in dx_show_leaf() is too low
> in the call tree; fscrypt_get_encryption_info() should have already been
> called when starting the directory operation.  And indeed, it already
> is.  Moreover, the encryption key is guaranteed to already be available
> because dx_show_leaf() is only called when adding a new directory entry.
> 
> And even if the key wasn't available, dx_show_leaf() uses
> fscrypt_fname_disk_to_usr() which knows how to create a no-key name.
> 
> So for the above reasons, and because it would be desirable to stop
> exporting fscrypt_get_encryption_info() directly to filesystems, remove
> the call to fscrypt_get_encryption_info() from dx_show_leaf().
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/ext4/namei.c | 8 +-------
> 1 file changed, 1 insertion(+), 7 deletions(-)
> 
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 793fc7db9d28..7b31aea3e025 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -643,13 +643,7 @@ static struct stats dx_show_leaf(struct inode *dir,
> 
> 				name  = de->name;
> 				len = de->name_len;
> -				if (IS_ENCRYPTED(dir))
> -					res = fscrypt_get_encryption_info(dir);
> -				if (res) {
> -					printk(KERN_WARNING "Error setting up"
> -					       " fname crypto: %d\n", res);
> -				}
> -				if (!fscrypt_has_encryption_key(dir)) {
> +				if (!IS_ENCRYPTED(dir)) {
> 					/* Directory is not encrypted */
> 					ext4fs_dirhash(dir, de->name,
> 						de->name_len, &h);
> --
> 2.29.2
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

[PATCH 5/9] fscrypt: introduce fscrypt_prepare_readdir()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

The last remaining use of fscrypt_get_encryption_info() from filesystems
is for readdir (->iterate_shared()).  Every other call is now in
fs/crypto/ as part of some other higher-level operation.

We need to add a new argument to fscrypt_get_encryption_info() to
indicate whether the encryption policy to allowed to be unrecognized or
not.  Doing this is easier if we can work with high-level operations
rather than direct filesystem use of fscrypt_get_encryption_info().

So add a function fscrypt_prepare_readdir() which wraps the call to
fscrypt_get_encryption_info() for the readdir use case.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/hooks.c       |  6 ++++++
 fs/ext4/dir.c           |  8 +++-----
 fs/ext4/namei.c         |  2 +-
 fs/f2fs/dir.c           |  2 +-
 fs/ubifs/dir.c          |  2 +-
 include/linux/fscrypt.h | 24 ++++++++++++++++++++++++
 6 files changed, 36 insertions(+), 8 deletions(-)

diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index c809a4afa057..82f351d3113a 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -114,6 +114,12 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
 }
 EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup);
 
+int __fscrypt_prepare_readdir(struct inode *dir)
+{
+	return fscrypt_get_encryption_info(dir);
+}
+EXPORT_SYMBOL_GPL(__fscrypt_prepare_readdir);
+
 /**
  * fscrypt_prepare_setflags() - prepare to change flags with FS_IOC_SETFLAGS
  * @inode: the inode on which flags are being changed
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
index 16bfbdd5007c..c6d16353326a 100644
--- a/fs/ext4/dir.c
+++ b/fs/ext4/dir.c
@@ -118,11 +118,9 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
 	struct buffer_head *bh = NULL;
 	struct fscrypt_str fstr = FSTR_INIT(NULL, 0);
 
-	if (IS_ENCRYPTED(inode)) {
-		err = fscrypt_get_encryption_info(inode);
-		if (err)
-			return err;
-	}
+	err = fscrypt_prepare_readdir(inode);
+	if (err)
+		return err;
 
 	if (is_dx_dir(inode)) {
 		err = ext4_dx_readdir(file, ctx);
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
index 7b31aea3e025..5fa8436cd5fa 100644
--- a/fs/ext4/namei.c
+++ b/fs/ext4/namei.c
@@ -1004,7 +1004,7 @@ static int htree_dirblock_to_tree(struct file *dir_file,
 					   EXT4_DIR_REC_LEN(0));
 	/* Check if the directory is encrypted */
 	if (IS_ENCRYPTED(dir)) {
-		err = fscrypt_get_encryption_info(dir);
+		err = fscrypt_prepare_readdir(dir);
 		if (err < 0) {
 			brelse(bh);
 			return err;
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
index 47bee953fc8d..049500f1e764 100644
--- a/fs/f2fs/dir.c
+++ b/fs/f2fs/dir.c
@@ -1022,7 +1022,7 @@ static int f2fs_readdir(struct file *file, struct dir_context *ctx)
 	int err = 0;
 
 	if (IS_ENCRYPTED(inode)) {
-		err = fscrypt_get_encryption_info(inode);
+		err = fscrypt_prepare_readdir(inode);
 		if (err)
 			goto out;
 
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 009fbf844d3e..1f33a5598b93 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -514,7 +514,7 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
 		return 0;
 
 	if (encrypted) {
-		err = fscrypt_get_encryption_info(dir);
+		err = fscrypt_prepare_readdir(dir);
 		if (err)
 			return err;
 
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 0c9e64969b73..8cbb26f55695 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -242,6 +242,7 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry,
 			     unsigned int flags);
 int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
 			     struct fscrypt_name *fname);
+int __fscrypt_prepare_readdir(struct inode *dir);
 int fscrypt_prepare_setflags(struct inode *inode,
 			     unsigned int oldflags, unsigned int flags);
 int fscrypt_prepare_symlink(struct inode *dir, const char *target,
@@ -537,6 +538,11 @@ static inline int __fscrypt_prepare_lookup(struct inode *dir,
 	return -EOPNOTSUPP;
 }
 
+static inline int __fscrypt_prepare_readdir(struct inode *dir)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int fscrypt_prepare_setflags(struct inode *inode,
 					   unsigned int oldflags,
 					   unsigned int flags)
@@ -795,6 +801,24 @@ static inline int fscrypt_prepare_lookup(struct inode *dir,
 	return 0;
 }
 
+/**
+ * fscrypt_prepare_readdir() - prepare to read a possibly-encrypted directory
+ * @dir: the directory inode
+ *
+ * If the directory is encrypted and it doesn't already have its encryption key
+ * set up, try to set it up so that the filenames will be listed in plaintext
+ * form rather than in no-key form.
+ *
+ * Return: 0 on success; -errno on error.  Note that the encryption key being
+ *	   unavailable is not considered an error.
+ */
+static inline int fscrypt_prepare_readdir(struct inode *dir)
+{
+	if (IS_ENCRYPTED(dir))
+		return __fscrypt_prepare_readdir(dir);
+	return 0;
+}
+
 /**
  * fscrypt_prepare_setattr() - prepare to change a possibly-encrypted inode's
  *			       attributes
-- 
2.29.2

Re: [PATCH 5/9] fscrypt: introduce fscrypt_prepare_readdir()

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 5583 bytes --]

On Nov 24, 2020, at 5:23 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> 
> From: Eric Biggers <ebiggers@google.com>
> 
> The last remaining use of fscrypt_get_encryption_info() from filesystems
> is for readdir (->iterate_shared()).  Every other call is now in
> fs/crypto/ as part of some other higher-level operation.
> 
> We need to add a new argument to fscrypt_get_encryption_info() to
> indicate whether the encryption policy to allowed to be unrecognized or
> not.  Doing this is easier if we can work with high-level operations
> rather than direct filesystem use of fscrypt_get_encryption_info().
> 
> So add a function fscrypt_prepare_readdir() which wraps the call to
> fscrypt_get_encryption_info() for the readdir use case.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/crypto/hooks.c       |  6 ++++++
> fs/ext4/dir.c           |  8 +++-----
> fs/ext4/namei.c         |  2 +-
> fs/f2fs/dir.c           |  2 +-
> fs/ubifs/dir.c          |  2 +-
> include/linux/fscrypt.h | 24 ++++++++++++++++++++++++
> 6 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
> index c809a4afa057..82f351d3113a 100644
> --- a/fs/crypto/hooks.c
> +++ b/fs/crypto/hooks.c
> @@ -114,6 +114,12 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
> }
> EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup);
> 
> +int __fscrypt_prepare_readdir(struct inode *dir)
> +{
> +	return fscrypt_get_encryption_info(dir);
> +}
> +EXPORT_SYMBOL_GPL(__fscrypt_prepare_readdir);
> +
> /**
>  * fscrypt_prepare_setflags() - prepare to change flags with FS_IOC_SETFLAGS
>  * @inode: the inode on which flags are being changed
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index 16bfbdd5007c..c6d16353326a 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -118,11 +118,9 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
> 	struct buffer_head *bh = NULL;
> 	struct fscrypt_str fstr = FSTR_INIT(NULL, 0);
> 
> -	if (IS_ENCRYPTED(inode)) {
> -		err = fscrypt_get_encryption_info(inode);
> -		if (err)
> -			return err;
> -	}
> +	err = fscrypt_prepare_readdir(inode);
> +	if (err)
> +		return err;
> 
> 	if (is_dx_dir(inode)) {
> 		err = ext4_dx_readdir(file, ctx);
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 7b31aea3e025..5fa8436cd5fa 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1004,7 +1004,7 @@ static int htree_dirblock_to_tree(struct file *dir_file,
> 					   EXT4_DIR_REC_LEN(0));
> 	/* Check if the directory is encrypted */
> 	if (IS_ENCRYPTED(dir)) {
> -		err = fscrypt_get_encryption_info(dir);
> +		err = fscrypt_prepare_readdir(dir);
> 		if (err < 0) {
> 			brelse(bh);
> 			return err;
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 47bee953fc8d..049500f1e764 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -1022,7 +1022,7 @@ static int f2fs_readdir(struct file *file, struct dir_context *ctx)
> 	int err = 0;
> 
> 	if (IS_ENCRYPTED(inode)) {
> -		err = fscrypt_get_encryption_info(inode);
> +		err = fscrypt_prepare_readdir(inode);
> 		if (err)
> 			goto out;
> 
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 009fbf844d3e..1f33a5598b93 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -514,7 +514,7 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
> 		return 0;
> 
> 	if (encrypted) {
> -		err = fscrypt_get_encryption_info(dir);
> +		err = fscrypt_prepare_readdir(dir);
> 		if (err)
> 			return err;
> 
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index 0c9e64969b73..8cbb26f55695 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -242,6 +242,7 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry,
> 			     unsigned int flags);
> int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
> 			     struct fscrypt_name *fname);
> +int __fscrypt_prepare_readdir(struct inode *dir);
> int fscrypt_prepare_setflags(struct inode *inode,
> 			     unsigned int oldflags, unsigned int flags);
> int fscrypt_prepare_symlink(struct inode *dir, const char *target,
> @@ -537,6 +538,11 @@ static inline int __fscrypt_prepare_lookup(struct inode *dir,
> 	return -EOPNOTSUPP;
> }
> 
> +static inline int __fscrypt_prepare_readdir(struct inode *dir)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> static inline int fscrypt_prepare_setflags(struct inode *inode,
> 					   unsigned int oldflags,
> 					   unsigned int flags)
> @@ -795,6 +801,24 @@ static inline int fscrypt_prepare_lookup(struct inode *dir,
> 	return 0;
> }
> 
> +/**
> + * fscrypt_prepare_readdir() - prepare to read a possibly-encrypted directory
> + * @dir: the directory inode
> + *
> + * If the directory is encrypted and it doesn't already have its encryption key
> + * set up, try to set it up so that the filenames will be listed in plaintext
> + * form rather than in no-key form.
> + *
> + * Return: 0 on success; -errno on error.  Note that the encryption key being
> + *	   unavailable is not considered an error.
> + */
> +static inline int fscrypt_prepare_readdir(struct inode *dir)
> +{
> +	if (IS_ENCRYPTED(dir))
> +		return __fscrypt_prepare_readdir(dir);
> +	return 0;
> +}
> +
> /**
>  * fscrypt_prepare_setattr() - prepare to change a possibly-encrypted inode's
>  *			       attributes
> --
> 2.29.2
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 5/9] fscrypt: introduce fscrypt_prepare_readdir()

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 5559 bytes --]


> On Nov 24, 2020, at 5:23 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> 
> From: Eric Biggers <ebiggers@google.com>
> 
> The last remaining use of fscrypt_get_encryption_info() from filesystems
> is for readdir (->iterate_shared()).  Every other call is now in
> fs/crypto/ as part of some other higher-level operation.
> 
> We need to add a new argument to fscrypt_get_encryption_info() to
> indicate whether the encryption policy to allowed to be unrecognized or

(typo) *is* allowed

> not.  Doing this is easier if we can work with high-level operations
> rather than direct filesystem use of fscrypt_get_encryption_info().
> 
> So add a function fscrypt_prepare_readdir() which wraps the call to
> fscrypt_get_encryption_info() for the readdir use case.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>
> ---
> fs/crypto/hooks.c       |  6 ++++++
> fs/ext4/dir.c           |  8 +++-----
> fs/ext4/namei.c         |  2 +-
> fs/f2fs/dir.c           |  2 +-
> fs/ubifs/dir.c          |  2 +-
> include/linux/fscrypt.h | 24 ++++++++++++++++++++++++
> 6 files changed, 36 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
> index c809a4afa057..82f351d3113a 100644
> --- a/fs/crypto/hooks.c
> +++ b/fs/crypto/hooks.c
> @@ -114,6 +114,12 @@ int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
> }
> EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup);
> 
> +int __fscrypt_prepare_readdir(struct inode *dir)
> +{
> +	return fscrypt_get_encryption_info(dir);
> +}
> +EXPORT_SYMBOL_GPL(__fscrypt_prepare_readdir);
> +
> /**
>  * fscrypt_prepare_setflags() - prepare to change flags with FS_IOC_SETFLAGS
>  * @inode: the inode on which flags are being changed
> diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c
> index 16bfbdd5007c..c6d16353326a 100644
> --- a/fs/ext4/dir.c
> +++ b/fs/ext4/dir.c
> @@ -118,11 +118,9 @@ static int ext4_readdir(struct file *file, struct dir_context *ctx)
> 	struct buffer_head *bh = NULL;
> 	struct fscrypt_str fstr = FSTR_INIT(NULL, 0);
> 
> -	if (IS_ENCRYPTED(inode)) {
> -		err = fscrypt_get_encryption_info(inode);
> -		if (err)
> -			return err;
> -	}
> +	err = fscrypt_prepare_readdir(inode);
> +	if (err)
> +		return err;
> 
> 	if (is_dx_dir(inode)) {
> 		err = ext4_dx_readdir(file, ctx);
> diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c
> index 7b31aea3e025..5fa8436cd5fa 100644
> --- a/fs/ext4/namei.c
> +++ b/fs/ext4/namei.c
> @@ -1004,7 +1004,7 @@ static int htree_dirblock_to_tree(struct file *dir_file,
> 					   EXT4_DIR_REC_LEN(0));
> 	/* Check if the directory is encrypted */
> 	if (IS_ENCRYPTED(dir)) {
> -		err = fscrypt_get_encryption_info(dir);
> +		err = fscrypt_prepare_readdir(dir);
> 		if (err < 0) {
> 			brelse(bh);
> 			return err;
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 47bee953fc8d..049500f1e764 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -1022,7 +1022,7 @@ static int f2fs_readdir(struct file *file, struct dir_context *ctx)
> 	int err = 0;
> 
> 	if (IS_ENCRYPTED(inode)) {
> -		err = fscrypt_get_encryption_info(inode);
> +		err = fscrypt_prepare_readdir(inode);
> 		if (err)
> 			goto out;
> 
> diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
> index 009fbf844d3e..1f33a5598b93 100644
> --- a/fs/ubifs/dir.c
> +++ b/fs/ubifs/dir.c
> @@ -514,7 +514,7 @@ static int ubifs_readdir(struct file *file, struct dir_context *ctx)
> 		return 0;
> 
> 	if (encrypted) {
> -		err = fscrypt_get_encryption_info(dir);
> +		err = fscrypt_prepare_readdir(dir);
> 		if (err)
> 			return err;
> 
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index 0c9e64969b73..8cbb26f55695 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -242,6 +242,7 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry,
> 			     unsigned int flags);
> int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
> 			     struct fscrypt_name *fname);
> +int __fscrypt_prepare_readdir(struct inode *dir);
> int fscrypt_prepare_setflags(struct inode *inode,
> 			     unsigned int oldflags, unsigned int flags);
> int fscrypt_prepare_symlink(struct inode *dir, const char *target,
> @@ -537,6 +538,11 @@ static inline int __fscrypt_prepare_lookup(struct inode *dir,
> 	return -EOPNOTSUPP;
> }
> 
> +static inline int __fscrypt_prepare_readdir(struct inode *dir)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> static inline int fscrypt_prepare_setflags(struct inode *inode,
> 					   unsigned int oldflags,
> 					   unsigned int flags)
> @@ -795,6 +801,24 @@ static inline int fscrypt_prepare_lookup(struct inode *dir,
> 	return 0;
> }
> 
> +/**
> + * fscrypt_prepare_readdir() - prepare to read a possibly-encrypted directory
> + * @dir: the directory inode
> + *
> + * If the directory is encrypted and it doesn't already have its encryption key
> + * set up, try to set it up so that the filenames will be listed in plaintext
> + * form rather than in no-key form.
> + *
> + * Return: 0 on success; -errno on error.  Note that the encryption key being
> + *	   unavailable is not considered an error.
> + */
> +static inline int fscrypt_prepare_readdir(struct inode *dir)
> +{
> +	if (IS_ENCRYPTED(dir))
> +		return __fscrypt_prepare_readdir(dir);
> +	return 0;
> +}
> +
> /**
>  * fscrypt_prepare_setattr() - prepare to change a possibly-encrypted inode's
>  *			       attributes
> --
> 2.29.2
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

[PATCH 6/9] fscrypt: move body of fscrypt_prepare_setattr() out-of-line

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

In preparation for reducing the visibility of fscrypt_require_key() by
moving it to fscrypt_private.h, move the call to it from
fscrypt_prepare_setattr() to an out-of-line function.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/hooks.c       |  8 ++++++++
 include/linux/fscrypt.h | 11 +++++++++--
 2 files changed, 17 insertions(+), 2 deletions(-)

diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 82f351d3113a..1c16dba222d9 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -120,6 +120,14 @@ int __fscrypt_prepare_readdir(struct inode *dir)
 }
 EXPORT_SYMBOL_GPL(__fscrypt_prepare_readdir);
 
+int __fscrypt_prepare_setattr(struct dentry *dentry, struct iattr *attr)
+{
+	if (attr->ia_valid & ATTR_SIZE)
+		return fscrypt_require_key(d_inode(dentry));
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__fscrypt_prepare_setattr);
+
 /**
  * fscrypt_prepare_setflags() - prepare to change flags with FS_IOC_SETFLAGS
  * @inode: the inode on which flags are being changed
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 8cbb26f55695..b20900bb829f 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -243,6 +243,7 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry,
 int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
 			     struct fscrypt_name *fname);
 int __fscrypt_prepare_readdir(struct inode *dir);
+int __fscrypt_prepare_setattr(struct dentry *dentry, struct iattr *attr);
 int fscrypt_prepare_setflags(struct inode *inode,
 			     unsigned int oldflags, unsigned int flags);
 int fscrypt_prepare_symlink(struct inode *dir, const char *target,
@@ -543,6 +544,12 @@ static inline int __fscrypt_prepare_readdir(struct inode *dir)
 	return -EOPNOTSUPP;
 }
 
+static inline int __fscrypt_prepare_setattr(struct dentry *dentry,
+					    struct iattr *attr)
+{
+	return -EOPNOTSUPP;
+}
+
 static inline int fscrypt_prepare_setflags(struct inode *inode,
 					   unsigned int oldflags,
 					   unsigned int flags)
@@ -840,8 +847,8 @@ static inline int fscrypt_prepare_readdir(struct inode *dir)
 static inline int fscrypt_prepare_setattr(struct dentry *dentry,
 					  struct iattr *attr)
 {
-	if (attr->ia_valid & ATTR_SIZE)
-		return fscrypt_require_key(d_inode(dentry));
+	if (IS_ENCRYPTED(d_inode(dentry)))
+		return __fscrypt_prepare_setattr(dentry, attr);
 	return 0;
 }
 
-- 
2.29.2

Re: [PATCH 6/9] fscrypt: move body of fscrypt_prepare_setattr() out-of-line

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 2788 bytes --]

On Nov 24, 2020, at 5:23 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> 
> From: Eric Biggers <ebiggers@google.com>
> 
> In preparation for reducing the visibility of fscrypt_require_key() by
> moving it to fscrypt_private.h, move the call to it from
> fscrypt_prepare_setattr() to an out-of-line function.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/crypto/hooks.c       |  8 ++++++++
> include/linux/fscrypt.h | 11 +++++++++--
> 2 files changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
> index 82f351d3113a..1c16dba222d9 100644
> --- a/fs/crypto/hooks.c
> +++ b/fs/crypto/hooks.c
> @@ -120,6 +120,14 @@ int __fscrypt_prepare_readdir(struct inode *dir)
> }
> EXPORT_SYMBOL_GPL(__fscrypt_prepare_readdir);
> 
> +int __fscrypt_prepare_setattr(struct dentry *dentry, struct iattr *attr)
> +{
> +	if (attr->ia_valid & ATTR_SIZE)
> +		return fscrypt_require_key(d_inode(dentry));
> +	return 0;
> +}
> +EXPORT_SYMBOL_GPL(__fscrypt_prepare_setattr);
> +
> /**
>  * fscrypt_prepare_setflags() - prepare to change flags with FS_IOC_SETFLAGS
>  * @inode: the inode on which flags are being changed
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index 8cbb26f55695..b20900bb829f 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -243,6 +243,7 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry,
> int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
> 			     struct fscrypt_name *fname);
> int __fscrypt_prepare_readdir(struct inode *dir);
> +int __fscrypt_prepare_setattr(struct dentry *dentry, struct iattr *attr);
> int fscrypt_prepare_setflags(struct inode *inode,
> 			     unsigned int oldflags, unsigned int flags);
> int fscrypt_prepare_symlink(struct inode *dir, const char *target,
> @@ -543,6 +544,12 @@ static inline int __fscrypt_prepare_readdir(struct inode *dir)
> 	return -EOPNOTSUPP;
> }
> 
> +static inline int __fscrypt_prepare_setattr(struct dentry *dentry,
> +					    struct iattr *attr)
> +{
> +	return -EOPNOTSUPP;
> +}
> +
> static inline int fscrypt_prepare_setflags(struct inode *inode,
> 					   unsigned int oldflags,
> 					   unsigned int flags)
> @@ -840,8 +847,8 @@ static inline int fscrypt_prepare_readdir(struct inode *dir)
> static inline int fscrypt_prepare_setattr(struct dentry *dentry,
> 					  struct iattr *attr)
> {
> -	if (attr->ia_valid & ATTR_SIZE)
> -		return fscrypt_require_key(d_inode(dentry));
> +	if (IS_ENCRYPTED(d_inode(dentry)))
> +		return __fscrypt_prepare_setattr(dentry, attr);
> 	return 0;
> }
> 
> --
> 2.29.2
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

[PATCH 7/9] fscrypt: move fscrypt_require_key() to fscrypt_private.h

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

fscrypt_require_key() is now only used by files in fs/crypto/.  So
reduce its visibility to fscrypt_private.h.  This is also a prerequsite
for unexporting fscrypt_get_encryption_info().

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h | 26 ++++++++++++++++++++++++++
 include/linux/fscrypt.h     | 26 --------------------------
 2 files changed, 26 insertions(+), 26 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index a61d4dbf0a0b..16dd55080127 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -571,6 +571,32 @@ int fscrypt_derive_dirhash_key(struct fscrypt_info *ci,
 void fscrypt_hash_inode_number(struct fscrypt_info *ci,
 			       const struct fscrypt_master_key *mk);
 
+/**
+ * fscrypt_require_key() - require an inode's encryption key
+ * @inode: the inode we need the key for
+ *
+ * If the inode is encrypted, set up its encryption key if not already done.
+ * Then require that the key be present and return -ENOKEY otherwise.
+ *
+ * No locks are needed, and the key will live as long as the struct inode --- so
+ * it won't go away from under you.
+ *
+ * Return: 0 on success, -ENOKEY if the key is missing, or another -errno code
+ * if a problem occurred while setting up the encryption key.
+ */
+static inline int fscrypt_require_key(struct inode *inode)
+{
+	if (IS_ENCRYPTED(inode)) {
+		int err = fscrypt_get_encryption_info(inode);
+
+		if (err)
+			return err;
+		if (!fscrypt_has_encryption_key(inode))
+			return -ENOKEY;
+	}
+	return 0;
+}
+
 /* keysetup_v1.c */
 
 void fscrypt_put_direct_key(struct fscrypt_direct_key *dk);
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index b20900bb829f..a07610f27926 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -688,32 +688,6 @@ static inline bool fscrypt_has_encryption_key(const struct inode *inode)
 	return fscrypt_get_info(inode) != NULL;
 }
 
-/**
- * fscrypt_require_key() - require an inode's encryption key
- * @inode: the inode we need the key for
- *
- * If the inode is encrypted, set up its encryption key if not already done.
- * Then require that the key be present and return -ENOKEY otherwise.
- *
- * No locks are needed, and the key will live as long as the struct inode --- so
- * it won't go away from under you.
- *
- * Return: 0 on success, -ENOKEY if the key is missing, or another -errno code
- * if a problem occurred while setting up the encryption key.
- */
-static inline int fscrypt_require_key(struct inode *inode)
-{
-	if (IS_ENCRYPTED(inode)) {
-		int err = fscrypt_get_encryption_info(inode);
-
-		if (err)
-			return err;
-		if (!fscrypt_has_encryption_key(inode))
-			return -ENOKEY;
-	}
-	return 0;
-}
-
 /**
  * fscrypt_prepare_link() - prepare to link an inode into a possibly-encrypted
  *			    directory
-- 
2.29.2

Re: [PATCH 7/9] fscrypt: move fscrypt_require_key() to fscrypt_private.h

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 3326 bytes --]


> On Nov 24, 2020, at 5:23 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> 
> From: Eric Biggers <ebiggers@google.com>
> 
> fscrypt_require_key() is now only used by files in fs/crypto/.  So
> reduce its visibility to fscrypt_private.h.  This is also a prerequsite
> for unexporting fscrypt_get_encryption_info().
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/crypto/fscrypt_private.h | 26 ++++++++++++++++++++++++++
> include/linux/fscrypt.h     | 26 --------------------------
> 2 files changed, 26 insertions(+), 26 deletions(-)
> 
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index a61d4dbf0a0b..16dd55080127 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -571,6 +571,32 @@ int fscrypt_derive_dirhash_key(struct fscrypt_info *ci,
> void fscrypt_hash_inode_number(struct fscrypt_info *ci,
> 			       const struct fscrypt_master_key *mk);
> 
> +/**
> + * fscrypt_require_key() - require an inode's encryption key
> + * @inode: the inode we need the key for
> + *
> + * If the inode is encrypted, set up its encryption key if not already done.
> + * Then require that the key be present and return -ENOKEY otherwise.
> + *
> + * No locks are needed, and the key will live as long as the struct inode --- so
> + * it won't go away from under you.
> + *
> + * Return: 0 on success, -ENOKEY if the key is missing, or another -errno code
> + * if a problem occurred while setting up the encryption key.
> + */
> +static inline int fscrypt_require_key(struct inode *inode)
> +{
> +	if (IS_ENCRYPTED(inode)) {
> +		int err = fscrypt_get_encryption_info(inode);
> +
> +		if (err)
> +			return err;
> +		if (!fscrypt_has_encryption_key(inode))
> +			return -ENOKEY;
> +	}
> +	return 0;
> +}
> +
> /* keysetup_v1.c */
> 
> void fscrypt_put_direct_key(struct fscrypt_direct_key *dk);
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index b20900bb829f..a07610f27926 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -688,32 +688,6 @@ static inline bool fscrypt_has_encryption_key(const struct inode *inode)
> 	return fscrypt_get_info(inode) != NULL;
> }
> 
> -/**
> - * fscrypt_require_key() - require an inode's encryption key
> - * @inode: the inode we need the key for
> - *
> - * If the inode is encrypted, set up its encryption key if not already done.
> - * Then require that the key be present and return -ENOKEY otherwise.
> - *
> - * No locks are needed, and the key will live as long as the struct inode --- so
> - * it won't go away from under you.
> - *
> - * Return: 0 on success, -ENOKEY if the key is missing, or another -errno code
> - * if a problem occurred while setting up the encryption key.
> - */
> -static inline int fscrypt_require_key(struct inode *inode)
> -{
> -	if (IS_ENCRYPTED(inode)) {
> -		int err = fscrypt_get_encryption_info(inode);
> -
> -		if (err)
> -			return err;
> -		if (!fscrypt_has_encryption_key(inode))
> -			return -ENOKEY;
> -	}
> -	return 0;
> -}
> -
> /**
>  * fscrypt_prepare_link() - prepare to link an inode into a possibly-encrypted
>  *			    directory
> --
> 2.29.2
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

[PATCH 8/9] fscrypt: unexport fscrypt_get_encryption_info()

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Now that fscrypt_get_encryption_info() is only called from files in
fs/crypto/ (due to all key setup now being handled by higher-level
helper functions instead of directly by filesystems), unexport it and
move its declaration to fscrypt_private.h.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fscrypt_private.h | 2 ++
 fs/crypto/keysetup.c        | 1 -
 include/linux/fscrypt.h     | 7 +------
 3 files changed, 3 insertions(+), 7 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 16dd55080127..c1c302656c34 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -571,6 +571,8 @@ int fscrypt_derive_dirhash_key(struct fscrypt_info *ci,
 void fscrypt_hash_inode_number(struct fscrypt_info *ci,
 			       const struct fscrypt_master_key *mk);
 
+int fscrypt_get_encryption_info(struct inode *inode);
+
 /**
  * fscrypt_require_key() - require an inode's encryption key
  * @inode: the inode we need the key for
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 50675b42d5b7..6339b3069a40 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -589,7 +589,6 @@ int fscrypt_get_encryption_info(struct inode *inode)
 		res = 0;
 	return res;
 }
-EXPORT_SYMBOL(fscrypt_get_encryption_info);
 
 /**
  * fscrypt_prepare_new_inode() - prepare to create a new inode in a directory
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index a07610f27926..4b163f5e58e9 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -75,7 +75,7 @@ struct fscrypt_operations {
 static inline struct fscrypt_info *fscrypt_get_info(const struct inode *inode)
 {
 	/*
-	 * Pairs with the cmpxchg_release() in fscrypt_get_encryption_info().
+	 * Pairs with the cmpxchg_release() in fscrypt_setup_encryption_info().
 	 * I.e., another task may publish ->i_crypt_info concurrently, executing
 	 * a RELEASE barrier.  We need to use smp_load_acquire() here to safely
 	 * ACQUIRE the memory the other task published.
@@ -200,7 +200,6 @@ int fscrypt_ioctl_remove_key_all_users(struct file *filp, void __user *arg);
 int fscrypt_ioctl_get_key_status(struct file *filp, void __user *arg);
 
 /* keysetup.c */
-int fscrypt_get_encryption_info(struct inode *inode);
 int fscrypt_prepare_new_inode(struct inode *dir, struct inode *inode,
 			      bool *encrypt_ret);
 void fscrypt_put_encryption_info(struct inode *inode);
@@ -408,10 +407,6 @@ static inline int fscrypt_ioctl_get_key_status(struct file *filp,
 }
 
 /* keysetup.c */
-static inline int fscrypt_get_encryption_info(struct inode *inode)
-{
-	return -EOPNOTSUPP;
-}
 
 static inline int fscrypt_prepare_new_inode(struct inode *dir,
 					    struct inode *inode,
-- 
2.29.2

Re: [PATCH 8/9] fscrypt: unexport fscrypt_get_encryption_info()

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 3130 bytes --]


> On Nov 24, 2020, at 5:23 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> 
> From: Eric Biggers <ebiggers@google.com>
> 
> Now that fscrypt_get_encryption_info() is only called from files in
> fs/crypto/ (due to all key setup now being handled by higher-level
> helper functions instead of directly by filesystems), unexport it and
> move its declaration to fscrypt_private.h.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/crypto/fscrypt_private.h | 2 ++
> fs/crypto/keysetup.c        | 1 -
> include/linux/fscrypt.h     | 7 +------
> 3 files changed, 3 insertions(+), 7 deletions(-)
> 
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index 16dd55080127..c1c302656c34 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -571,6 +571,8 @@ int fscrypt_derive_dirhash_key(struct fscrypt_info *ci,
> void fscrypt_hash_inode_number(struct fscrypt_info *ci,
> 			       const struct fscrypt_master_key *mk);
> 
> +int fscrypt_get_encryption_info(struct inode *inode);
> +
> /**
>  * fscrypt_require_key() - require an inode's encryption key
>  * @inode: the inode we need the key for
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index 50675b42d5b7..6339b3069a40 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -589,7 +589,6 @@ int fscrypt_get_encryption_info(struct inode *inode)
> 		res = 0;
> 	return res;
> }
> -EXPORT_SYMBOL(fscrypt_get_encryption_info);
> 
> /**
>  * fscrypt_prepare_new_inode() - prepare to create a new inode in a directory
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index a07610f27926..4b163f5e58e9 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -75,7 +75,7 @@ struct fscrypt_operations {
> static inline struct fscrypt_info *fscrypt_get_info(const struct inode *inode)
> {
> 	/*
> -	 * Pairs with the cmpxchg_release() in fscrypt_get_encryption_info().
> +	 * Pairs with the cmpxchg_release() in fscrypt_setup_encryption_info().
> 	 * I.e., another task may publish ->i_crypt_info concurrently, executing
> 	 * a RELEASE barrier.  We need to use smp_load_acquire() here to safely
> 	 * ACQUIRE the memory the other task published.
> @@ -200,7 +200,6 @@ int fscrypt_ioctl_remove_key_all_users(struct file *filp, void __user *arg);
> int fscrypt_ioctl_get_key_status(struct file *filp, void __user *arg);
> 
> /* keysetup.c */
> -int fscrypt_get_encryption_info(struct inode *inode);
> int fscrypt_prepare_new_inode(struct inode *dir, struct inode *inode,
> 			      bool *encrypt_ret);
> void fscrypt_put_encryption_info(struct inode *inode);
> @@ -408,10 +407,6 @@ static inline int fscrypt_ioctl_get_key_status(struct file *filp,
> }
> 
> /* keysetup.c */
> -static inline int fscrypt_get_encryption_info(struct inode *inode)
> -{
> -	return -EOPNOTSUPP;
> -}
> 
> static inline int fscrypt_prepare_new_inode(struct inode *dir,
> 					    struct inode *inode,
> --
> 2.29.2
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

[PATCH 9/9] fscrypt: allow deleting files with unsupported encryption policy

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Currently it's impossible to delete files that use an unsupported
encryption policy, as the kernel will just return an error when
performing any operation on the top-level encrypted directory, even just
a path lookup into the directory or opening the directory for readdir.

More specifically, this occurs in any of the following cases:

- The encryption context has an unrecognized version number.  Current
  kernels know about v1 and v2, but there could be more versions in the
  future.

- The encryption context has unrecognized encryption modes
  (FSCRYPT_MODE_*) or flags (FSCRYPT_POLICY_FLAG_*), an unrecognized
  combination of modes, or reserved bits set.

- The encryption key has been added and the encryption modes are
  recognized but aren't available in the crypto API -- for example, a
  directory is encrypted with FSCRYPT_MODE_ADIANTUM but the kernel
  doesn't have CONFIG_CRYPTO_ADIANTUM enabled.

It's desirable to return errors for most operations on files that use an
unsupported encryption policy, but the current behavior is too strict.
We need to allow enough to delete files, so that people can't be stuck
with undeletable files when downgrading kernel versions.  That includes
allowing directories to be listed and allowing dentries to be looked up.

Fix this by modifying the key setup logic to treat an unsupported
encryption policy in the same way as "key unavailable" in the cases that
are required for a recursive delete to work: preparing for a readdir or
a dentry lookup, revalidating a dentry, or checking whether an inode has
the same encryption policy as its parent directory.

Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/fname.c           |  8 ++++++--
 fs/crypto/fscrypt_private.h |  4 ++--
 fs/crypto/hooks.c           |  4 ++--
 fs/crypto/keysetup.c        | 19 +++++++++++++++++--
 fs/crypto/policy.c          | 22 ++++++++++++++--------
 include/linux/fscrypt.h     |  9 ++++++---
 6 files changed, 47 insertions(+), 19 deletions(-)

diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 1fbe6c24d705..988dadf7a94d 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -404,7 +404,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 		fname->disk_name.len = iname->len;
 		return 0;
 	}
-	ret = fscrypt_get_encryption_info(dir);
+	ret = fscrypt_get_encryption_info(dir, lookup);
 	if (ret)
 		return ret;
 
@@ -560,7 +560,11 @@ int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 		return -ECHILD;
 
 	dir = dget_parent(dentry);
-	err = fscrypt_get_encryption_info(d_inode(dir));
+	/*
+	 * Pass allow_unsupported=true, so that files with an unsupported
+	 * encryption policy can be deleted.
+	 */
+	err = fscrypt_get_encryption_info(d_inode(dir), true);
 	valid = !fscrypt_has_encryption_key(d_inode(dir));
 	dput(dir);
 
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index c1c302656c34..f0bed6b06fa6 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -571,7 +571,7 @@ int fscrypt_derive_dirhash_key(struct fscrypt_info *ci,
 void fscrypt_hash_inode_number(struct fscrypt_info *ci,
 			       const struct fscrypt_master_key *mk);
 
-int fscrypt_get_encryption_info(struct inode *inode);
+int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported);
 
 /**
  * fscrypt_require_key() - require an inode's encryption key
@@ -589,7 +589,7 @@ int fscrypt_get_encryption_info(struct inode *inode);
 static inline int fscrypt_require_key(struct inode *inode)
 {
 	if (IS_ENCRYPTED(inode)) {
-		int err = fscrypt_get_encryption_info(inode);
+		int err = fscrypt_get_encryption_info(inode, false);
 
 		if (err)
 			return err;
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 1c16dba222d9..79570e0e8e61 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -116,7 +116,7 @@ EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup);
 
 int __fscrypt_prepare_readdir(struct inode *dir)
 {
-	return fscrypt_get_encryption_info(dir);
+	return fscrypt_get_encryption_info(dir, true);
 }
 EXPORT_SYMBOL_GPL(__fscrypt_prepare_readdir);
 
@@ -332,7 +332,7 @@ const char *fscrypt_get_symlink(struct inode *inode, const void *caddr,
 	 * Try to set up the symlink's encryption key, but we can continue
 	 * regardless of whether the key is available or not.
 	 */
-	err = fscrypt_get_encryption_info(inode);
+	err = fscrypt_get_encryption_info(inode, false);
 	if (err)
 		return ERR_PTR(err);
 	has_key = fscrypt_has_encryption_key(inode);
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 6339b3069a40..261293fb7097 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -546,6 +546,11 @@ fscrypt_setup_encryption_info(struct inode *inode,
 /**
  * fscrypt_get_encryption_info() - set up an inode's encryption key
  * @inode: the inode to set up the key for.  Must be encrypted.
+ * @allow_unsupported: if %true, treat an unsupported encryption policy (or
+ *		       unrecognized encryption context) the same way as the key
+ *		       being unavailable, instead of returning an error.  Use
+ *		       %false unless the operation being performed is needed in
+ *		       order for files (or directories) to be deleted.
  *
  * Set up ->i_crypt_info, if it hasn't already been done.
  *
@@ -556,7 +561,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
  *	   encryption key is unavailable.  (Use fscrypt_has_encryption_key() to
  *	   distinguish these cases.)  Also can return another -errno code.
  */
-int fscrypt_get_encryption_info(struct inode *inode)
+int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
 {
 	int res;
 	union fscrypt_context ctx;
@@ -567,24 +572,34 @@ int fscrypt_get_encryption_info(struct inode *inode)
 
 	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
 	if (res < 0) {
+		if (res == -ERANGE && allow_unsupported)
+			return 0;
 		fscrypt_warn(inode, "Error %d getting encryption context", res);
 		return res;
 	}
 
 	res = fscrypt_policy_from_context(&policy, &ctx, res);
 	if (res) {
+		if (allow_unsupported)
+			return 0;
 		fscrypt_warn(inode,
 			     "Unrecognized or corrupt encryption context");
 		return res;
 	}
 
-	if (!fscrypt_supported_policy(&policy, inode))
+	if (!fscrypt_supported_policy(&policy, inode)) {
+		if (allow_unsupported)
+			return 0;
 		return -EINVAL;
+	}
 
 	res = fscrypt_setup_encryption_info(inode, &policy,
 					    fscrypt_context_nonce(&ctx),
 					    IS_CASEFOLDED(inode) &&
 					    S_ISDIR(inode->i_mode));
+
+	if (res == -ENOPKG && allow_unsupported) /* Algorithm unavailable? */
+		res = 0;
 	if (res == -ENOKEY)
 		res = 0;
 	return res;
diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
index faa0f21daa68..a51cef6bd27f 100644
--- a/fs/crypto/policy.c
+++ b/fs/crypto/policy.c
@@ -590,7 +590,7 @@ EXPORT_SYMBOL_GPL(fscrypt_ioctl_get_nonce);
 int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 {
 	union fscrypt_policy parent_policy, child_policy;
-	int err;
+	int err, err1, err2;
 
 	/* No restrictions on file types which are never encrypted */
 	if (!S_ISREG(child->i_mode) && !S_ISDIR(child->i_mode) &&
@@ -620,19 +620,25 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
 	 * In any case, if an unexpected error occurs, fall back to "forbidden".
 	 */
 
-	err = fscrypt_get_encryption_info(parent);
+	err = fscrypt_get_encryption_info(parent, true);
 	if (err)
 		return 0;
-	err = fscrypt_get_encryption_info(child);
+	err = fscrypt_get_encryption_info(child, true);
 	if (err)
 		return 0;
 
-	err = fscrypt_get_policy(parent, &parent_policy);
-	if (err)
-		return 0;
+	err1 = fscrypt_get_policy(parent, &parent_policy);
+	err2 = fscrypt_get_policy(child, &child_policy);
 
-	err = fscrypt_get_policy(child, &child_policy);
-	if (err)
+	/*
+	 * Allow the case where the parent and child both have an unrecognized
+	 * encryption policy, so that files with an unrecognized encryption
+	 * policy can be deleted.
+	 */
+	if (err1 == -EINVAL && err2 == -EINVAL)
+		return 1;
+
+	if (err1 || err2)
 		return 0;
 
 	return fscrypt_policies_equal(&parent_policy, &child_policy);
diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
index 4b163f5e58e9..d23156d1ac94 100644
--- a/include/linux/fscrypt.h
+++ b/include/linux/fscrypt.h
@@ -753,8 +753,9 @@ static inline int fscrypt_prepare_rename(struct inode *old_dir,
  *
  * Prepare for ->lookup() in a directory which may be encrypted by determining
  * the name that will actually be used to search the directory on-disk.  If the
- * directory's encryption key is available, then the lookup is assumed to be by
- * plaintext name; otherwise, it is assumed to be by no-key name.
+ * directory's encryption policy is supported by this kernel and its encryption
+ * key is available, then the lookup is assumed to be by plaintext name;
+ * otherwise, it is assumed to be by no-key name.
  *
  * This also installs a custom ->d_revalidate() method which will invalidate the
  * dentry if it was created without the key and the key is later added.
@@ -786,7 +787,9 @@ static inline int fscrypt_prepare_lookup(struct inode *dir,
  * form rather than in no-key form.
  *
  * Return: 0 on success; -errno on error.  Note that the encryption key being
- *	   unavailable is not considered an error.
+ *	   unavailable is not considered an error.  It is also not an error if
+ *	   the encryption policy is unsupported by this kernel; that is treated
+ *	   like the key being unavailable, so that files can still be deleted.
  */
 static inline int fscrypt_prepare_readdir(struct inode *dir)
 {
-- 
2.29.2

Re: [PATCH 9/9] fscrypt: allow deleting files with unsupported encryption policy

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 10476 bytes --]

On Nov 24, 2020, at 5:23 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> 
> From: Eric Biggers <ebiggers@google.com>
> 
> Currently it's impossible to delete files that use an unsupported
> encryption policy, as the kernel will just return an error when
> performing any operation on the top-level encrypted directory, even just
> a path lookup into the directory or opening the directory for readdir.
> 
> More specifically, this occurs in any of the following cases:
> 
> - The encryption context has an unrecognized version number.  Current
>  kernels know about v1 and v2, but there could be more versions in the
>  future.
> 
> - The encryption context has unrecognized encryption modes
>  (FSCRYPT_MODE_*) or flags (FSCRYPT_POLICY_FLAG_*), an unrecognized
>  combination of modes, or reserved bits set.
> 
> - The encryption key has been added and the encryption modes are
>  recognized but aren't available in the crypto API -- for example, a
>  directory is encrypted with FSCRYPT_MODE_ADIANTUM but the kernel
>  doesn't have CONFIG_CRYPTO_ADIANTUM enabled.
> 
> It's desirable to return errors for most operations on files that use an
> unsupported encryption policy, but the current behavior is too strict.
> We need to allow enough to delete files, so that people can't be stuck
> with undeletable files when downgrading kernel versions.  That includes
> allowing directories to be listed and allowing dentries to be looked up.
> 
> Fix this by modifying the key setup logic to treat an unsupported
> encryption policy in the same way as "key unavailable" in the cases that
> are required for a recursive delete to work: preparing for a readdir or
> a dentry lookup, revalidating a dentry, or checking whether an inode has
> the same encryption policy as its parent directory.
> 
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Reviewed-by: Andreas Dilger <adilger@dilger.ca>

> ---
> fs/crypto/fname.c           |  8 ++++++--
> fs/crypto/fscrypt_private.h |  4 ++--
> fs/crypto/hooks.c           |  4 ++--
> fs/crypto/keysetup.c        | 19 +++++++++++++++++--
> fs/crypto/policy.c          | 22 ++++++++++++++--------
> include/linux/fscrypt.h     |  9 ++++++---
> 6 files changed, 47 insertions(+), 19 deletions(-)
> 
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 1fbe6c24d705..988dadf7a94d 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -404,7 +404,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
> 		fname->disk_name.len = iname->len;
> 		return 0;
> 	}
> -	ret = fscrypt_get_encryption_info(dir);
> +	ret = fscrypt_get_encryption_info(dir, lookup);
> 	if (ret)
> 		return ret;
> 
> @@ -560,7 +560,11 @@ int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
> 		return -ECHILD;
> 
> 	dir = dget_parent(dentry);
> -	err = fscrypt_get_encryption_info(d_inode(dir));
> +	/*
> +	 * Pass allow_unsupported=true, so that files with an unsupported
> +	 * encryption policy can be deleted.
> +	 */
> +	err = fscrypt_get_encryption_info(d_inode(dir), true);
> 	valid = !fscrypt_has_encryption_key(d_inode(dir));
> 	dput(dir);
> 
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index c1c302656c34..f0bed6b06fa6 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -571,7 +571,7 @@ int fscrypt_derive_dirhash_key(struct fscrypt_info *ci,
> void fscrypt_hash_inode_number(struct fscrypt_info *ci,
> 			       const struct fscrypt_master_key *mk);
> 
> -int fscrypt_get_encryption_info(struct inode *inode);
> +int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported);
> 
> /**
>  * fscrypt_require_key() - require an inode's encryption key
> @@ -589,7 +589,7 @@ int fscrypt_get_encryption_info(struct inode *inode);
> static inline int fscrypt_require_key(struct inode *inode)
> {
> 	if (IS_ENCRYPTED(inode)) {
> -		int err = fscrypt_get_encryption_info(inode);
> +		int err = fscrypt_get_encryption_info(inode, false);
> 
> 		if (err)
> 			return err;
> diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
> index 1c16dba222d9..79570e0e8e61 100644
> --- a/fs/crypto/hooks.c
> +++ b/fs/crypto/hooks.c
> @@ -116,7 +116,7 @@ EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup);
> 
> int __fscrypt_prepare_readdir(struct inode *dir)
> {
> -	return fscrypt_get_encryption_info(dir);
> +	return fscrypt_get_encryption_info(dir, true);
> }
> EXPORT_SYMBOL_GPL(__fscrypt_prepare_readdir);
> 
> @@ -332,7 +332,7 @@ const char *fscrypt_get_symlink(struct inode *inode, const void *caddr,
> 	 * Try to set up the symlink's encryption key, but we can continue
> 	 * regardless of whether the key is available or not.
> 	 */
> -	err = fscrypt_get_encryption_info(inode);
> +	err = fscrypt_get_encryption_info(inode, false);
> 	if (err)
> 		return ERR_PTR(err);
> 	has_key = fscrypt_has_encryption_key(inode);
> diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
> index 6339b3069a40..261293fb7097 100644
> --- a/fs/crypto/keysetup.c
> +++ b/fs/crypto/keysetup.c
> @@ -546,6 +546,11 @@ fscrypt_setup_encryption_info(struct inode *inode,
> /**
>  * fscrypt_get_encryption_info() - set up an inode's encryption key
>  * @inode: the inode to set up the key for.  Must be encrypted.
> + * @allow_unsupported: if %true, treat an unsupported encryption policy (or
> + *		       unrecognized encryption context) the same way as the key
> + *		       being unavailable, instead of returning an error.  Use
> + *		       %false unless the operation being performed is needed in
> + *		       order for files (or directories) to be deleted.
>  *
>  * Set up ->i_crypt_info, if it hasn't already been done.
>  *
> @@ -556,7 +561,7 @@ fscrypt_setup_encryption_info(struct inode *inode,
>  *	   encryption key is unavailable.  (Use fscrypt_has_encryption_key() to
>  *	   distinguish these cases.)  Also can return another -errno code.
>  */
> -int fscrypt_get_encryption_info(struct inode *inode)
> +int fscrypt_get_encryption_info(struct inode *inode, bool allow_unsupported)
> {
> 	int res;
> 	union fscrypt_context ctx;
> @@ -567,24 +572,34 @@ int fscrypt_get_encryption_info(struct inode *inode)
> 
> 	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
> 	if (res < 0) {
> +		if (res == -ERANGE && allow_unsupported)
> +			return 0;
> 		fscrypt_warn(inode, "Error %d getting encryption context", res);
> 		return res;
> 	}
> 
> 	res = fscrypt_policy_from_context(&policy, &ctx, res);
> 	if (res) {
> +		if (allow_unsupported)
> +			return 0;
> 		fscrypt_warn(inode,
> 			     "Unrecognized or corrupt encryption context");
> 		return res;
> 	}
> 
> -	if (!fscrypt_supported_policy(&policy, inode))
> +	if (!fscrypt_supported_policy(&policy, inode)) {
> +		if (allow_unsupported)
> +			return 0;
> 		return -EINVAL;
> +	}
> 
> 	res = fscrypt_setup_encryption_info(inode, &policy,
> 					    fscrypt_context_nonce(&ctx),
> 					    IS_CASEFOLDED(inode) &&
> 					    S_ISDIR(inode->i_mode));
> +
> +	if (res == -ENOPKG && allow_unsupported) /* Algorithm unavailable? */
> +		res = 0;
> 	if (res == -ENOKEY)
> 		res = 0;
> 	return res;
> diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c
> index faa0f21daa68..a51cef6bd27f 100644
> --- a/fs/crypto/policy.c
> +++ b/fs/crypto/policy.c
> @@ -590,7 +590,7 @@ EXPORT_SYMBOL_GPL(fscrypt_ioctl_get_nonce);
> int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
> {
> 	union fscrypt_policy parent_policy, child_policy;
> -	int err;
> +	int err, err1, err2;
> 
> 	/* No restrictions on file types which are never encrypted */
> 	if (!S_ISREG(child->i_mode) && !S_ISDIR(child->i_mode) &&
> @@ -620,19 +620,25 @@ int fscrypt_has_permitted_context(struct inode *parent, struct inode *child)
> 	 * In any case, if an unexpected error occurs, fall back to "forbidden".
> 	 */
> 
> -	err = fscrypt_get_encryption_info(parent);
> +	err = fscrypt_get_encryption_info(parent, true);
> 	if (err)
> 		return 0;
> -	err = fscrypt_get_encryption_info(child);
> +	err = fscrypt_get_encryption_info(child, true);
> 	if (err)
> 		return 0;
> 
> -	err = fscrypt_get_policy(parent, &parent_policy);
> -	if (err)
> -		return 0;
> +	err1 = fscrypt_get_policy(parent, &parent_policy);
> +	err2 = fscrypt_get_policy(child, &child_policy);
> 
> -	err = fscrypt_get_policy(child, &child_policy);
> -	if (err)
> +	/*
> +	 * Allow the case where the parent and child both have an unrecognized
> +	 * encryption policy, so that files with an unrecognized encryption
> +	 * policy can be deleted.
> +	 */
> +	if (err1 == -EINVAL && err2 == -EINVAL)
> +		return 1;
> +
> +	if (err1 || err2)
> 		return 0;
> 
> 	return fscrypt_policies_equal(&parent_policy, &child_policy);
> diff --git a/include/linux/fscrypt.h b/include/linux/fscrypt.h
> index 4b163f5e58e9..d23156d1ac94 100644
> --- a/include/linux/fscrypt.h
> +++ b/include/linux/fscrypt.h
> @@ -753,8 +753,9 @@ static inline int fscrypt_prepare_rename(struct inode *old_dir,
>  *
>  * Prepare for ->lookup() in a directory which may be encrypted by determining
>  * the name that will actually be used to search the directory on-disk.  If the
> - * directory's encryption key is available, then the lookup is assumed to be by
> - * plaintext name; otherwise, it is assumed to be by no-key name.
> + * directory's encryption policy is supported by this kernel and its encryption
> + * key is available, then the lookup is assumed to be by plaintext name;
> + * otherwise, it is assumed to be by no-key name.
>  *
>  * This also installs a custom ->d_revalidate() method which will invalidate the
>  * dentry if it was created without the key and the key is later added.
> @@ -786,7 +787,9 @@ static inline int fscrypt_prepare_lookup(struct inode *dir,
>  * form rather than in no-key form.
>  *
>  * Return: 0 on success; -errno on error.  Note that the encryption key being
> - *	   unavailable is not considered an error.
> + *	   unavailable is not considered an error.  It is also not an error if
> + *	   the encryption policy is unsupported by this kernel; that is treated
> + *	   like the key being unavailable, so that files can still be deleted.
>  */
> static inline int fscrypt_prepare_readdir(struct inode *dir)
> {
> --
> 2.29.2
> 


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]

Re: [PATCH 0/9] Allow deleting files with unsupported encryption policy

[Eric Biggers]

On Tue, Nov 24, 2020 at 04:23:27PM -0800, Eric Biggers wrote:
> Currently it's impossible to delete files that use an unsupported
> encryption policy, as the kernel will just return an error when
> performing any operation on the top-level encrypted directory, even just
> a path lookup into the directory or opening the directory for readdir.
> 
> It's desirable to return errors for most operations on files that use an
> unsupported encryption policy, but the current behavior is too strict.
> We need to allow enough to delete files, so that people can't be stuck
> with undeletable files when downgrading kernel versions.  That includes
> allowing directories to be listed and allowing dentries to be looked up.
> 
> This series fixes this (on ext4, f2fs, and ubifs) by treating an
> unsupported encryption policy in the same way as "key unavailable" in
> the cases that are required for a recursive delete to work.
> 
> The actual fix is in patch 9, so see that for more details.
> 
> Patches 1-8 are cleanups that prepare for the actual fix by removing
> direct use of fscrypt_get_encryption_info() by filesystems.
> 
> This patchset applies to branch "master" (commit 4a4b8721f1a5) of
> https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git.
> 
> Eric Biggers (9):
>   ext4: remove ext4_dir_open()
>   f2fs: remove f2fs_dir_open()
>   ubifs: remove ubifs_dir_open()
>   ext4: don't call fscrypt_get_encryption_info() from dx_show_leaf()
>   fscrypt: introduce fscrypt_prepare_readdir()
>   fscrypt: move body of fscrypt_prepare_setattr() out-of-line
>   fscrypt: move fscrypt_require_key() to fscrypt_private.h
>   fscrypt: unexport fscrypt_get_encryption_info()
>   fscrypt: allow deleting files with unsupported encryption policy
> 
>  fs/crypto/fname.c           |  8 +++-
>  fs/crypto/fscrypt_private.h | 28 ++++++++++++++
>  fs/crypto/hooks.c           | 16 +++++++-
>  fs/crypto/keysetup.c        | 20 ++++++++--
>  fs/crypto/policy.c          | 22 +++++++----
>  fs/ext4/dir.c               | 16 ++------
>  fs/ext4/namei.c             | 10 +----
>  fs/f2fs/dir.c               | 10 +----
>  fs/ubifs/dir.c              | 11 +-----
>  include/linux/fscrypt.h     | 75 +++++++++++++++++++------------------
>  10 files changed, 126 insertions(+), 90 deletions(-)
> 
> 
> base-commit: 4a4b8721f1a5e4b01e45b3153c68d5a1014b25de

Any more comments on this patch series?

- Eric

Re: [PATCH 0/9] Allow deleting files with unsupported encryption policy

[Andreas Dilger]

[-- Attachment #1: Type: text/plain, Size: 2657 bytes --]

On Dec 2, 2020, at 2:07 PM, Eric Biggers <ebiggers@kernel.org> wrote:
> 
> On Tue, Nov 24, 2020 at 04:23:27PM -0800, Eric Biggers wrote:
>> Currently it's impossible to delete files that use an unsupported
>> encryption policy, as the kernel will just return an error when
>> performing any operation on the top-level encrypted directory, even just
>> a path lookup into the directory or opening the directory for readdir.
>> 
>> It's desirable to return errors for most operations on files that use an
>> unsupported encryption policy, but the current behavior is too strict.
>> We need to allow enough to delete files, so that people can't be stuck
>> with undeletable files when downgrading kernel versions.  That includes
>> allowing directories to be listed and allowing dentries to be looked up.
>> 
>> This series fixes this (on ext4, f2fs, and ubifs) by treating an
>> unsupported encryption policy in the same way as "key unavailable" in
>> the cases that are required for a recursive delete to work.
>> 
>> The actual fix is in patch 9, so see that for more details.
>> 
>> Patches 1-8 are cleanups that prepare for the actual fix by removing
>> direct use of fscrypt_get_encryption_info() by filesystems.
>> 
>> This patchset applies to branch "master" (commit 4a4b8721f1a5) of
>> https://git.kernel.org/pub/scm/fs/fscrypt/fscrypt.git.
>> 
>> Eric Biggers (9):
>>  ext4: remove ext4_dir_open()
>>  f2fs: remove f2fs_dir_open()
>>  ubifs: remove ubifs_dir_open()
>>  ext4: don't call fscrypt_get_encryption_info() from dx_show_leaf()
>>  fscrypt: introduce fscrypt_prepare_readdir()
>>  fscrypt: move body of fscrypt_prepare_setattr() out-of-line
>>  fscrypt: move fscrypt_require_key() to fscrypt_private.h
>>  fscrypt: unexport fscrypt_get_encryption_info()
>>  fscrypt: allow deleting files with unsupported encryption policy
>> 
>> fs/crypto/fname.c           |  8 +++-
>> fs/crypto/fscrypt_private.h | 28 ++++++++++++++
>> fs/crypto/hooks.c           | 16 +++++++-
>> fs/crypto/keysetup.c        | 20 ++++++++--
>> fs/crypto/policy.c          | 22 +++++++----
>> fs/ext4/dir.c               | 16 ++------
>> fs/ext4/namei.c             | 10 +----
>> fs/f2fs/dir.c               | 10 +----
>> fs/ubifs/dir.c              | 11 +-----
>> include/linux/fscrypt.h     | 75 +++++++++++++++++++------------------
>> 10 files changed, 126 insertions(+), 90 deletions(-)
>> 
>> 
>> base-commit: 4a4b8721f1a5e4b01e45b3153c68d5a1014b25de
> 
> Any more comments on this patch series?

I think the general idea makes sense.  I'll review the ext4 patches in the series.


Cheers, Andreas






[-- Attachment #2: Message signed with OpenPGP --]
[-- Type: application/pgp-signature, Size: 873 bytes --]