fscrypt: track master key presence separately from secret

Master keys can be in one of three states: present, incompletely
removed, and absent (as per FSCRYPT_KEY_STATUS_* used in the UAPI).
Currently, the way that "present" is distinguished from "incompletely
removed" internally is by whether ->mk_secret exists or not.

With extent-based encryption, it will be necessary to allow per-extent
keys to be derived while the master key is incompletely removed, so that
I/O on open files will reliably continue working after removal of the
key has been initiated.  (We could allow I/O to sometimes fail in that
case, but that seems problematic for reasons such as writes getting
silently thrown away and diverging from the existing fscrypt semantics.)
Therefore, when the filesystem is using extent-based encryption,
->mk_secret can't be wiped when the key becomes incompletely removed.

As a prerequisite for doing that, this patch makes the "present" state
be tracked using a new field, ->mk_present.  No behavior is changed yet.

The basic idea here is borrowed from Josef Bacik's patch
"fscrypt: use a flag to indicate that the master key is being evicted"
(https://lore.kernel.org/r/e86c16dddc049ff065f877d793ad773e4c6bfad9.1696970227.git.josef@toxicpanda.com).
I reimplemented it using a "present" bool instead of an "evicted" flag,
fixed a couple bugs, and tried to update everything to be consistent.

Note: I considered adding a ->mk_status field instead, holding one of
FSCRYPT_KEY_STATUS_*.  At first that seemed nice, but it ended up being
more complex (despite simplifying FS_IOC_GET_ENCRYPTION_KEY_STATUS),
since it would have introduced redundancy and had weird locking rules.

Reviewed-by: Neal Gompa <neal@gompa.dev>
Reviewed-by: Josef Bacik <josef@toxicpanda.com>
Link: https://lore.kernel.org/r/20231015061055.62673-1-ebiggers@kernel.org
Signed-off-by: Eric Biggers <ebiggers@google.com>

1 files changed, 7 insertions, 6 deletions

diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 094d1b7a1ae6..d71f7c799e79 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -486,8 +486,8 @@ static int setup_file_encryption_key(struct fscrypt_inode_info *ci,
 	}
 	down_read(&mk->mk_sem);
 
-	/* Has the secret been removed (via FS_IOC_REMOVE_ENCRYPTION_KEY)? */
-	if (!is_master_key_secret_present(&mk->mk_secret)) {
+	if (!mk->mk_present) {
+		/* FS_IOC_REMOVE_ENCRYPTION_KEY has been executed on this key */
 		err = -ENOKEY;
 		goto out_release_key;
 	}
@@ -539,8 +539,8 @@ static void put_crypt_info(struct fscrypt_inode_info *ci)
 		/*
 		 * Remove this inode from the list of inodes that were unlocked
 		 * with the master key.  In addition, if we're removing the last
-		 * inode from a master key struct that already had its secret
-		 * removed, then complete the full removal of the struct.
+		 * inode from an incompletely removed key, then complete the
+		 * full removal of the key.
 		 */
 		spin_lock(&mk->mk_decrypted_inodes_lock);
 		list_del(&ci->ci_master_key_link);
@@ -801,13 +801,14 @@ int fscrypt_drop_inode(struct inode *inode)
 		return 0;
 
 	/*
-	 * Note: since we aren't holding the key semaphore, the result here can
+	 * We can't take ->mk_sem here, since this runs in atomic context.
+	 * Therefore, ->mk_present can change concurrently, and our result may
 	 * immediately become outdated.  But there's no correctness problem with
 	 * unnecessarily evicting.  Nor is there a correctness problem with not
 	 * evicting while iput() is racing with the key being removed, since
 	 * then the thread removing the key will either evict the inode itself
 	 * or will correctly detect that it wasn't evicted due to the race.
 	 */
-	return !is_master_key_secret_present(&ci->ci_master_key->mk_secret);
+	return !READ_ONCE(ci->ci_master_key->mk_present);
 }
 EXPORT_SYMBOL_GPL(fscrypt_drop_inode);