diff options
Diffstat (limited to 'fs/crypto')
| -rw-r--r-- | fs/crypto/fscrypt_private.h | 66 | ||||
| -rw-r--r-- | fs/crypto/hooks.c | 2 | ||||
| -rw-r--r-- | fs/crypto/keyring.c | 78 | ||||
| -rw-r--r-- | fs/crypto/keysetup.c | 13 |
4 files changed, 93 insertions, 66 deletions
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h index 2fb4ba435d27..1892356cf924 100644 --- a/fs/crypto/fscrypt_private.h +++ b/fs/crypto/fscrypt_private.h @@ -475,8 +475,28 @@ struct fscrypt_master_key_secret { * 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 { @@ -486,7 +506,7 @@ struct fscrypt_master_key { */ 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; /* @@ -496,8 +516,8 @@ struct fscrypt_master_key { * ->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, @@ -509,17 +529,10 @@ struct fscrypt_master_key { 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; @@ -542,7 +555,7 @@ struct fscrypt_master_key { * * 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; @@ -565,20 +578,17 @@ struct fscrypt_master_key { 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) diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c index 85d2975b69b7..52504dd478d3 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 (mk->mk_present) err = fscrypt_derive_dirhash_key(ci, mk); else err = -ENOKEY; diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c index a51fa6a33de1..f34a9b0b9e92 100644 --- a/fs/crypto/keyring.c +++ b/fs/crypto/keyring.c @@ -99,10 +99,10 @@ void fscrypt_put_master_key_activeref(struct super_block *sb, 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++) { @@ -121,6 +121,18 @@ void fscrypt_put_master_key_activeref(struct super_block *sb, 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) @@ -234,14 +246,13 @@ void fscrypt_destroy_keyring(struct super_block *sb) * 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); @@ -439,7 +450,8 @@ static int add_new_master_key(struct super_block *sb, } 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, @@ -478,11 +490,18 @@ static int add_existing_master_key(struct fscrypt_master_key *mk, 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; @@ -506,8 +525,8 @@ static int do_add_master_key(struct super_block *sb, 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); @@ -989,9 +1008,8 @@ static int try_to_lock_encrypted_files(struct super_block *sb, * * 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. @@ -1053,11 +1071,10 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users) } } - /* 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; @@ -1074,9 +1091,9 @@ static int do_remove_key(struct file *filp, void __user *_uarg, bool all_users) } /* * 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); @@ -1103,12 +1120,11 @@ 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 @@ -1150,7 +1166,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 (!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 */; 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); |