All of lore.kernel.org
 help / color / mirror / Atom feed
From: Josef Bacik <josef@toxicpanda.com>
To: linux-fscrypt@vger.kernel.org, ebiggers@kernel.org,
	linux-btrfs@vger.kernel.org
Subject: [PATCH v2 01/36] fscrypt: use a flag to indicate that the master key is being evicted
Date: Tue, 10 Oct 2023 16:40:16 -0400	[thread overview]
Message-ID: <e86c16dddc049ff065f877d793ad773e4c6bfad9.1696970227.git.josef@toxicpanda.com> (raw)
In-Reply-To: <cover.1696970227.git.josef@toxicpanda.com>

Currently we wipe the mk->mk_secret when we remove the master key, and
we use this status to tell everybody whether or not the master key is
available for use.

With extent based encryption we're going to need to keep the secret
around until the inode is evicted, so we need a different mechanism to
tell everybody that the key is currently unusable.

Accomplish this with a mk_flags member in the master key, and update the
is_master_key_secret_present() helper to return the status of this bit.
Update the removal and adding helpers to manipulate this bit and use it
as the source of truth about whether or not the key is available for
use.

Signed-off-by: Josef Bacik <josef@toxicpanda.com>
---
 fs/crypto/fscrypt_private.h | 17 ++++++++---------
 fs/crypto/hooks.c           |  2 +-
 fs/crypto/keyring.c         | 20 ++++++++++++++------
 fs/crypto/keysetup.c        |  4 ++--
 4 files changed, 25 insertions(+), 18 deletions(-)

diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 2fb4ba435d27..f44342f17269 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -471,6 +471,10 @@ struct fscrypt_master_key_secret {
 
 } __randomize_layout;
 
+enum fscrypt_mk_flags {
+	FSCRYPT_MK_FLAG_EVICTED = BIT(0),
+};
+
 /*
  * fscrypt_master_key - an in-use master key
  *
@@ -565,19 +569,14 @@ struct fscrypt_master_key {
 	siphash_key_t		mk_ino_hash_key;
 	bool			mk_ino_hash_key_initialized;
 
+	/* Flags for the master key. */
+	unsigned long		mk_flags;
 } __randomize_layout;
 
 static inline bool
-is_master_key_secret_present(const struct fscrypt_master_key_secret *secret)
+is_master_key_secret_present(const struct fscrypt_master_key *mk)
 {
-	/*
-	 * The READ_ONCE() is only necessary for fscrypt_drop_inode().
-	 * fscrypt_drop_inode() runs in atomic context, so it can't take the key
-	 * semaphore and thus 'secret' can change concurrently which would be a
-	 * data race.  But fscrypt_drop_inode() only need to know whether the
-	 * secret *was* present at the time of check, so READ_ONCE() suffices.
-	 */
-	return READ_ONCE(secret->size) != 0;
+	return !test_bit(FSCRYPT_MK_FLAG_EVICTED, &mk->mk_flags);
 }
 
 static inline const char *master_key_spec_type(
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 85d2975b69b7..f7cf724cf256 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -187,7 +187,7 @@ int fscrypt_prepare_setflags(struct inode *inode,
 			return -EINVAL;
 		mk = ci->ci_master_key;
 		down_read(&mk->mk_sem);
-		if (is_master_key_secret_present(&mk->mk_secret))
+		if (is_master_key_secret_present(mk))
 			err = fscrypt_derive_dirhash_key(ci, mk);
 		else
 			err = -ENOKEY;
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index a51fa6a33de1..e0e311ed6b88 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -102,7 +102,7 @@ void fscrypt_put_master_key_activeref(struct super_block *sb,
 	 * ->mk_active_refs == 0 implies that ->mk_secret is not present and
 	 * that ->mk_decrypted_inodes is empty.
 	 */
-	WARN_ON_ONCE(is_master_key_secret_present(&mk->mk_secret));
+	WARN_ON_ONCE(is_master_key_secret_present(mk));
 	WARN_ON_ONCE(!list_empty(&mk->mk_decrypted_inodes));
 
 	for (i = 0; i <= FSCRYPT_MODE_MAX; i++) {
@@ -236,11 +236,17 @@ void fscrypt_destroy_keyring(struct super_block *sb)
 			 * the keyring due to the single active ref associated
 			 * with ->mk_secret.  There should be no structural refs
 			 * beyond the one associated with the active ref.
+			 *
+			 * We set the EVICTED flag in order to avoid the
+			 * WARN_ON_ONCE(is_master_key_secret_present(mk)) in
+			 * fscrypt_put_master_key_activeref(), as we want to
+			 * maintain that warning for improper cleanup elsewhere.
 			 */
 			WARN_ON_ONCE(refcount_read(&mk->mk_active_refs) != 1);
 			WARN_ON_ONCE(refcount_read(&mk->mk_struct_refs) != 1);
-			WARN_ON_ONCE(!is_master_key_secret_present(&mk->mk_secret));
+			WARN_ON_ONCE(!is_master_key_secret_present(mk));
 			wipe_master_key_secret(&mk->mk_secret);
+			set_bit(FSCRYPT_MK_FLAG_EVICTED, &mk->mk_flags);
 			fscrypt_put_master_key_activeref(sb, mk);
 		}
 	}
@@ -479,9 +485,11 @@ static int add_existing_master_key(struct fscrypt_master_key *mk,
 	}
 
 	/* Re-add the secret if needed. */
-	if (!is_master_key_secret_present(&mk->mk_secret)) {
-		if (!refcount_inc_not_zero(&mk->mk_active_refs))
+	if (test_and_clear_bit(FSCRYPT_MK_FLAG_EVICTED, &mk->mk_flags)) {
+		if (!refcount_inc_not_zero(&mk->mk_active_refs)) {
+			set_bit(FSCRYPT_MK_FLAG_EVICTED, &mk->mk_flags);
 			return KEY_DEAD;
+		}
 		move_master_key_secret(&mk->mk_secret, secret);
 	}
 
@@ -1055,7 +1063,7 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users)
 
 	/* No user claims remaining.  Go ahead and wipe the secret. */
 	err = -ENOKEY;
-	if (is_master_key_secret_present(&mk->mk_secret)) {
+	if (!test_and_set_bit(FSCRYPT_MK_FLAG_EVICTED, &mk->mk_flags)) {
 		wipe_master_key_secret(&mk->mk_secret);
 		fscrypt_put_master_key_activeref(sb, mk);
 		err = 0;
@@ -1150,7 +1158,7 @@ int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
 	}
 	down_read(&mk->mk_sem);
 
-	if (!is_master_key_secret_present(&mk->mk_secret)) {
+	if (!is_master_key_secret_present(mk)) {
 		arg.status = refcount_read(&mk->mk_active_refs) > 0 ?
 			FSCRYPT_KEY_STATUS_INCOMPLETELY_REMOVED :
 			FSCRYPT_KEY_STATUS_ABSENT /* raced with full removal */;
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 094d1b7a1ae6..92eca1400b2d 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -487,7 +487,7 @@ 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 (!is_master_key_secret_present(mk)) {
 		err = -ENOKEY;
 		goto out_release_key;
 	}
@@ -808,6 +808,6 @@ int fscrypt_drop_inode(struct inode *inode)
 	 * 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 !is_master_key_secret_present(ci->ci_master_key);
 }
 EXPORT_SYMBOL_GPL(fscrypt_drop_inode);
-- 
2.41.0


  reply	other threads:[~2023-10-10 20:41 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-10-10 20:40 [PATCH v2 00/36] btrfs: add fscrypt support Josef Bacik
2023-10-10 20:40 ` Josef Bacik [this message]
2023-10-15  6:22   ` [PATCH v2 01/36] fscrypt: use a flag to indicate that the master key is being evicted Eric Biggers
2023-10-10 20:40 ` [PATCH v2 02/36] fscrypt: don't wipe mk secret until the last active user is gone Josef Bacik
2023-10-15  6:25   ` Eric Biggers
2023-10-10 20:40 ` [PATCH v2 03/36] fscrypt: add per-extent encryption support Josef Bacik
2023-10-15  6:36   ` Eric Biggers
2023-10-10 20:40 ` [PATCH v2 04/36] fscrypt: disable all but standard v2 policies for extent encryption Josef Bacik
2023-10-15  6:27   ` Eric Biggers
2023-10-10 20:40 ` [PATCH v2 05/36] blk-crypto: add a process bio callback Josef Bacik
2023-10-15  6:32   ` Eric Biggers
2023-10-10 20:40 ` [PATCH v2 06/36] fscrypt: expose fscrypt_nokey_name Josef Bacik
2023-10-10 20:40 ` [PATCH v2 07/36] fscrypt: add documentation about extent encryption Josef Bacik
2023-10-10 20:40 ` [PATCH v2 08/36] btrfs: add infrastructure for safe em freeing Josef Bacik
2023-10-10 20:40 ` [PATCH v2 09/36] btrfs: disable various operations on encrypted inodes Josef Bacik
2023-10-10 20:40 ` [PATCH v2 10/36] btrfs: disable verity " Josef Bacik
2023-10-10 20:40 ` [PATCH v2 11/36] btrfs: start using fscrypt hooks Josef Bacik
2023-10-10 20:40 ` [PATCH v2 12/36] btrfs: add inode encryption contexts Josef Bacik
2023-10-10 20:40 ` [PATCH v2 13/36] btrfs: add new FEATURE_INCOMPAT_ENCRYPT flag Josef Bacik
2023-10-10 20:40 ` [PATCH v2 14/36] btrfs: adapt readdir for encrypted and nokey names Josef Bacik
2023-10-10 20:40 ` [PATCH v2 15/36] btrfs: handle " Josef Bacik
2023-10-10 20:40 ` [PATCH v2 16/36] btrfs: implement fscrypt ioctls Josef Bacik
2023-10-10 20:40 ` [PATCH v2 17/36] btrfs: add encryption to CONFIG_BTRFS_DEBUG Josef Bacik
2023-10-10 20:40 ` [PATCH v2 18/36] btrfs: add get_devices hook for fscrypt Josef Bacik
2023-10-10 20:40 ` [PATCH v2 19/36] btrfs: turn on inlinecrypt mount option for encrypt Josef Bacik
2023-10-10 20:40 ` [PATCH v2 20/36] btrfs: set file extent encryption excplicitly Josef Bacik
2023-10-10 20:40 ` [PATCH v2 21/36] btrfs: add fscrypt_info and encryption_type to extent_map Josef Bacik
2023-10-10 20:40 ` [PATCH v2 22/36] btrfs: add fscrypt_info and encryption_type to ordered_extent Josef Bacik
2023-10-10 20:40 ` [PATCH v2 23/36] btrfs: plumb through setting the fscrypt_info for ordered extents Josef Bacik
2023-10-10 20:40 ` [PATCH v2 24/36] btrfs: populate the ordered_extent with the fscrypt context Josef Bacik
2023-10-10 20:40 ` [PATCH v2 25/36] btrfs: keep track of fscrypt info and orig_start for dio reads Josef Bacik
2023-10-10 20:40 ` [PATCH v2 26/36] btrfs: add an optional encryption context to the end of file extents Josef Bacik
2023-10-10 20:40 ` [PATCH v2 27/36] btrfs: explicitly track file extent length for replace and drop Josef Bacik
2023-10-10 20:40 ` [PATCH v2 28/36] btrfs: pass through fscrypt_extent_info to the file extent helpers Josef Bacik
2023-10-10 20:40 ` [PATCH v2 29/36] btrfs: pass the fscrypt_info through the replace extent infrastructure Josef Bacik
2023-10-10 20:40 ` [PATCH v2 30/36] btrfs: implement the fscrypt extent encryption hooks Josef Bacik
2023-10-10 20:40 ` [PATCH v2 31/36] btrfs: setup fscrypt_extent_info for new extents Josef Bacik
2023-10-10 20:40 ` [PATCH v2 32/36] btrfs: populate ordered_extent with the orig offset Josef Bacik
2023-10-10 20:40 ` [PATCH v2 33/36] btrfs: set the bio fscrypt context when applicable Josef Bacik
2023-10-10 20:40 ` [PATCH v2 34/36] btrfs: add a bio argument to btrfs_csum_one_bio Josef Bacik
2023-10-10 20:40 ` [PATCH v2 35/36] btrfs: add orig_logical to btrfs_bio Josef Bacik
2023-10-10 20:40 ` [PATCH v2 36/36] btrfs: implement process_bio cb for fscrypt Josef Bacik
2023-11-21 23:02 ` [PATCH v2 00/36] btrfs: add fscrypt support Eric Biggers
2023-11-22 13:58   ` Josef Bacik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=e86c16dddc049ff065f877d793ad773e4c6bfad9.1696970227.git.josef@toxicpanda.com \
    --to=josef@toxicpanda.com \
    --cc=ebiggers@kernel.org \
    --cc=linux-btrfs@vger.kernel.org \
    --cc=linux-fscrypt@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.