From: Eric Biggers <ebiggers@xxxxxxxxxx>
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@xxxxxxxxxxxxxx).
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.
Signed-off-by: Eric Biggers <ebiggers@xxxxxxxxxx>
---
Documentation/filesystems/fscrypt.rst | 4 +-
fs/crypto/fscrypt_private.h | 66 +++++++++++++----------
fs/crypto/hooks.c | 2 +-
fs/crypto/keyring.c | 78 ++++++++++++++++-----------
fs/crypto/keysetup.c | 13 ++---
5 files changed, 95 insertions(+), 68 deletions(-)
diff --git a/Documentation/filesystems/fscrypt.rst b/Documentation/filesystems/fscrypt.rst
index 28700fb41a00b..1b84f818e574e 100644
--- a/Documentation/filesystems/fscrypt.rst
+++ b/Documentation/filesystems/fscrypt.rst
@@ -1127,22 +1127,22 @@ The caller must zero all input fields, then fill in ``key_spec``:
``key_spec.type`` to FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR and fill
in ``key_spec.u.descriptor``.
- To get the status of a key for v2 encryption policies, set
``key_spec.type`` to FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER and fill
in ``key_spec.u.identifier``.
On success, 0 is returned and the kernel fills in the output fields:
- ``status`` indicates whether the key is absent, present, or
- incompletely removed. Incompletely removed means that the master
- secret has been removed, but some files are still in use; i.e.,
+ incompletely removed. Incompletely removed means that removal has
+ been initiated, but some files are still in use; i.e.,
`FS_IOC_REMOVE_ENCRYPTION_KEY`_ returned 0 but set the informational
status flag FSCRYPT_KEY_REMOVAL_STATUS_FLAG_FILES_BUSY.
- ``status_flags`` can contain the following flags:
- ``FSCRYPT_KEY_STATUS_FLAG_ADDED_BY_SELF`` indicates that the key
has added by the current user. This is only set for keys
identified by ``identifier`` rather than by ``descriptor``.
- ``user_count`` specifies the number of users who have added the key.
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 2fb4ba435d27d..1892356cf924a 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -468,65 +468,78 @@ struct fscrypt_master_key_secret {
/* For v1 policy keys: the raw key. Wiped for v2 policy keys. */
u8 raw[FSCRYPT_MAX_KEY_SIZE];
} __randomize_layout;
/*
* fscrypt_master_key - an in-use master key
*
* This represents a master encryption key which has been added to the
- * filesystem and can be used to "unlock" the encrypted files which were
- * encrypted with it.
+ * filesystem. There are three high-level states that a key can be in:
+ *
+ * FSCRYPT_KEY_STATUS_PRESENT
+ * Key is fully usable; it can be used to unlock inodes that are encrypted
+ * with it (this includes being able to create new inodes). ->mk_present
+ * indicates whether the key is in this state. ->mk_secret exists, the key
+ * is in the keyring, and ->mk_active_refs > 0 due to ->mk_present.
+ *
+ * FSCRYPT_KEY_STATUS_INCOMPLETELY_REMOVED
+ * Removal of this key has been initiated, but some inodes that were
+ * unlocked with it are still in-use. Like ABSENT, ->mk_secret is wiped,
+ * and the key can no longer be used to unlock inodes. Unlike ABSENT, the
+ * key is still in the keyring; ->mk_decrypted_inodes is nonempty; and
+ * ->mk_active_refs > 0, being equal to the size of ->mk_decrypted_inodes.
+ *
+ * This state transitions to ABSENT if ->mk_decrypted_inodes becomes empty,
+ * or to PRESENT if FS_IOC_ADD_ENCRYPTION_KEY is called again for this key.
+ *
+ * FSCRYPT_KEY_STATUS_ABSENT
+ * Key is fully removed. The key is no longer in the keyring,
+ * ->mk_decrypted_inodes is empty, ->mk_active_refs == 0, ->mk_secret is
+ * wiped, and the key can no longer be used to unlock inodes.
*/
struct fscrypt_master_key {
/*
* Link in ->s_master_keys->key_hashtable.
* Only valid if ->mk_active_refs > 0.
*/
struct hlist_node mk_node;
- /* Semaphore that protects ->mk_secret and ->mk_users */
+ /* Semaphore that protects ->mk_secret, ->mk_users, and ->mk_present */
struct rw_semaphore mk_sem;
/*
* Active and structural reference counts. An active ref guarantees
* that the struct continues to exist, continues to be in the keyring
* ->s_master_keys, and that any embedded subkeys (e.g.
* ->mk_direct_keys) that have been prepared continue to exist.
* A structural ref only guarantees that the struct continues to exist.
*
- * There is one active ref associated with ->mk_secret being present,
- * and one active ref for each inode in ->mk_decrypted_inodes.
+ * There is one active ref associated with ->mk_present being true, and
+ * one active ref for each inode in ->mk_decrypted_inodes.
*
* There is one structural ref associated with the active refcount being
* nonzero. Finding a key in the keyring also takes a structural ref,
* which is then held temporarily while the key is operated on.
*/
refcount_t mk_active_refs;
refcount_t mk_struct_refs;
struct rcu_head mk_rcu_head;
/*
- * The secret key material. After FS_IOC_REMOVE_ENCRYPTION_KEY is
- * executed, this is wiped and no new inodes can be unlocked with this
- * key; however, there may still be inodes in ->mk_decrypted_inodes
- * which could not be evicted. As long as some inodes still remain,
- * FS_IOC_REMOVE_ENCRYPTION_KEY can be retried, or
- * FS_IOC_ADD_ENCRYPTION_KEY can add the secret again.
+ * The secret key material. Wiped as soon as it is no longer needed;
+ * for details, see the fscrypt_master_key struct comment.
*
- * While ->mk_secret is present, one ref in ->mk_active_refs is held.
- *
- * Locking: protected by ->mk_sem. The manipulation of ->mk_active_refs
- * associated with this field is protected by ->mk_sem as well.
+ * Locking: protected by ->mk_sem.
*/
struct fscrypt_master_key_secret mk_secret;
/*
* For v1 policy keys: an arbitrary key descriptor which was assigned by
* userspace (->descriptor).
*
* For v2 policy keys: a cryptographic hash of this key (->identifier).
*/
struct fscrypt_key_specifier mk_spec;
@@ -535,21 +548,21 @@ struct fscrypt_master_key {
* Keyring which contains a key of type 'key_type_fscrypt_user' for each
* user who has added this key. Normally each key will be added by just
* one user, but it's possible that multiple users share a key, and in
* that case we need to keep track of those users so that one user can't
* remove the key before the others want it removed too.
*
* This is NULL for v1 policy keys; those can only be added by root.
*
* Locking: protected by ->mk_sem. (We don't just rely on the keyrings
* subsystem semaphore ->mk_users->sem, as we need support for atomic
- * search+insert along with proper synchronization with ->mk_secret.)
+ * search+insert along with proper synchronization with other fields.)
*/
struct key *mk_users;
/*
* List of inodes that were unlocked using this key. This allows the
* inodes to be evicted efficiently if the key is removed.
*/
struct list_head mk_decrypted_inodes;
spinlock_t mk_decrypted_inodes_lock;
@@ -558,34 +571,31 @@ struct fscrypt_master_key {
* that use them. Allocated and derived on-demand.
*/
struct fscrypt_prepared_key mk_direct_keys[FSCRYPT_MODE_MAX + 1];
struct fscrypt_prepared_key mk_iv_ino_lblk_64_keys[FSCRYPT_MODE_MAX + 1];
struct fscrypt_prepared_key mk_iv_ino_lblk_32_keys[FSCRYPT_MODE_MAX + 1];
/* Hash key for inode numbers. Initialized only when needed. */
siphash_key_t mk_ino_hash_key;
bool mk_ino_hash_key_initialized;
-} __randomize_layout;
-
-static inline bool
-is_master_key_secret_present(const struct fscrypt_master_key_secret *secret)
-{
/*
- * 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.
+ * Whether this key is in the "present" state, i.e. fully usable. For
+ * details, see the fscrypt_master_key struct comment.
+ *
+ * Locking: protected by ->mk_sem, but can be read locklessly using
+ * READ_ONCE(). Writers must use WRITE_ONCE() when concurrent readers
+ * are possible.
*/
- return READ_ONCE(secret->size) != 0;
-}
+ bool mk_present;
+
+} __randomize_layout;
static inline const char *master_key_spec_type(
const struct fscrypt_key_specifier *spec)
{
switch (spec->type) {
case FSCRYPT_KEY_SPEC_TYPE_DESCRIPTOR:
return "descriptor";
case FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER:
return "identifier";
}
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 85d2975b69b78..52504dd478d31 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -180,21 +180,21 @@ int fscrypt_prepare_setflags(struct inode *inode,
*/
if (IS_ENCRYPTED(inode) && (flags & ~oldflags & FS_CASEFOLD_FL)) {
err = fscrypt_require_key(inode);
if (err)
return err;
ci = inode->i_crypt_info;
if (ci->ci_policy.version != FSCRYPT_POLICY_V2)
return -EINVAL;
mk = ci->ci_master_key;
down_read(&mk->mk_sem);
- if (is_master_key_secret_present(&mk->mk_secret))
+ if (mk->mk_present)
err = fscrypt_derive_dirhash_key(ci, mk);
else
err = -ENOKEY;
up_read(&mk->mk_sem);
return err;
}
return 0;
}
/**
diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c
index a51fa6a33de10..f34a9b0b9e922 100644
--- a/fs/crypto/keyring.c
+++ b/fs/crypto/keyring.c
@@ -92,42 +92,54 @@ void fscrypt_put_master_key_activeref(struct super_block *sb,
* destroying any subkeys embedded in it.
*/
if (WARN_ON_ONCE(!sb->s_master_keys))
return;
spin_lock(&sb->s_master_keys->lock);
hlist_del_rcu(&mk->mk_node);
spin_unlock(&sb->s_master_keys->lock);
/*
- * ->mk_active_refs == 0 implies that ->mk_secret is not present and
- * that ->mk_decrypted_inodes is empty.
+ * ->mk_active_refs == 0 implies that ->mk_present is false and
+ * ->mk_decrypted_inodes is empty.
*/
- WARN_ON_ONCE(is_master_key_secret_present(&mk->mk_secret));
+ WARN_ON_ONCE(mk->mk_present);
WARN_ON_ONCE(!list_empty(&mk->mk_decrypted_inodes));
for (i = 0; i <= FSCRYPT_MODE_MAX; i++) {
fscrypt_destroy_prepared_key(
sb, &mk->mk_direct_keys[i]);
fscrypt_destroy_prepared_key(
sb, &mk->mk_iv_ino_lblk_64_keys[i]);
fscrypt_destroy_prepared_key(
sb, &mk->mk_iv_ino_lblk_32_keys[i]);
}
memzero_explicit(&mk->mk_ino_hash_key,
sizeof(mk->mk_ino_hash_key));
mk->mk_ino_hash_key_initialized = false;
/* Drop the structural ref associated with the active refs. */
fscrypt_put_master_key(mk);
}
+/*
+ * This transitions the key state from present to incompletely removed, and then
+ * potentially to absent (depending on whether inodes remain).
+ */
+static void fscrypt_initiate_key_removal(struct super_block *sb,
+ struct fscrypt_master_key *mk)
+{
+ WRITE_ONCE(mk->mk_present, false);
+ wipe_master_key_secret(&mk->mk_secret);
+ fscrypt_put_master_key_activeref(sb, mk);
+}
+
static inline bool valid_key_spec(const struct fscrypt_key_specifier *spec)
{
if (spec->__reserved)
return false;
return master_key_spec_len(spec) != 0;
}
static int fscrypt_user_key_instantiate(struct key *key,
struct key_preparsed_payload *prep)
{
@@ -227,28 +239,27 @@ void fscrypt_destroy_keyring(struct super_block *sb)
struct hlist_head *bucket = &keyring->key_hashtable[i];
struct fscrypt_master_key *mk;
struct hlist_node *tmp;
hlist_for_each_entry_safe(mk, tmp, bucket, mk_node) {
/*
* Since all potentially-encrypted inodes were already
* evicted, every key remaining in the keyring should
* have an empty inode list, and should only still be in
* 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.
+ * with ->mk_present. There should be no structural
+ * refs beyond the one associated with the active ref.
*/
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));
- wipe_master_key_secret(&mk->mk_secret);
- fscrypt_put_master_key_activeref(sb, mk);
+ WARN_ON_ONCE(!mk->mk_present);
+ fscrypt_initiate_key_removal(sb, mk);
}
}
kfree_sensitive(keyring);
sb->s_master_keys = NULL;
}
static struct hlist_head *
fscrypt_mk_hash_bucket(struct fscrypt_keyring *keyring,
const struct fscrypt_key_specifier *mk_spec)
{
@@ -432,21 +443,22 @@ static int add_new_master_key(struct super_block *sb,
if (mk_spec->type == FSCRYPT_KEY_SPEC_TYPE_IDENTIFIER) {
err = allocate_master_key_users_keyring(mk);
if (err)
goto out_put;
err = add_master_key_user(mk);
if (err)
goto out_put;
}
move_master_key_secret(&mk->mk_secret, secret);
- refcount_set(&mk->mk_active_refs, 1); /* ->mk_secret is present */
+ mk->mk_present = true;
+ refcount_set(&mk->mk_active_refs, 1); /* ->mk_present is true */
spin_lock(&keyring->lock);
hlist_add_head_rcu(&mk->mk_node,
fscrypt_mk_hash_bucket(keyring, mk_spec));
spin_unlock(&keyring->lock);
return 0;
out_put:
fscrypt_put_master_key(mk);
return err;
@@ -471,25 +483,32 @@ static int add_existing_master_key(struct fscrypt_master_key *mk,
if (IS_ERR(mk_user))
return PTR_ERR(mk_user);
key_put(mk_user);
return 0;
}
err = add_master_key_user(mk);
if (err)
return err;
}
- /* 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 the key is incompletely removed, make it present again. */
+ if (!mk->mk_present) {
+ if (!refcount_inc_not_zero(&mk->mk_active_refs)) {
+ /*
+ * Raced with the last active ref being dropped, so the
+ * key has become, or is about to become, "absent".
+ * Therefore, we need to allocate a new key struct.
+ */
return KEY_DEAD;
+ }
move_master_key_secret(&mk->mk_secret, secret);
+ WRITE_ONCE(mk->mk_present, true);
}
return 0;
}
static int do_add_master_key(struct super_block *sb,
struct fscrypt_master_key_secret *secret,
const struct fscrypt_key_specifier *mk_spec)
{
static DEFINE_MUTEX(fscrypt_add_key_mutex);
@@ -499,22 +518,22 @@ static int do_add_master_key(struct super_block *sb,
mutex_lock(&fscrypt_add_key_mutex); /* serialize find + link */
mk = fscrypt_find_master_key(sb, mk_spec);
if (!mk) {
/* Didn't find the key in ->s_master_keys. Add it. */
err = allocate_filesystem_keyring(sb);
if (!err)
err = add_new_master_key(sb, secret, mk_spec);
} else {
/*
- * Found the key in ->s_master_keys. Re-add the secret if
- * needed, and add the user to ->mk_users if needed.
+ * Found the key in ->s_master_keys. Add the user to ->mk_users
+ * if needed, and make the key "present" again if possible.
*/
down_write(&mk->mk_sem);
err = add_existing_master_key(mk, secret);
up_write(&mk->mk_sem);
if (err == KEY_DEAD) {
/*
* We found a key struct, but it's already been fully
* removed. Ignore the old struct and add a new one.
* fscrypt_add_key_mutex means we don't need to worry
* about concurrent adds.
@@ -982,23 +1001,22 @@ static int try_to_lock_encrypted_files(struct super_block *sb,
* claim to the key, then removes the key itself if no other users have claims.
* FS_IOC_REMOVE_ENCRYPTION_KEY_ALL_USERS (all_users=true) always removes the
* key itself.
*
* To "remove the key itself", first we wipe the actual master key secret, so
* that no more inodes can be unlocked with it. Then we try to evict all cached
* inodes that had been unlocked with the key.
*
* If all inodes were evicted, then we unlink the fscrypt_master_key from the
* keyring. Otherwise it remains in the keyring in the "incompletely removed"
- * state (without the actual secret key) where it tracks the list of remaining
- * inodes. Userspace can execute the ioctl again later to retry eviction, or
- * alternatively can re-add the secret key again.
+ * state where it tracks the list of remaining inodes. Userspace can execute
+ * the ioctl again later to retry eviction, or alternatively can re-add the key.
*
* For more details, see the "Removing keys" section of
* Documentation/filesystems/fscrypt.rst.
*/
static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users)
{
struct super_block *sb = file_inode(filp)->i_sb;
struct fscrypt_remove_key_arg __user *uarg = _uarg;
struct fscrypt_remove_key_arg arg;
struct fscrypt_master_key *mk;
@@ -1046,44 +1064,43 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users)
* can't remove the key itself.
*/
status_flags |=
FSCRYPT_KEY_REMOVAL_STATUS_FLAG_OTHER_USERS;
err = 0;
up_write(&mk->mk_sem);
goto out_put_key;
}
}
- /* No user claims remaining. Go ahead and wipe the secret. */
+ /* No user claims remaining. Initiate removal of the key. */
err = -ENOKEY;
- if (is_master_key_secret_present(&mk->mk_secret)) {
- wipe_master_key_secret(&mk->mk_secret);
- fscrypt_put_master_key_activeref(sb, mk);
+ if (mk->mk_present) {
+ fscrypt_initiate_key_removal(sb, mk);
err = 0;
}
inodes_remain = refcount_read(&mk->mk_active_refs) > 0;
up_write(&mk->mk_sem);
if (inodes_remain) {
/* Some inodes still reference this key; try to evict them. */
err = try_to_lock_encrypted_files(sb, mk);
if (err == -EBUSY) {
status_flags |=
FSCRYPT_KEY_REMOVAL_STATUS_FLAG_FILES_BUSY;
err = 0;
}
}
/*
* We return 0 if we successfully did something: removed a claim to the
- * key, wiped the secret, or tried locking the files again. Users need
- * to check the informational status flags if they care whether the key
- * has been fully removed including all files locked.
+ * key, initiated removal of the key, or tried locking the files again.
+ * Users need to check the informational status flags if they care
+ * whether the key has been fully removed including all files locked.
*/
out_put_key:
fscrypt_put_master_key(mk);
if (err == 0)
err = put_user(status_flags, &uarg->removal_status_flags);
return err;
}
int fscrypt_ioctl_remove_key(struct file *filp, void __user *uarg)
{
@@ -1096,26 +1113,25 @@ int fscrypt_ioctl_remove_key_all_users(struct file *filp, void __user *uarg)
if (!capable(CAP_SYS_ADMIN))
return -EACCES;
return do_remove_key(filp, uarg, true);
}
EXPORT_SYMBOL_GPL(fscrypt_ioctl_remove_key_all_users);
/*
* Retrieve the status of an fscrypt master encryption key.
*
* We set ->status to indicate whether the key is absent, present, or
- * incompletely removed. "Incompletely removed" means that the master key
- * secret has been removed, but some files which had been unlocked with it are
- * still in use. This field allows applications to easily determine the state
- * of an encrypted directory without using a hack such as trying to open a
- * regular file in it (which can confuse the "incompletely removed" state with
- * absent or present).
+ * incompletely removed. (For an explanation of what these statuses mean and
+ * how they are represented internally, see struct fscrypt_master_key.) This
+ * field allows applications to easily determine the status of an encrypted
+ * directory without using a hack such as trying to open a regular file in it
+ * (which can confuse the "incompletely removed" status with absent or present).
*
* In addition, for v2 policy keys we allow applications to determine, via
* ->status_flags and ->user_count, whether the key has been added by the
* current user, by other users, or by both. Most applications should not need
* this, since ordinarily only one user should know a given key. However, if a
* secret key is shared by multiple users, applications may wish to add an
* already-present key to prevent other users from removing it. This ioctl can
* be used to check whether that really is the case before the work is done to
* add the key --- which might e.g. require prompting the user for a passphrase.
*
@@ -1143,21 +1159,21 @@ int fscrypt_ioctl_get_key_status(struct file *filp, void __user *uarg)
memset(arg.__out_reserved, 0, sizeof(arg.__out_reserved));
mk = fscrypt_find_master_key(sb, &arg.key_spec);
if (!mk) {
arg.status = FSCRYPT_KEY_STATUS_ABSENT;
err = 0;
goto out;
}
down_read(&mk->mk_sem);
- if (!is_master_key_secret_present(&mk->mk_secret)) {
+ if (!mk->mk_present) {
arg.status = refcount_read(&mk->mk_active_refs) > 0 ?
FSCRYPT_KEY_STATUS_INCOMPLETELY_REMOVED :
FSCRYPT_KEY_STATUS_ABSENT /* raced with full removal */;
err = 0;
goto out_release_key;
}
arg.status = FSCRYPT_KEY_STATUS_PRESENT;
if (mk->mk_users) {
struct key *mk_user;
diff --git a/fs/crypto/keysetup.c b/fs/crypto/keysetup.c
index 094d1b7a1ae61..d71f7c799e79e 100644
--- a/fs/crypto/keysetup.c
+++ b/fs/crypto/keysetup.c
@@ -479,22 +479,22 @@ static int setup_file_encryption_key(struct fscrypt_inode_info *ci,
/*
* As a legacy fallback for v1 policies, search for the key in
* the current task's subscribed keyrings too. Don't move this
* to before the search of ->s_master_keys, since users
* shouldn't be able to override filesystem-level keys.
*/
return fscrypt_setup_v1_file_key_via_subscribed_keyrings(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;
}
if (!fscrypt_valid_master_key_size(mk, ci)) {
err = -ENOKEY;
goto out_release_key;
}
switch (ci->ci_policy.version) {
@@ -532,22 +532,22 @@ static void put_crypt_info(struct fscrypt_inode_info *ci)
fscrypt_put_direct_key(ci->ci_direct_key);
else if (ci->ci_owns_key)
fscrypt_destroy_prepared_key(ci->ci_inode->i_sb,
&ci->ci_enc_key);
mk = ci->ci_master_key;
if (mk) {
/*
* 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);
spin_unlock(&mk->mk_decrypted_inodes_lock);
fscrypt_put_master_key_activeref(ci->ci_inode->i_sb, mk);
}
memzero_explicit(ci, sizeof(*ci));
kmem_cache_free(fscrypt_inode_info_cachep, ci);
}
@@ -794,20 +794,21 @@ int fscrypt_drop_inode(struct inode *inode)
/*
* With proper, non-racy use of FS_IOC_REMOVE_ENCRYPTION_KEY, all inodes
* protected by the key were cleaned by sync_filesystem(). But if
* userspace is still using the files, inodes can be dirtied between
* then and now. We mustn't lose any writes, so skip dirty inodes here.
*/
if (inode->i_state & I_DIRTY_ALL)
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);
base-commit: 3e7807d5a7d770c59837026e9967fe99ad043174
--
2.42.0
|