[PATCH] fscrypt: remove broken support for detecting keyring key revocation

[PATCH] fscrypt: remove broken support for detecting keyring key revocation

[Eric Biggers]

commit 1b53cf9815bb4744958d41f3795d5d5a1d365e2d upstream.  Please apply
to 4.4-stable.

Filesystem encryption ostensibly supported revoking a keyring key that
had been used to "unlock" encrypted files, causing those files to become
"locked" again.  This was, however, buggy for several reasons, the most
severe of which was that when key revocation happened to be detected for
an inode, its fscrypt_info was immediately freed, even while other
threads could be using it for encryption or decryption concurrently.
This could be exploited to crash the kernel or worse.

This patch fixes the use-after-free by removing the code which detects
the keyring key having been revoked, invalidated, or expired.  Instead,
an encrypted inode that is "unlocked" now simply remains unlocked until
it is evicted from memory.  Note that this is no worse than the case for
block device-level encryption, e.g. dm-crypt, and it still remains
possible for a privileged user to evict unused pages, inodes, and
dentries by running 'sync; echo 3 > /proc/sys/vm/drop_caches', or by
simply unmounting the filesystem.  In fact, one of those actions was
already needed anyway for key revocation to work even somewhat sanely.
This change is not expected to break any applications.

In the future I'd like to implement a real API for fscrypt key
revocation that interacts sanely with ongoing filesystem operations ---
waiting for existing operations to complete and blocking new operations,
and invalidating and sanitizing key material and plaintext from the VFS
caches.  But this is a hard problem, and for now this bug must be fixed.

This bug affected almost all versions of ext4, f2fs, and ubifs
encryption, and it was potentially reachable in any kernel configured
with encryption support (CONFIG_EXT4_ENCRYPTION=y,
CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or
CONFIG_UBIFS_FS_ENCRYPTION=y).  Note that older kernels did not use the
shared fs/crypto/ code, but due to the potential security implications
of this bug, it may still be worthwhile to backport this fix to them.

Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode")
Cc: stable@vger.kernel.org # v4.2+
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Acked-by: Michael Halcrow <mhalcrow@google.com>
---
 fs/ext4/crypto_key.c  | 28 +++++++---------------------
 fs/ext4/ext4.h        | 14 +-------------
 fs/ext4/ext4_crypto.h |  1 -
 fs/f2fs/crypto_key.c  | 28 +++++++---------------------
 fs/f2fs/f2fs.h        | 14 +-------------
 fs/f2fs/f2fs_crypto.h |  1 -
 6 files changed, 16 insertions(+), 70 deletions(-)

diff --git a/fs/ext4/crypto_key.c b/fs/ext4/crypto_key.c
index 9a16d1e75a49..505f8afde57c 100644
--- a/fs/ext4/crypto_key.c
+++ b/fs/ext4/crypto_key.c
@@ -88,8 +88,6 @@ void ext4_free_crypt_info(struct ext4_crypt_info *ci)
 	if (!ci)
 		return;
 
-	if (ci->ci_keyring_key)
-		key_put(ci->ci_keyring_key);
 	crypto_free_ablkcipher(ci->ci_ctfm);
 	kmem_cache_free(ext4_crypt_info_cachep, ci);
 }
@@ -111,7 +109,7 @@ void ext4_free_encryption_info(struct inode *inode,
 	ext4_free_crypt_info(ci);
 }
 
-int _ext4_get_encryption_info(struct inode *inode)
+int ext4_get_encryption_info(struct inode *inode)
 {
 	struct ext4_inode_info *ei = EXT4_I(inode);
 	struct ext4_crypt_info *crypt_info;
@@ -128,22 +126,15 @@ int _ext4_get_encryption_info(struct inode *inode)
 	char mode;
 	int res;
 
+	if (ei->i_crypt_info)
+		return 0;
+
 	if (!ext4_read_workqueue) {
 		res = ext4_init_crypto();
 		if (res)
 			return res;
 	}
 
-retry:
-	crypt_info = ACCESS_ONCE(ei->i_crypt_info);
-	if (crypt_info) {
-		if (!crypt_info->ci_keyring_key ||
-		    key_validate(crypt_info->ci_keyring_key) == 0)
-			return 0;
-		ext4_free_encryption_info(inode, crypt_info);
-		goto retry;
-	}
-
 	res = ext4_xattr_get(inode, EXT4_XATTR_INDEX_ENCRYPTION,
 				 EXT4_XATTR_NAME_ENCRYPTION_CONTEXT,
 				 &ctx, sizeof(ctx));
@@ -166,7 +157,6 @@ retry:
 	crypt_info->ci_data_mode = ctx.contents_encryption_mode;
 	crypt_info->ci_filename_mode = ctx.filenames_encryption_mode;
 	crypt_info->ci_ctfm = NULL;
-	crypt_info->ci_keyring_key = NULL;
 	memcpy(crypt_info->ci_master_key, ctx.master_key_descriptor,
 	       sizeof(crypt_info->ci_master_key));
 	if (S_ISREG(inode->i_mode))
@@ -206,7 +196,6 @@ retry:
 		keyring_key = NULL;
 		goto out;
 	}
-	crypt_info->ci_keyring_key = keyring_key;
 	if (keyring_key->type != &key_type_logon) {
 		printk_once(KERN_WARNING
 			    "ext4: key type must be logon\n");
@@ -253,16 +242,13 @@ got_key:
 				       ext4_encryption_key_size(mode));
 	if (res)
 		goto out;
-	memzero_explicit(raw_key, sizeof(raw_key));
-	if (cmpxchg(&ei->i_crypt_info, NULL, crypt_info) != NULL) {
-		ext4_free_crypt_info(crypt_info);
-		goto retry;
-	}
-	return 0;
 
+	if (cmpxchg(&ei->i_crypt_info, NULL, crypt_info) == NULL)
+		crypt_info = NULL;
 out:
 	if (res == -ENOKEY)
 		res = 0;
+	key_put(keyring_key);
 	ext4_free_crypt_info(crypt_info);
 	memzero_explicit(raw_key, sizeof(raw_key));
 	return res;
diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index cd5914495ad7..362d59b24f1d 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2330,23 +2330,11 @@ static inline void ext4_fname_free_filename(struct ext4_filename *fname) { }
 /* crypto_key.c */
 void ext4_free_crypt_info(struct ext4_crypt_info *ci);
 void ext4_free_encryption_info(struct inode *inode, struct ext4_crypt_info *ci);
-int _ext4_get_encryption_info(struct inode *inode);
 
 #ifdef CONFIG_EXT4_FS_ENCRYPTION
 int ext4_has_encryption_key(struct inode *inode);
 
-static inline int ext4_get_encryption_info(struct inode *inode)
-{
-	struct ext4_crypt_info *ci = EXT4_I(inode)->i_crypt_info;
-
-	if (!ci ||
-	    (ci->ci_keyring_key &&
-	     (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) |
-					   (1 << KEY_FLAG_REVOKED) |
-					   (1 << KEY_FLAG_DEAD)))))
-		return _ext4_get_encryption_info(inode);
-	return 0;
-}
+int ext4_get_encryption_info(struct inode *inode);
 
 static inline struct ext4_crypt_info *ext4_encryption_info(struct inode *inode)
 {
diff --git a/fs/ext4/ext4_crypto.h b/fs/ext4/ext4_crypto.h
index ac7d4e813796..1b17b05b9f4d 100644
--- a/fs/ext4/ext4_crypto.h
+++ b/fs/ext4/ext4_crypto.h
@@ -78,7 +78,6 @@ struct ext4_crypt_info {
 	char		ci_filename_mode;
 	char		ci_flags;
 	struct crypto_ablkcipher *ci_ctfm;
-	struct key	*ci_keyring_key;
 	char		ci_master_key[EXT4_KEY_DESCRIPTOR_SIZE];
 };
 
diff --git a/fs/f2fs/crypto_key.c b/fs/f2fs/crypto_key.c
index 5de2d866a25c..18595d7a0efc 100644
--- a/fs/f2fs/crypto_key.c
+++ b/fs/f2fs/crypto_key.c
@@ -92,7 +92,6 @@ static void f2fs_free_crypt_info(struct f2fs_crypt_info *ci)
 	if (!ci)
 		return;
 
-	key_put(ci->ci_keyring_key);
 	crypto_free_ablkcipher(ci->ci_ctfm);
 	kmem_cache_free(f2fs_crypt_info_cachep, ci);
 }
@@ -113,7 +112,7 @@ void f2fs_free_encryption_info(struct inode *inode, struct f2fs_crypt_info *ci)
 	f2fs_free_crypt_info(ci);
 }
 
-int _f2fs_get_encryption_info(struct inode *inode)
+int f2fs_get_encryption_info(struct inode *inode)
 {
 	struct f2fs_inode_info *fi = F2FS_I(inode);
 	struct f2fs_crypt_info *crypt_info;
@@ -129,18 +128,12 @@ int _f2fs_get_encryption_info(struct inode *inode)
 	char mode;
 	int res;
 
+	if (fi->i_crypt_info)
+		return 0;
+
 	res = f2fs_crypto_initialize();
 	if (res)
 		return res;
-retry:
-	crypt_info = ACCESS_ONCE(fi->i_crypt_info);
-	if (crypt_info) {
-		if (!crypt_info->ci_keyring_key ||
-				key_validate(crypt_info->ci_keyring_key) == 0)
-			return 0;
-		f2fs_free_encryption_info(inode, crypt_info);
-		goto retry;
-	}
 
 	res = f2fs_getxattr(inode, F2FS_XATTR_INDEX_ENCRYPTION,
 				F2FS_XATTR_NAME_ENCRYPTION_CONTEXT,
@@ -159,7 +152,6 @@ retry:
 	crypt_info->ci_data_mode = ctx.contents_encryption_mode;
 	crypt_info->ci_filename_mode = ctx.filenames_encryption_mode;
 	crypt_info->ci_ctfm = NULL;
-	crypt_info->ci_keyring_key = NULL;
 	memcpy(crypt_info->ci_master_key, ctx.master_key_descriptor,
 				sizeof(crypt_info->ci_master_key));
 	if (S_ISREG(inode->i_mode))
@@ -197,7 +189,6 @@ retry:
 		keyring_key = NULL;
 		goto out;
 	}
-	crypt_info->ci_keyring_key = keyring_key;
 	BUG_ON(keyring_key->type != &key_type_logon);
 	ukp = user_key_payload(keyring_key);
 	if (ukp->datalen != sizeof(struct f2fs_encryption_key)) {
@@ -230,17 +221,12 @@ retry:
 	if (res)
 		goto out;
 
-	memzero_explicit(raw_key, sizeof(raw_key));
-	if (cmpxchg(&fi->i_crypt_info, NULL, crypt_info) != NULL) {
-		f2fs_free_crypt_info(crypt_info);
-		goto retry;
-	}
-	return 0;
-
+	if (cmpxchg(&fi->i_crypt_info, NULL, crypt_info) == NULL)
+		crypt_info = NULL;
 out:
 	if (res == -ENOKEY && !S_ISREG(inode->i_mode))
 		res = 0;
-
+	key_put(keyring_key);
 	f2fs_free_crypt_info(crypt_info);
 	memzero_explicit(raw_key, sizeof(raw_key));
 	return res;
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
index 9db5500d63d9..b1aeca83f4be 100644
--- a/fs/f2fs/f2fs.h
+++ b/fs/f2fs/f2fs.h
@@ -2149,7 +2149,6 @@ void f2fs_end_io_crypto_work(struct f2fs_crypto_ctx *, struct bio *);
 
 /* crypto_key.c */
 void f2fs_free_encryption_info(struct inode *, struct f2fs_crypt_info *);
-int _f2fs_get_encryption_info(struct inode *inode);
 
 /* crypto_fname.c */
 bool f2fs_valid_filenames_enc_mode(uint32_t);
@@ -2170,18 +2169,7 @@ void f2fs_exit_crypto(void);
 
 int f2fs_has_encryption_key(struct inode *);
 
-static inline int f2fs_get_encryption_info(struct inode *inode)
-{
-	struct f2fs_crypt_info *ci = F2FS_I(inode)->i_crypt_info;
-
-	if (!ci ||
-		(ci->ci_keyring_key &&
-		 (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) |
-					       (1 << KEY_FLAG_REVOKED) |
-					       (1 << KEY_FLAG_DEAD)))))
-		return _f2fs_get_encryption_info(inode);
-	return 0;
-}
+int f2fs_get_encryption_info(struct inode *inode);
 
 void f2fs_fname_crypto_free_buffer(struct f2fs_str *);
 int f2fs_fname_setup_filename(struct inode *, const struct qstr *,
diff --git a/fs/f2fs/f2fs_crypto.h b/fs/f2fs/f2fs_crypto.h
index c2c1c2b63b25..f113f1a1e8c1 100644
--- a/fs/f2fs/f2fs_crypto.h
+++ b/fs/f2fs/f2fs_crypto.h
@@ -79,7 +79,6 @@ struct f2fs_crypt_info {
 	char		ci_filename_mode;
 	char		ci_flags;
 	struct crypto_ablkcipher *ci_ctfm;
-	struct key	*ci_keyring_key;
 	char		ci_master_key[F2FS_KEY_DESCRIPTOR_SIZE];
 };
 
-- 
2.12.2.564.g063fe858b8-goog

Re: [PATCH] fscrypt: remove broken support for detecting keyring key revocation

[Greg Kroah-Hartman]

On Tue, Mar 28, 2017 at 01:59:21PM -0700, Eric Biggers wrote:
> commit 1b53cf9815bb4744958d41f3795d5d5a1d365e2d upstream.  Please apply
> to 4.4-stable.

Thanks for this, and the 4.9-stable backports, both now applied.

greg k-h

[PATCH] fscrypt: remove broken support for detecting keyring key revocation

[Eric Biggers]

commit 1b53cf9815bb4744958d41f3795d5d5a1d365e2d upstream.  Please apply
to 4.9-stable.

Filesystem encryption ostensibly supported revoking a keyring key that
had been used to "unlock" encrypted files, causing those files to become
"locked" again.  This was, however, buggy for several reasons, the most
severe of which was that when key revocation happened to be detected for
an inode, its fscrypt_info was immediately freed, even while other
threads could be using it for encryption or decryption concurrently.
This could be exploited to crash the kernel or worse.

This patch fixes the use-after-free by removing the code which detects
the keyring key having been revoked, invalidated, or expired.  Instead,
an encrypted inode that is "unlocked" now simply remains unlocked until
it is evicted from memory.  Note that this is no worse than the case for
block device-level encryption, e.g. dm-crypt, and it still remains
possible for a privileged user to evict unused pages, inodes, and
dentries by running 'sync; echo 3 > /proc/sys/vm/drop_caches', or by
simply unmounting the filesystem.  In fact, one of those actions was
already needed anyway for key revocation to work even somewhat sanely.
This change is not expected to break any applications.

In the future I'd like to implement a real API for fscrypt key
revocation that interacts sanely with ongoing filesystem operations ---
waiting for existing operations to complete and blocking new operations,
and invalidating and sanitizing key material and plaintext from the VFS
caches.  But this is a hard problem, and for now this bug must be fixed.

This bug affected almost all versions of ext4, f2fs, and ubifs
encryption, and it was potentially reachable in any kernel configured
with encryption support (CONFIG_EXT4_ENCRYPTION=y,
CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or
CONFIG_UBIFS_FS_ENCRYPTION=y).  Note that older kernels did not use the
shared fs/crypto/ code, but due to the potential security implications
of this bug, it may still be worthwhile to backport this fix to them.

Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode")
Cc: stable@vger.kernel.org # v4.2+
Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Acked-by: Michael Halcrow <mhalcrow@google.com>
---
 fs/crypto/crypto.c       | 10 +---------
 fs/crypto/fname.c        |  2 +-
 fs/crypto/keyinfo.c      | 52 +++++++++---------------------------------------
 include/linux/fscrypto.h |  2 --
 4 files changed, 11 insertions(+), 55 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 98f87fe8f186..61cfccea77bc 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -352,7 +352,6 @@ EXPORT_SYMBOL(fscrypt_zeroout_range);
 static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 {
 	struct dentry *dir;
-	struct fscrypt_info *ci;
 	int dir_has_key, cached_with_key;
 
 	if (flags & LOOKUP_RCU)
@@ -364,18 +363,11 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 		return 0;
 	}
 
-	ci = d_inode(dir)->i_crypt_info;
-	if (ci && ci->ci_keyring_key &&
-	    (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) |
-					  (1 << KEY_FLAG_REVOKED) |
-					  (1 << KEY_FLAG_DEAD))))
-		ci = NULL;
-
 	/* this should eventually be an flag in d_flags */
 	spin_lock(&dentry->d_lock);
 	cached_with_key = dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY;
 	spin_unlock(&dentry->d_lock);
-	dir_has_key = (ci != NULL);
+	dir_has_key = (d_inode(dir)->i_crypt_info != NULL);
 	dput(dir);
 
 	/*
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 9b774f4b50c8..80bb956e14e5 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -350,7 +350,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 		fname->disk_name.len = iname->len;
 		return 0;
 	}
-	ret = get_crypt_info(dir);
+	ret = fscrypt_get_encryption_info(dir);
 	if (ret && ret != -EOPNOTSUPP)
 		return ret;
 
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 67fb6d8876d0..bb4606368eb1 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -99,6 +99,7 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 	kfree(full_key_descriptor);
 	if (IS_ERR(keyring_key))
 		return PTR_ERR(keyring_key);
+	down_read(&keyring_key->sem);
 
 	if (keyring_key->type != &key_type_logon) {
 		printk_once(KERN_WARNING
@@ -106,11 +107,9 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 		res = -ENOKEY;
 		goto out;
 	}
-	down_read(&keyring_key->sem);
 	ukp = user_key_payload(keyring_key);
 	if (ukp->datalen != sizeof(struct fscrypt_key)) {
 		res = -EINVAL;
-		up_read(&keyring_key->sem);
 		goto out;
 	}
 	master_key = (struct fscrypt_key *)ukp->data;
@@ -121,17 +120,11 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 				"%s: key size incorrect: %d\n",
 				__func__, master_key->size);
 		res = -ENOKEY;
-		up_read(&keyring_key->sem);
 		goto out;
 	}
 	res = derive_key_aes(ctx->nonce, master_key->raw, raw_key);
-	up_read(&keyring_key->sem);
-	if (res)
-		goto out;
-
-	crypt_info->ci_keyring_key = keyring_key;
-	return 0;
 out:
+	up_read(&keyring_key->sem);
 	key_put(keyring_key);
 	return res;
 }
@@ -173,12 +166,11 @@ static void put_crypt_info(struct fscrypt_info *ci)
 	if (!ci)
 		return;
 
-	key_put(ci->ci_keyring_key);
 	crypto_free_skcipher(ci->ci_ctfm);
 	kmem_cache_free(fscrypt_info_cachep, ci);
 }
 
-int get_crypt_info(struct inode *inode)
+int fscrypt_get_encryption_info(struct inode *inode)
 {
 	struct fscrypt_info *crypt_info;
 	struct fscrypt_context ctx;
@@ -188,21 +180,15 @@ int get_crypt_info(struct inode *inode)
 	u8 *raw_key = NULL;
 	int res;
 
+	if (inode->i_crypt_info)
+		return 0;
+
 	res = fscrypt_initialize();
 	if (res)
 		return res;
 
 	if (!inode->i_sb->s_cop->get_context)
 		return -EOPNOTSUPP;
-retry:
-	crypt_info = ACCESS_ONCE(inode->i_crypt_info);
-	if (crypt_info) {
-		if (!crypt_info->ci_keyring_key ||
-				key_validate(crypt_info->ci_keyring_key) == 0)
-			return 0;
-		fscrypt_put_encryption_info(inode, crypt_info);
-		goto retry;
-	}
 
 	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
 	if (res < 0) {
@@ -230,7 +216,6 @@ int get_crypt_info(struct inode *inode)
 	crypt_info->ci_data_mode = ctx.contents_encryption_mode;
 	crypt_info->ci_filename_mode = ctx.filenames_encryption_mode;
 	crypt_info->ci_ctfm = NULL;
-	crypt_info->ci_keyring_key = NULL;
 	memcpy(crypt_info->ci_master_key, ctx.master_key_descriptor,
 				sizeof(crypt_info->ci_master_key));
 
@@ -285,14 +270,8 @@ int get_crypt_info(struct inode *inode)
 	if (res)
 		goto out;
 
-	kzfree(raw_key);
-	raw_key = NULL;
-	if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) != NULL) {
-		put_crypt_info(crypt_info);
-		goto retry;
-	}
-	return 0;
-
+	if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) == NULL)
+		crypt_info = NULL;
 out:
 	if (res == -ENOKEY)
 		res = 0;
@@ -300,6 +279,7 @@ int get_crypt_info(struct inode *inode)
 	kzfree(raw_key);
 	return res;
 }
+EXPORT_SYMBOL(fscrypt_get_encryption_info);
 
 void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci)
 {
@@ -317,17 +297,3 @@ void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci)
 	put_crypt_info(ci);
 }
 EXPORT_SYMBOL(fscrypt_put_encryption_info);
-
-int fscrypt_get_encryption_info(struct inode *inode)
-{
-	struct fscrypt_info *ci = inode->i_crypt_info;
-
-	if (!ci ||
-		(ci->ci_keyring_key &&
-		 (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) |
-					       (1 << KEY_FLAG_REVOKED) |
-					       (1 << KEY_FLAG_DEAD)))))
-		return get_crypt_info(inode);
-	return 0;
-}
-EXPORT_SYMBOL(fscrypt_get_encryption_info);
diff --git a/include/linux/fscrypto.h b/include/linux/fscrypto.h
index ff8b11b26f31..f6dfc2950f76 100644
--- a/include/linux/fscrypto.h
+++ b/include/linux/fscrypto.h
@@ -79,7 +79,6 @@ struct fscrypt_info {
 	u8 ci_filename_mode;
 	u8 ci_flags;
 	struct crypto_skcipher *ci_ctfm;
-	struct key *ci_keyring_key;
 	u8 ci_master_key[FS_KEY_DESCRIPTOR_SIZE];
 };
 
@@ -256,7 +255,6 @@ extern int fscrypt_has_permitted_context(struct inode *, struct inode *);
 extern int fscrypt_inherit_context(struct inode *, struct inode *,
 					void *, bool);
 /* keyinfo.c */
-extern int get_crypt_info(struct inode *);
 extern int fscrypt_get_encryption_info(struct inode *);
 extern void fscrypt_put_encryption_info(struct inode *, struct fscrypt_info *);
 
-- 
2.12.2.564.g063fe858b8-goog

Re: [PATCH] fscrypt: remove broken support for detecting keyring key revocation

[Theodore Ts'o]

On Tue, Feb 21, 2017 at 03:07:11PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Filesystem encryption ostensibly supported revoking a keyring key that
> had been used to "unlock" encrypted files, causing those files to become
> "locked" again.  This was, however, buggy for several reasons, the most
> severe of which was that when key revocation happened to be detected for
> an inode, its fscrypt_info was immediately freed, even while other
> threads could be using it for encryption or decryption concurrently.
> This could be exploited to crash the kernel or worse.
> 
> This patch fixes the use-after-free by removing the code which detects
> the keyring key having been revoked, invalidated, or expired.  Instead,
> an encrypted inode that is "unlocked" now simply remains unlocked until
> it is evicted from memory.  Note that this is no worse than the case for
> block device-level encryption, e.g. dm-crypt, and it still remains
> possible for a privileged user to evict unused pages, inodes, and
> dentries by running 'sync; echo 3 > /proc/sys/vm/drop_caches', or by
> simply unmounting the filesystem.  In fact, one of those actions was
> already needed anyway for key revocation to work even somewhat sanely.
> This change is not expected to break any applications.
> 
> In the future I'd like to implement a real API for fscrypt key
> revocation that interacts sanely with ongoing filesystem operations ---
> waiting for existing operations to complete and blocking new operations,
> and invalidating and sanitizing key material and plaintext from the VFS
> caches.  But this is a hard problem, and for now this bug must be fixed.
> 
> This bug affected almost all versions of ext4, f2fs, and ubifs
> encryption, and it was potentially reachable in any kernel configured
> with encryption support (CONFIG_EXT4_ENCRYPTION=y,
> CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or
> CONFIG_UBIFS_FS_ENCRYPTION=y).  Note that older kernels did not use the
> shared fs/crypto/ code, but due to the potential security implications
> of this bug, it may still be worthwhile to backport this fix to them.
> 
> Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode")
> Cc: stable@vger.kernel.org # v4.2+
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks, applied.

						- Ted

Re: [PATCH] fscrypt: remove broken support for detecting keyring key revocation

[Theodore Ts'o]

On Tue, Feb 21, 2017 at 03:07:11PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Filesystem encryption ostensibly supported revoking a keyring key that
> had been used to "unlock" encrypted files, causing those files to become
> "locked" again.  This was, however, buggy for several reasons, the most
> severe of which was that when key revocation happened to be detected for
> an inode, its fscrypt_info was immediately freed, even while other
> threads could be using it for encryption or decryption concurrently.
> This could be exploited to crash the kernel or worse.
> 
> This patch fixes the use-after-free by removing the code which detects
> the keyring key having been revoked, invalidated, or expired.  Instead,
> an encrypted inode that is "unlocked" now simply remains unlocked until
> it is evicted from memory.  Note that this is no worse than the case for
> block device-level encryption, e.g. dm-crypt, and it still remains
> possible for a privileged user to evict unused pages, inodes, and
> dentries by running 'sync; echo 3 > /proc/sys/vm/drop_caches', or by
> simply unmounting the filesystem.  In fact, one of those actions was
> already needed anyway for key revocation to work even somewhat sanely.
> This change is not expected to break any applications.
> 
> In the future I'd like to implement a real API for fscrypt key
> revocation that interacts sanely with ongoing filesystem operations ---
> waiting for existing operations to complete and blocking new operations,
> and invalidating and sanitizing key material and plaintext from the VFS
> caches.  But this is a hard problem, and for now this bug must be fixed.
> 
> This bug affected almost all versions of ext4, f2fs, and ubifs
> encryption, and it was potentially reachable in any kernel configured
> with encryption support (CONFIG_EXT4_ENCRYPTION=y,
> CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or
> CONFIG_UBIFS_FS_ENCRYPTION=y).  Note that older kernels did not use the
> shared fs/crypto/ code, but due to the potential security implications
> of this bug, it may still be worthwhile to backport this fix to them.
> 
> Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode")
> Cc: stable@vger.kernel.org # v4.2+
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Thanks, applied.

						- Ted

------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot

Re: [PATCH] fscrypt: remove broken support for detecting keyring key revocation

[Eric Biggers]

On Tue, Feb 21, 2017 at 03:07:11PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Filesystem encryption ostensibly supported revoking a keyring key that
> had been used to "unlock" encrypted files, causing those files to become
> "locked" again.  This was, however, buggy for several reasons, the most
> severe of which was that when key revocation happened to be detected for
> an inode, its fscrypt_info was immediately freed, even while other
> threads could be using it for encryption or decryption concurrently.
> This could be exploited to crash the kernel or worse.
> 
> This patch fixes the use-after-free by removing the code which detects
> the keyring key having been revoked, invalidated, or expired.  Instead,
> an encrypted inode that is "unlocked" now simply remains unlocked until
> it is evicted from memory.  Note that this is no worse than the case for
> block device-level encryption, e.g. dm-crypt, and it still remains
> possible for a privileged user to evict unused pages, inodes, and
> dentries by running 'sync; echo 3 > /proc/sys/vm/drop_caches', or by
> simply unmounting the filesystem.  In fact, one of those actions was
> already needed anyway for key revocation to work even somewhat sanely.
> This change is not expected to break any applications.
> 
> In the future I'd like to implement a real API for fscrypt key
> revocation that interacts sanely with ongoing filesystem operations ---
> waiting for existing operations to complete and blocking new operations,
> and invalidating and sanitizing key material and plaintext from the VFS
> caches.  But this is a hard problem, and for now this bug must be fixed.
> 
> This bug affected almost all versions of ext4, f2fs, and ubifs
> encryption, and it was potentially reachable in any kernel configured
> with encryption support (CONFIG_EXT4_ENCRYPTION=y,
> CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or
> CONFIG_UBIFS_FS_ENCRYPTION=y).  Note that older kernels did not use the
> shared fs/crypto/ code, but due to the potential security implications
> of this bug, it may still be worthwhile to backport this fix to them.
> 
> Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode")
> Cc: stable@vger.kernel.org # v4.2+
> Signed-off-by: Eric Biggers <ebiggers@google.com>


Ted, can this be sent to Linus soon?  This needs to be fixed as it's a security
vulnerability on some systems.

Eric

Re: [PATCH] fscrypt: remove broken support for detecting keyring key revocation

[Michael Halcrow]

On Tue, Feb 21, 2017 at 03:07:11PM -0800, Eric Biggers wrote:
> From: Eric Biggers <ebiggers@google.com>
> 
> Filesystem encryption ostensibly supported revoking a keyring key
> that had been used to "unlock" encrypted files, causing those files
> to become "locked" again. This was, however, buggy for several
> reasons, the most severe of which was that when key revocation
> happened to be detected for an inode, its fscrypt_info was
> immediately freed, even while other threads could be using it for
> encryption or decryption concurrently.  This could be exploited to
> crash the kernel or worse.

Removing the attempt at that functionality seems like the right
approach.

> This patch fixes the use-after-free by removing the code which
> detects the keyring key having been revoked, invalidated, or
> expired.  Instead, an encrypted inode that is "unlocked" now simply
> remains unlocked until it is evicted from memory.  Note that this is
> no worse than the case for block device-level encryption,
> e.g. dm-crypt, and it still remains possible for a privileged user
> to evict unused pages, inodes, and dentries by running 'sync; echo 3
> > /proc/sys/vm/drop_caches', or by simply unmounting the filesystem.
> In fact, one of those actions was already needed anyway for key
> revocation to work even somewhat sanely.  This change is not
> expected to break any applications.

I don't see any problem with this reasoning.

> In the future I'd like to implement a real API for fscrypt key
> revocation that interacts sanely with ongoing filesystem operations ---
> waiting for existing operations to complete and blocking new operations,
> and invalidating and sanitizing key material and plaintext from the VFS
> caches.  But this is a hard problem, and for now this bug must be fixed.
> 
> This bug affected almost all versions of ext4, f2fs, and ubifs
> encryption, and it was potentially reachable in any kernel configured
> with encryption support (CONFIG_EXT4_ENCRYPTION=y,
> CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or
> CONFIG_UBIFS_FS_ENCRYPTION=y).  Note that older kernels did not use the
> shared fs/crypto/ code, but due to the potential security implications
> of this bug, it may still be worthwhile to backport this fix to them.

Agreed.

> Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode")
> Cc: stable@vger.kernel.org # v4.2+
> Signed-off-by: Eric Biggers <ebiggers@google.com>

Acked-by: Michael Halcrow <mhalcrow@google.com>

> ---
>  fs/crypto/crypto.c          | 10 +--------
>  fs/crypto/fname.c           |  2 +-
>  fs/crypto/fscrypt_private.h |  4 ----
>  fs/crypto/keyinfo.c         | 52 ++++++++-------------------------------------
>  4 files changed, 11 insertions(+), 57 deletions(-)
> 
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index 02a7a9286449..6d6eca394d4d 100644
> --- a/fs/crypto/crypto.c
> +++ b/fs/crypto/crypto.c
> @@ -327,7 +327,6 @@ EXPORT_SYMBOL(fscrypt_decrypt_page);
>  static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
>  {
>  	struct dentry *dir;
> -	struct fscrypt_info *ci;
>  	int dir_has_key, cached_with_key;
>  
>  	if (flags & LOOKUP_RCU)
> @@ -339,18 +338,11 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
>  		return 0;
>  	}
>  
> -	ci = d_inode(dir)->i_crypt_info;
> -	if (ci && ci->ci_keyring_key &&
> -	    (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) |
> -					  (1 << KEY_FLAG_REVOKED) |
> -					  (1 << KEY_FLAG_DEAD))))
> -		ci = NULL;
> -
>  	/* this should eventually be an flag in d_flags */
>  	spin_lock(&dentry->d_lock);
>  	cached_with_key = dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY;
>  	spin_unlock(&dentry->d_lock);
> -	dir_has_key = (ci != NULL);
> +	dir_has_key = (d_inode(dir)->i_crypt_info != NULL);
>  	dput(dir);
>  
>  	/*
> diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
> index 13052b85c393..37b49894c762 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -350,7 +350,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
>  		fname->disk_name.len = iname->len;
>  		return 0;
>  	}
> -	ret = fscrypt_get_crypt_info(dir);
> +	ret = fscrypt_get_encryption_info(dir);
>  	if (ret && ret != -EOPNOTSUPP)
>  		return ret;
>  
> diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
> index fdbb8af32eaf..e39696e64494 100644
> --- a/fs/crypto/fscrypt_private.h
> +++ b/fs/crypto/fscrypt_private.h
> @@ -67,7 +67,6 @@ struct fscrypt_info {
>  	u8 ci_filename_mode;
>  	u8 ci_flags;
>  	struct crypto_skcipher *ci_ctfm;
> -	struct key *ci_keyring_key;
>  	u8 ci_master_key[FS_KEY_DESCRIPTOR_SIZE];
>  };
>  
> @@ -101,7 +100,4 @@ extern int fscrypt_do_page_crypto(const struct inode *inode,
>  extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
>  					      gfp_t gfp_flags);
>  
> -/* keyinfo.c */
> -extern int fscrypt_get_crypt_info(struct inode *);
> -
>  #endif /* _FSCRYPT_PRIVATE_H */
> diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
> index 02eb6b9e4438..cb3e82abf034 100644
> --- a/fs/crypto/keyinfo.c
> +++ b/fs/crypto/keyinfo.c
> @@ -95,6 +95,7 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
>  	kfree(description);
>  	if (IS_ERR(keyring_key))
>  		return PTR_ERR(keyring_key);
> +	down_read(&keyring_key->sem);
>  
>  	if (keyring_key->type != &key_type_logon) {
>  		printk_once(KERN_WARNING
> @@ -102,11 +103,9 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
>  		res = -ENOKEY;
>  		goto out;
>  	}
> -	down_read(&keyring_key->sem);
>  	ukp = user_key_payload(keyring_key);
>  	if (ukp->datalen != sizeof(struct fscrypt_key)) {
>  		res = -EINVAL;
> -		up_read(&keyring_key->sem);
>  		goto out;
>  	}
>  	master_key = (struct fscrypt_key *)ukp->data;
> @@ -117,17 +116,11 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
>  				"%s: key size incorrect: %d\n",
>  				__func__, master_key->size);
>  		res = -ENOKEY;
> -		up_read(&keyring_key->sem);
>  		goto out;
>  	}
>  	res = derive_key_aes(ctx->nonce, master_key->raw, raw_key);
> -	up_read(&keyring_key->sem);
> -	if (res)
> -		goto out;
> -
> -	crypt_info->ci_keyring_key = keyring_key;
> -	return 0;
>  out:
> +	up_read(&keyring_key->sem);
>  	key_put(keyring_key);
>  	return res;
>  }
> @@ -169,12 +162,11 @@ static void put_crypt_info(struct fscrypt_info *ci)
>  	if (!ci)
>  		return;
>  
> -	key_put(ci->ci_keyring_key);
>  	crypto_free_skcipher(ci->ci_ctfm);
>  	kmem_cache_free(fscrypt_info_cachep, ci);
>  }
>  
> -int fscrypt_get_crypt_info(struct inode *inode)
> +int fscrypt_get_encryption_info(struct inode *inode)
>  {
>  	struct fscrypt_info *crypt_info;
>  	struct fscrypt_context ctx;
> @@ -184,21 +176,15 @@ int fscrypt_get_crypt_info(struct inode *inode)
>  	u8 *raw_key = NULL;
>  	int res;
>  
> +	if (inode->i_crypt_info)
> +		return 0;
> +
>  	res = fscrypt_initialize(inode->i_sb->s_cop->flags);
>  	if (res)
>  		return res;
>  
>  	if (!inode->i_sb->s_cop->get_context)
>  		return -EOPNOTSUPP;
> -retry:
> -	crypt_info = ACCESS_ONCE(inode->i_crypt_info);
> -	if (crypt_info) {
> -		if (!crypt_info->ci_keyring_key ||
> -				key_validate(crypt_info->ci_keyring_key) == 0)
> -			return 0;
> -		fscrypt_put_encryption_info(inode, crypt_info);
> -		goto retry;
> -	}
>  
>  	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
>  	if (res < 0) {
> @@ -229,7 +215,6 @@ int fscrypt_get_crypt_info(struct inode *inode)
>  	crypt_info->ci_data_mode = ctx.contents_encryption_mode;
>  	crypt_info->ci_filename_mode = ctx.filenames_encryption_mode;
>  	crypt_info->ci_ctfm = NULL;
> -	crypt_info->ci_keyring_key = NULL;
>  	memcpy(crypt_info->ci_master_key, ctx.master_key_descriptor,
>  				sizeof(crypt_info->ci_master_key));
>  
> @@ -273,14 +258,8 @@ int fscrypt_get_crypt_info(struct inode *inode)
>  	if (res)
>  		goto out;
>  
> -	kzfree(raw_key);
> -	raw_key = NULL;
> -	if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) != NULL) {
> -		put_crypt_info(crypt_info);
> -		goto retry;
> -	}
> -	return 0;
> -
> +	if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) == NULL)
> +		crypt_info = NULL;
>  out:
>  	if (res == -ENOKEY)
>  		res = 0;
> @@ -288,6 +267,7 @@ int fscrypt_get_crypt_info(struct inode *inode)
>  	kzfree(raw_key);
>  	return res;
>  }
> +EXPORT_SYMBOL(fscrypt_get_encryption_info);
>  
>  void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci)
>  {
> @@ -305,17 +285,3 @@ void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci)
>  	put_crypt_info(ci);
>  }
>  EXPORT_SYMBOL(fscrypt_put_encryption_info);
> -
> -int fscrypt_get_encryption_info(struct inode *inode)
> -{
> -	struct fscrypt_info *ci = inode->i_crypt_info;
> -
> -	if (!ci ||
> -		(ci->ci_keyring_key &&
> -		 (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) |
> -					       (1 << KEY_FLAG_REVOKED) |
> -					       (1 << KEY_FLAG_DEAD)))))
> -		return fscrypt_get_crypt_info(inode);
> -	return 0;
> -}
> -EXPORT_SYMBOL(fscrypt_get_encryption_info);
> -- 
> 2.11.0.483.g087da7b7c-goog
> 

[PATCH] fscrypt: remove broken support for detecting keyring key revocation

[Eric Biggers]

From: Eric Biggers <ebiggers@google.com>

Filesystem encryption ostensibly supported revoking a keyring key that
had been used to "unlock" encrypted files, causing those files to become
"locked" again.  This was, however, buggy for several reasons, the most
severe of which was that when key revocation happened to be detected for
an inode, its fscrypt_info was immediately freed, even while other
threads could be using it for encryption or decryption concurrently.
This could be exploited to crash the kernel or worse.

This patch fixes the use-after-free by removing the code which detects
the keyring key having been revoked, invalidated, or expired.  Instead,
an encrypted inode that is "unlocked" now simply remains unlocked until
it is evicted from memory.  Note that this is no worse than the case for
block device-level encryption, e.g. dm-crypt, and it still remains
possible for a privileged user to evict unused pages, inodes, and
dentries by running 'sync; echo 3 > /proc/sys/vm/drop_caches', or by
simply unmounting the filesystem.  In fact, one of those actions was
already needed anyway for key revocation to work even somewhat sanely.
This change is not expected to break any applications.

In the future I'd like to implement a real API for fscrypt key
revocation that interacts sanely with ongoing filesystem operations ---
waiting for existing operations to complete and blocking new operations,
and invalidating and sanitizing key material and plaintext from the VFS
caches.  But this is a hard problem, and for now this bug must be fixed.

This bug affected almost all versions of ext4, f2fs, and ubifs
encryption, and it was potentially reachable in any kernel configured
with encryption support (CONFIG_EXT4_ENCRYPTION=y,
CONFIG_EXT4_FS_ENCRYPTION=y, CONFIG_F2FS_FS_ENCRYPTION=y, or
CONFIG_UBIFS_FS_ENCRYPTION=y).  Note that older kernels did not use the
shared fs/crypto/ code, but due to the potential security implications
of this bug, it may still be worthwhile to backport this fix to them.

Fixes: b7236e21d55f ("ext4 crypto: reorganize how we store keys in the inode")
Cc: stable@vger.kernel.org # v4.2+
Signed-off-by: Eric Biggers <ebiggers@google.com>
---
 fs/crypto/crypto.c          | 10 +--------
 fs/crypto/fname.c           |  2 +-
 fs/crypto/fscrypt_private.h |  4 ----
 fs/crypto/keyinfo.c         | 52 ++++++++-------------------------------------
 4 files changed, 11 insertions(+), 57 deletions(-)

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 02a7a9286449..6d6eca394d4d 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -327,7 +327,6 @@ EXPORT_SYMBOL(fscrypt_decrypt_page);
 static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 {
 	struct dentry *dir;
-	struct fscrypt_info *ci;
 	int dir_has_key, cached_with_key;
 
 	if (flags & LOOKUP_RCU)
@@ -339,18 +338,11 @@ static int fscrypt_d_revalidate(struct dentry *dentry, unsigned int flags)
 		return 0;
 	}
 
-	ci = d_inode(dir)->i_crypt_info;
-	if (ci && ci->ci_keyring_key &&
-	    (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) |
-					  (1 << KEY_FLAG_REVOKED) |
-					  (1 << KEY_FLAG_DEAD))))
-		ci = NULL;
-
 	/* this should eventually be an flag in d_flags */
 	spin_lock(&dentry->d_lock);
 	cached_with_key = dentry->d_flags & DCACHE_ENCRYPTED_WITH_KEY;
 	spin_unlock(&dentry->d_lock);
-	dir_has_key = (ci != NULL);
+	dir_has_key = (d_inode(dir)->i_crypt_info != NULL);
 	dput(dir);
 
 	/*
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 13052b85c393..37b49894c762 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -350,7 +350,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
 		fname->disk_name.len = iname->len;
 		return 0;
 	}
-	ret = fscrypt_get_crypt_info(dir);
+	ret = fscrypt_get_encryption_info(dir);
 	if (ret && ret != -EOPNOTSUPP)
 		return ret;
 
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index fdbb8af32eaf..e39696e64494 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -67,7 +67,6 @@ struct fscrypt_info {
 	u8 ci_filename_mode;
 	u8 ci_flags;
 	struct crypto_skcipher *ci_ctfm;
-	struct key *ci_keyring_key;
 	u8 ci_master_key[FS_KEY_DESCRIPTOR_SIZE];
 };
 
@@ -101,7 +100,4 @@ extern int fscrypt_do_page_crypto(const struct inode *inode,
 extern struct page *fscrypt_alloc_bounce_page(struct fscrypt_ctx *ctx,
 					      gfp_t gfp_flags);
 
-/* keyinfo.c */
-extern int fscrypt_get_crypt_info(struct inode *);
-
 #endif /* _FSCRYPT_PRIVATE_H */
diff --git a/fs/crypto/keyinfo.c b/fs/crypto/keyinfo.c
index 02eb6b9e4438..cb3e82abf034 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -95,6 +95,7 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 	kfree(description);
 	if (IS_ERR(keyring_key))
 		return PTR_ERR(keyring_key);
+	down_read(&keyring_key->sem);
 
 	if (keyring_key->type != &key_type_logon) {
 		printk_once(KERN_WARNING
@@ -102,11 +103,9 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 		res = -ENOKEY;
 		goto out;
 	}
-	down_read(&keyring_key->sem);
 	ukp = user_key_payload(keyring_key);
 	if (ukp->datalen != sizeof(struct fscrypt_key)) {
 		res = -EINVAL;
-		up_read(&keyring_key->sem);
 		goto out;
 	}
 	master_key = (struct fscrypt_key *)ukp->data;
@@ -117,17 +116,11 @@ static int validate_user_key(struct fscrypt_info *crypt_info,
 				"%s: key size incorrect: %d\n",
 				__func__, master_key->size);
 		res = -ENOKEY;
-		up_read(&keyring_key->sem);
 		goto out;
 	}
 	res = derive_key_aes(ctx->nonce, master_key->raw, raw_key);
-	up_read(&keyring_key->sem);
-	if (res)
-		goto out;
-
-	crypt_info->ci_keyring_key = keyring_key;
-	return 0;
 out:
+	up_read(&keyring_key->sem);
 	key_put(keyring_key);
 	return res;
 }
@@ -169,12 +162,11 @@ static void put_crypt_info(struct fscrypt_info *ci)
 	if (!ci)
 		return;
 
-	key_put(ci->ci_keyring_key);
 	crypto_free_skcipher(ci->ci_ctfm);
 	kmem_cache_free(fscrypt_info_cachep, ci);
 }
 
-int fscrypt_get_crypt_info(struct inode *inode)
+int fscrypt_get_encryption_info(struct inode *inode)
 {
 	struct fscrypt_info *crypt_info;
 	struct fscrypt_context ctx;
@@ -184,21 +176,15 @@ int fscrypt_get_crypt_info(struct inode *inode)
 	u8 *raw_key = NULL;
 	int res;
 
+	if (inode->i_crypt_info)
+		return 0;
+
 	res = fscrypt_initialize(inode->i_sb->s_cop->flags);
 	if (res)
 		return res;
 
 	if (!inode->i_sb->s_cop->get_context)
 		return -EOPNOTSUPP;
-retry:
-	crypt_info = ACCESS_ONCE(inode->i_crypt_info);
-	if (crypt_info) {
-		if (!crypt_info->ci_keyring_key ||
-				key_validate(crypt_info->ci_keyring_key) == 0)
-			return 0;
-		fscrypt_put_encryption_info(inode, crypt_info);
-		goto retry;
-	}
 
 	res = inode->i_sb->s_cop->get_context(inode, &ctx, sizeof(ctx));
 	if (res < 0) {
@@ -229,7 +215,6 @@ int fscrypt_get_crypt_info(struct inode *inode)
 	crypt_info->ci_data_mode = ctx.contents_encryption_mode;
 	crypt_info->ci_filename_mode = ctx.filenames_encryption_mode;
 	crypt_info->ci_ctfm = NULL;
-	crypt_info->ci_keyring_key = NULL;
 	memcpy(crypt_info->ci_master_key, ctx.master_key_descriptor,
 				sizeof(crypt_info->ci_master_key));
 
@@ -273,14 +258,8 @@ int fscrypt_get_crypt_info(struct inode *inode)
 	if (res)
 		goto out;
 
-	kzfree(raw_key);
-	raw_key = NULL;
-	if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) != NULL) {
-		put_crypt_info(crypt_info);
-		goto retry;
-	}
-	return 0;
-
+	if (cmpxchg(&inode->i_crypt_info, NULL, crypt_info) == NULL)
+		crypt_info = NULL;
 out:
 	if (res == -ENOKEY)
 		res = 0;
@@ -288,6 +267,7 @@ int fscrypt_get_crypt_info(struct inode *inode)
 	kzfree(raw_key);
 	return res;
 }
+EXPORT_SYMBOL(fscrypt_get_encryption_info);
 
 void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci)
 {
@@ -305,17 +285,3 @@ void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci)
 	put_crypt_info(ci);
 }
 EXPORT_SYMBOL(fscrypt_put_encryption_info);
-
-int fscrypt_get_encryption_info(struct inode *inode)
-{
-	struct fscrypt_info *ci = inode->i_crypt_info;
-
-	if (!ci ||
-		(ci->ci_keyring_key &&
-		 (ci->ci_keyring_key->flags & ((1 << KEY_FLAG_INVALIDATED) |
-					       (1 << KEY_FLAG_REVOKED) |
-					       (1 << KEY_FLAG_DEAD)))))
-		return fscrypt_get_crypt_info(inode);
-	return 0;
-}
-EXPORT_SYMBOL(fscrypt_get_encryption_info);
-- 
2.11.0.483.g087da7b7c-goog