[PATCH] fscrypt: Initialize ->ci_master_key before publication

[Eric Biggers <ebiggers@kernel.org>]

fscrypt_prepare_setflags() assumes that if the key for an inode with a
v2 encryption policy has been set up, then ->ci_master_key is non-NULL.
This is a reasonable assumption, but it does not take into account the
very short gap in fscrypt_setup_encryption_info() between the inode's
fscrypt_inode_info being set and ->ci_master_key being initialized.
Thus, a NULL pointer dereference was possible.

Fix this by closing the gap: make fscrypt_setup_encryption_info()
initialize ->ci_master_key before publication.  If it loses the cmpxchg
race afterwards, set it back to NULL so that put_crypt_info() continues
to have the same behavior.

Fixes: b1c0ec3599f4 ("fscrypt: add FS_IOC_REMOVE_ENCRYPTION_KEY ioctl")
Cc: stable@vger.kernel.org
Reported-by: Ada Smith <adasmithgt@gmail.com>
Signed-off-by: Eric Biggers <ebiggers@kernel.org>
---

This patch applies to v7.4-rc2, and I'm planning to take it via the
fscrypt tree.

 fs/crypto/keysetup.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index a77b889d7578c..0beec01c04185 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -621,6 +621,13 @@ fscrypt_setup_encryption_info(struct inode *inode,
 	if (res)
 		goto out;
 
+	/*
+	 * Preemptively initialize ->ci_master_key, even though we don't know
+	 * whether we'll win the race below yet, because if we do win the race
+	 * then other tasks can see it right away and need ->ci_master_key.
+	 */
+	crypt_info->ci_master_key = mk;
+
 	/*
 	 * For existing inodes, multiple tasks may race to set the inode's
 	 * fscrypt info pointer.  So use cmpxchg_release().  This pairs with the
@@ -634,7 +641,6 @@ fscrypt_setup_encryption_info(struct inode *inode,
 		 * crypt_info.  Now link it into the master key's inode list.
 		 */
 		if (mk) {
-			crypt_info->ci_master_key = mk;
 			refcount_inc(&mk->mk_active_refs);
 			spin_lock(&mk->mk_decrypted_inodes_lock);
 			list_add(&crypt_info->ci_master_key_link,
@@ -642,6 +648,12 @@ fscrypt_setup_encryption_info(struct inode *inode,
 			spin_unlock(&mk->mk_decrypted_inodes_lock);
 		}
 		crypt_info = NULL;
+	} else {
+		/*
+		 * Lost the race, so we won't be taking an active reference to
+		 * the master key.  Clear ->ci_master_key accordingly.
+		 */
+		crypt_info->ci_master_key = NULL;
 	}
 	res = 0;
 out:

base-commit: 7475ea8b58616b33c04afb976cf0fbaad9dd63fb
-- 
2.56.0