@@ -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(
@@ -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;
@@ -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 */;
@@ -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);
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(-)