summaryrefslogtreecommitdiff
path: root/fs/crypto/fscrypt_private.h
diff options
context:
space:
mode:
authorEric Biggers <ebiggers@google.com>2020-11-16 19:26:26 -0800
committerEric Biggers <ebiggers@google.com>2020-11-24 15:29:47 -0800
commit4a4b8721f1a5e4b01e45b3153c68d5a1014b25de (patch)
treef9e59b463ec9d631ef1624450405a29e51befa79 /fs/crypto/fscrypt_private.h
parent234f1b7f8daf112655c87f75ae8932564f871225 (diff)
downloadlwn-4a4b8721f1a5e4b01e45b3153c68d5a1014b25de.tar.gz
lwn-4a4b8721f1a5e4b01e45b3153c68d5a1014b25de.zip
fscrypt: simplify master key locking
The stated reasons for separating fscrypt_master_key::mk_secret_sem from the standard semaphore contained in every 'struct key' no longer apply. First, due to commit a992b20cd4ee ("fscrypt: add fscrypt_prepare_new_inode() and fscrypt_set_context()"), fscrypt_get_encryption_info() is no longer called from within a filesystem transaction. Second, due to commit d3ec10aa9581 ("KEYS: Don't write out to userspace while holding key semaphore"), the semaphore for the "keyring" key type no longer ranks above page faults. That leaves performance as the only possible reason to keep the separate mk_secret_sem. Specifically, having mk_secret_sem reduces the contention between setup_file_encryption_key() and FS_IOC_{ADD,REMOVE}_ENCRYPTION_KEY. However, these ioctls aren't executed often, so this doesn't seem to be worth the extra complexity. Therefore, simplify the locking design by just using key->sem instead of mk_secret_sem. Link: https://lore.kernel.org/r/20201117032626.320275-1-ebiggers@kernel.org Signed-off-by: Eric Biggers <ebiggers@google.com>
Diffstat (limited to 'fs/crypto/fscrypt_private.h')
-rw-r--r--fs/crypto/fscrypt_private.h19
1 files changed, 6 insertions, 13 deletions
diff --git a/fs/crypto/fscrypt_private.h b/fs/crypto/fscrypt_private.h
index 322ecae9a758..a61d4dbf0a0b 100644
--- a/fs/crypto/fscrypt_private.h
+++ b/fs/crypto/fscrypt_private.h
@@ -439,16 +439,9 @@ struct fscrypt_master_key {
* FS_IOC_REMOVE_ENCRYPTION_KEY can be retried, or
* FS_IOC_ADD_ENCRYPTION_KEY can add the secret again.
*
- * Locking: protected by key->sem (outer) and mk_secret_sem (inner).
- * The reason for two locks is that key->sem also protects modifying
- * mk_users, which ranks it above the semaphore for the keyring key
- * type, which is in turn above page faults (via keyring_read). But
- * sometimes filesystems call fscrypt_get_encryption_info() from within
- * a transaction, which ranks it below page faults. So we need a
- * separate lock which protects mk_secret but not also mk_users.
+ * Locking: protected by this master key's key->sem.
*/
struct fscrypt_master_key_secret mk_secret;
- struct rw_semaphore mk_secret_sem;
/*
* For v1 policy keys: an arbitrary key descriptor which was assigned by
@@ -467,8 +460,8 @@ struct fscrypt_master_key {
*
* This is NULL for v1 policy keys; those can only be added by root.
*
- * Locking: in addition to this keyrings own semaphore, this is
- * protected by the master key's key->sem, so we can do atomic
+ * Locking: in addition to this keyring's own semaphore, this is
+ * protected by this master key's key->sem, so we can do atomic
* search+insert. It can also be searched without taking any locks, but
* in that case the returned key may have already been removed.
*/
@@ -510,9 +503,9 @@ is_master_key_secret_present(const struct fscrypt_master_key_secret *secret)
/*
* The READ_ONCE() is only necessary for fscrypt_drop_inode() and
* fscrypt_key_describe(). These run in atomic context, so they can't
- * take ->mk_secret_sem and thus 'secret' can change concurrently which
- * would be a data race. But they only need to know whether the secret
- * *was* present at the time of check, so READ_ONCE() suffices.
+ * take the key semaphore and thus 'secret' can change concurrently
+ * which would be a data race. But they only need to know whether the
+ * secret *was* present at the time of check, so READ_ONCE() suffices.
*/
return READ_ONCE(secret->size) != 0;
}