diff mbox

[Yakkety,SRU] Backport fix for CVE-2017-7374

Message ID 1496751755-21184-1-git-send-email-stefan.bader@canonical.com
State New
Headers show

Commit Message

Stefan Bader June 6, 2017, 12:22 p.m. UTC 
Xenial and Zesty already are fixed. But for Yakkety the patch
needed some adjustments. This backport was test-compiled for
amd64/yakkety. Of course some more eyeyballing never hurts.

-Stefan


---

From 1a87bc35c4392db1efa5bbabf99c341ed9962e00 Mon Sep 17 00:00:00 2001
From: Eric Biggers <ebiggers@google.com>
Date: Tue, 21 Feb 2017 15:07:11 -0800
Subject: [PATCH] fscrypt: remove broken support for detecting keyring key
 revocation

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>

CVE-2017-7374

(backportded from 1b53cf9815bb4744958d41f3795d5d5a1d365e2d)

[function fscrypt_get_crypt_info was still named get_crypt_info,
 private header does not yet exist]

Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
---
 fs/crypto/crypto.c       | 10 +---------
 fs/crypto/fname.c        |  2 +-
 fs/crypto/keyinfo.c      | 50 +++++++++---------------------------------------
 include/linux/fscrypto.h |  2 --
 4 files changed, 11 insertions(+), 53 deletions(-)

Comments

Colin Ian King June 7, 2017, 8:52 a.m. UTC | #1 
On 06/06/17 13:22, Stefan Bader wrote:
> Xenial and Zesty already are fixed. But for Yakkety the patch
> needed some adjustments. This backport was test-compiled for
> amd64/yakkety. Of course some more eyeyballing never hurts.
> 
> -Stefan
> 
> 
> ---
> 
> From 1a87bc35c4392db1efa5bbabf99c341ed9962e00 Mon Sep 17 00:00:00 2001
> From: Eric Biggers <ebiggers@google.com>
> Date: Tue, 21 Feb 2017 15:07:11 -0800
> Subject: [PATCH] fscrypt: remove broken support for detecting keyring key
>  revocation
> 
> 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>
> 
> CVE-2017-7374
> 
> (backportded from 1b53cf9815bb4744958d41f3795d5d5a1d365e2d)
> 
> [function fscrypt_get_crypt_info was still named get_crypt_info,
>  private header does not yet exist]
> 
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  fs/crypto/crypto.c       | 10 +---------
>  fs/crypto/fname.c        |  2 +-
>  fs/crypto/keyinfo.c      | 50 +++++++++---------------------------------------
>  include/linux/fscrypto.h |  2 --
>  4 files changed, 11 insertions(+), 53 deletions(-)
> 
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index 55d64fb..624807c 100644
> --- a/fs/crypto/crypto.c
> +++ b/fs/crypto/crypto.c
> @@ -353,7 +353,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)
> @@ -365,18 +364,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 5d6d491..8dcbfee 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -360,7 +360,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 1ac263e..8ec4371 100644
> --- a/fs/crypto/keyinfo.c
> +++ b/fs/crypto/keyinfo.c
> @@ -102,6 +102,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
> @@ -109,11 +110,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;
> @@ -124,17 +123,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;
>  }
> @@ -144,12 +137,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;
> @@ -159,21 +151,15 @@ int get_crypt_info(struct inode *inode)
>  	u8 mode;
>  	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) {
> @@ -195,7 +181,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))
> @@ -257,12 +242,8 @@ got_key:
>  	if (res)
>  		goto out;
>  
> -	memzero_explicit(raw_key, sizeof(raw_key));
> -	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)
> @@ -271,6 +252,7 @@ out:
>  	memzero_explicit(raw_key, sizeof(raw_key));
>  	return res;
>  }
> +EXPORT_SYMBOL(fscrypt_get_encryption_info);
>  
>  void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci)
>  {
> @@ -288,17 +270,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 76cff18..8e0bc584 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];
>  };
>  
> @@ -280,7 +279,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 *);
>  
> 

Looks OK to me, builds OK, not tested it though.

Acked-by: Colin Ian King <colin.king@canonical.com>
Andy Whitcroft June 8, 2017, 9:23 a.m. UTC | #2 
On Tue, Jun 06, 2017 at 02:22:35PM +0200, Stefan Bader wrote:
> Xenial and Zesty already are fixed. But for Yakkety the patch
> needed some adjustments. This backport was test-compiled for
> amd64/yakkety. Of course some more eyeyballing never hurts.
> 
> -Stefan
> 
> 
> ---
> 
> From 1a87bc35c4392db1efa5bbabf99c341ed9962e00 Mon Sep 17 00:00:00 2001
> From: Eric Biggers <ebiggers@google.com>
> Date: Tue, 21 Feb 2017 15:07:11 -0800
> Subject: [PATCH] fscrypt: remove broken support for detecting keyring key
>  revocation
> 
> 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>
> 
> CVE-2017-7374
> 
> (backportded from 1b53cf9815bb4744958d41f3795d5d5a1d365e2d)

^ spelling matters in this line ... pls fix on application.

> [function fscrypt_get_crypt_info was still named get_crypt_info,
>  private header does not yet exist]
> 
> Signed-off-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  fs/crypto/crypto.c       | 10 +---------
>  fs/crypto/fname.c        |  2 +-
>  fs/crypto/keyinfo.c      | 50 +++++++++---------------------------------------
>  include/linux/fscrypto.h |  2 --
>  4 files changed, 11 insertions(+), 53 deletions(-)
> 
> diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
> index 55d64fb..624807c 100644
> --- a/fs/crypto/crypto.c
> +++ b/fs/crypto/crypto.c
> @@ -353,7 +353,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)
> @@ -365,18 +364,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 5d6d491..8dcbfee 100644
> --- a/fs/crypto/fname.c
> +++ b/fs/crypto/fname.c
> @@ -360,7 +360,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 1ac263e..8ec4371 100644
> --- a/fs/crypto/keyinfo.c
> +++ b/fs/crypto/keyinfo.c
> @@ -102,6 +102,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
> @@ -109,11 +110,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;
> @@ -124,17 +123,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;
>  }
> @@ -144,12 +137,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;
> @@ -159,21 +151,15 @@ int get_crypt_info(struct inode *inode)
>  	u8 mode;
>  	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) {
> @@ -195,7 +181,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))
> @@ -257,12 +242,8 @@ got_key:
>  	if (res)
>  		goto out;
>  
> -	memzero_explicit(raw_key, sizeof(raw_key));
> -	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)
> @@ -271,6 +252,7 @@ out:
>  	memzero_explicit(raw_key, sizeof(raw_key));
>  	return res;
>  }
> +EXPORT_SYMBOL(fscrypt_get_encryption_info);
>  
>  void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci)
>  {
> @@ -288,17 +270,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 76cff18..8e0bc584 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];
>  };
>  
> @@ -280,7 +279,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 *);
>  

Looks to be a reasonable backport.

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Kleber Sacilotto de Souza June 8, 2017, 9:38 a.m. UTC | #3 
Applied to yakkety/master-next branch. Thank you.
diff mbox

Patch

diff --git a/fs/crypto/crypto.c b/fs/crypto/crypto.c
index 55d64fb..624807c 100644
--- a/fs/crypto/crypto.c
+++ b/fs/crypto/crypto.c
@@ -353,7 +353,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)
@@ -365,18 +364,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 5d6d491..8dcbfee 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -360,7 +360,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 1ac263e..8ec4371 100644
--- a/fs/crypto/keyinfo.c
+++ b/fs/crypto/keyinfo.c
@@ -102,6 +102,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
@@ -109,11 +110,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;
@@ -124,17 +123,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;
 }
@@ -144,12 +137,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;
@@ -159,21 +151,15 @@  int get_crypt_info(struct inode *inode)
 	u8 mode;
 	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) {
@@ -195,7 +181,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))
@@ -257,12 +242,8 @@  got_key:
 	if (res)
 		goto out;
 
-	memzero_explicit(raw_key, sizeof(raw_key));
-	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)
@@ -271,6 +252,7 @@  out:
 	memzero_explicit(raw_key, sizeof(raw_key));
 	return res;
 }
+EXPORT_SYMBOL(fscrypt_get_encryption_info);
 
 void fscrypt_put_encryption_info(struct inode *inode, struct fscrypt_info *ci)
 {
@@ -288,17 +270,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 76cff18..8e0bc584 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];
 };
 
@@ -280,7 +279,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 *);