[linux] 01/01: fscrypt: remove broken support for detecting keyring key revocation (CVE-2017-7374)
debian-kernel at lists.debian.org
debian-kernel at lists.debian.org
Sat Apr 8 07:41:33 UTC 2017
This is an automated email from the git hooks/post-receive script.
carnil pushed a commit to branch sid
in repository linux.
commit 5547db97a6bff7bdf24fa79fba93899e2f3c16ff
Author: Salvatore Bonaccorso <carnil at debian.org>
Date: Sat Apr 8 09:32:12 2017 +0200
fscrypt: remove broken support for detecting keyring key revocation (CVE-2017-7374)
---
debian/changelog | 2 +
...ove-broken-support-for-detecting-keyring-.patch | 253 +++++++++++++++++++++
debian/patches/series | 1 +
3 files changed, 256 insertions(+)
diff --git a/debian/changelog b/debian/changelog
index b223cf0..0473f95 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -13,6 +13,8 @@ linux (4.9.18-2) UNRELEASED; urgency=medium
[ Salvatore Bonaccorso ]
* ping: implement proper locking (CVE-2017-2671)
+ * fscrypt: remove broken support for detecting keyring key revocation
+ (CVE-2017-7374)
-- Ben Hutchings <ben at decadent.org.uk> Thu, 30 Mar 2017 18:27:30 +0100
diff --git a/debian/patches/bugfix/all/fscrypt-remove-broken-support-for-detecting-keyring-.patch b/debian/patches/bugfix/all/fscrypt-remove-broken-support-for-detecting-keyring-.patch
new file mode 100644
index 0000000..0e58294
--- /dev/null
+++ b/debian/patches/bugfix/all/fscrypt-remove-broken-support-for-detecting-keyring-.patch
@@ -0,0 +1,253 @@
+From: Eric Biggers <ebiggers at google.com>
+Date: Tue, 21 Feb 2017 15:07:11 -0800
+Subject: fscrypt: remove broken support for detecting keyring key revocation
+Origin: https://git.kernel.org/linus/1b53cf9815bb4744958d41f3795d5d5a1d365e2d
+Bug-Debian-Security: https://security-tracker.debian.org/tracker/CVE-2017-7374
+
+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 at google.com>
+Signed-off-by: Theodore Ts'o <tytso at mit.edu>
+Acked-by: Michael Halcrow <mhalcrow at google.com>
+Signed-off-by: Greg Kroah-Hartman <gregkh at linuxfoundation.org>
+[carnil: backport synced with 2984e52c75c657db7901f6189f02e0251ca963c2 in 4.9.20]
+---
+ 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.11.0
+
diff --git a/debian/patches/series b/debian/patches/series
index c10d9fd..4b8473c 100644
--- a/debian/patches/series
+++ b/debian/patches/series
@@ -131,6 +131,7 @@ bugfix/all/net-packet-fix-overflow-in-check-for-priv-area-size.patch
bugfix/all/net-packet-fix-overflow-in-check-for-tp_frame_nr.patch
bugfix/all/net-packet-fix-overflow-in-check-for-tp_reserve.patch
bugfix/all/ping-implement-proper-locking.patch
+bugfix/all/fscrypt-remove-broken-support-for-detecting-keyring-.patch
# Fix exported symbol versions
bugfix/ia64/revert-ia64-move-exports-to-definitions.patch
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/kernel/linux.git
More information about the Kernel-svn-changes
mailing list