summaryrefslogtreecommitdiff
path: root/fs/ext4
diff options
context:
space:
mode:
authorEric Biggers <ebiggers@google.com>2017-02-21 15:07:11 -0800
committerGreg Kroah-Hartman <gregkh@linuxfoundation.org>2017-03-31 09:49:54 +0200
commit7a5202190810dde1467718235c1f650fcf57592a (patch)
treeafbe017cb81f8b77aa7d8df5043075b6414671fc /fs/ext4
parent573341eba9c44b0b2198373cb453bbbb5b3f066a (diff)
fscrypt: remove broken support for detecting keyring key revocation
commit 1b53cf9815bb4744958d41f3795d5d5a1d365e2d upstream. 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") Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Theodore Ts'o <tytso@mit.edu> Acked-by: Michael Halcrow <mhalcrow@google.com> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Diffstat (limited to 'fs/ext4')
-rw-r--r--fs/ext4/crypto_key.c28
-rw-r--r--fs/ext4/ext4.h14
-rw-r--r--fs/ext4/ext4_crypto.h1
3 files changed, 8 insertions, 35 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];
};