From 5fee36095cda45d34555aed3a2e8973b80cd6bf8 Mon Sep 17 00:00:00 2001 From: Satya Tangirala Date: Thu, 2 Jul 2020 01:56:05 +0000 Subject: fscrypt: add inline encryption support Add support for inline encryption to fs/crypto/. With "inline encryption", the block layer handles the decryption/encryption as part of the bio, instead of the filesystem doing the crypto itself via Linux's crypto API. This model is needed in order to take advantage of the inline encryption hardware present on most modern mobile SoCs. To use inline encryption, the filesystem needs to be mounted with '-o inlinecrypt'. Blk-crypto will then be used instead of the traditional filesystem-layer crypto whenever possible to encrypt the contents of any encrypted files in that filesystem. Fscrypt still provides the key and IV to use, and the actual ciphertext on-disk is still the same; therefore it's testable using the existing fscrypt ciphertext verification tests. Note that since blk-crypto has a fallback to Linux's crypto API, and also supports all the encryption modes currently supported by fscrypt, this feature is usable and testable even without actual inline encryption hardware. Per-filesystem changes will be needed to set encryption contexts when submitting bios and to implement the 'inlinecrypt' mount option. This patch just adds the common code. Signed-off-by: Satya Tangirala Reviewed-by: Jaegeuk Kim Reviewed-by: Eric Biggers Reviewed-by: Theodore Ts'o Link: https://lore.kernel.org/r/20200702015607.1215430-3-satyat@google.com Co-developed-by: Eric Biggers Signed-off-by: Eric Biggers --- fs/crypto/keyring.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'fs/crypto/keyring.c') diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c index e24eb48bfbe1..7f8ac61a20d6 100644 --- a/fs/crypto/keyring.c +++ b/fs/crypto/keyring.c @@ -45,9 +45,9 @@ static void free_master_key(struct fscrypt_master_key *mk) wipe_master_key_secret(&mk->mk_secret); for (i = 0; i <= __FSCRYPT_MODE_MAX; i++) { - crypto_free_skcipher(mk->mk_direct_keys[i]); - crypto_free_skcipher(mk->mk_iv_ino_lblk_64_keys[i]); - crypto_free_skcipher(mk->mk_iv_ino_lblk_32_keys[i]); + fscrypt_destroy_prepared_key(&mk->mk_direct_keys[i]); + fscrypt_destroy_prepared_key(&mk->mk_iv_ino_lblk_64_keys[i]); + fscrypt_destroy_prepared_key(&mk->mk_iv_ino_lblk_32_keys[i]); } key_put(mk->mk_users); -- cgit From 777afe4e68d7ac37711d36098bea65650ec305a0 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 21 Jul 2020 15:59:18 -0700 Subject: fscrypt: use smp_load_acquire() for ->s_master_keys Normally smp_store_release() or cmpxchg_release() is paired with smp_load_acquire(). Sometimes smp_load_acquire() can be replaced with the more lightweight READ_ONCE(). However, for this to be safe, all the published memory must only be accessed in a way that involves the pointer itself. This may not be the case if allocating the object also involves initializing a static or global variable, for example. super_block::s_master_keys is a keyring, which is internal to and is allocated by the keyrings subsystem. By using READ_ONCE() for it, we're relying on internal implementation details of the keyrings subsystem. Remove this fragile assumption by using smp_load_acquire() instead. (Note: I haven't seen any real-world problems here. This change is just fixing the code to be guaranteed correct and less fragile.) Fixes: 22d94f493bfb ("fscrypt: add FS_IOC_ADD_ENCRYPTION_KEY ioctl") Link: https://lore.kernel.org/r/20200721225920.114347-4-ebiggers@kernel.org Signed-off-by: Eric Biggers --- fs/crypto/keyring.c | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) (limited to 'fs/crypto/keyring.c') diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c index 7f8ac61a20d6..71d56f8e2870 100644 --- a/fs/crypto/keyring.c +++ b/fs/crypto/keyring.c @@ -213,7 +213,11 @@ static int allocate_filesystem_keyring(struct super_block *sb) if (IS_ERR(keyring)) return PTR_ERR(keyring); - /* Pairs with READ_ONCE() in fscrypt_find_master_key() */ + /* + * Pairs with the smp_load_acquire() in fscrypt_find_master_key(). + * I.e., here we publish ->s_master_keys with a RELEASE barrier so that + * concurrent tasks can ACQUIRE it. + */ smp_store_release(&sb->s_master_keys, keyring); return 0; } @@ -234,8 +238,13 @@ struct key *fscrypt_find_master_key(struct super_block *sb, struct key *keyring; char description[FSCRYPT_MK_DESCRIPTION_SIZE]; - /* pairs with smp_store_release() in allocate_filesystem_keyring() */ - keyring = READ_ONCE(sb->s_master_keys); + /* + * Pairs with the smp_store_release() in allocate_filesystem_keyring(). + * I.e., another task can publish ->s_master_keys concurrently, + * executing a RELEASE barrier. We need to use smp_load_acquire() here + * to safely ACQUIRE the memory the other task published. + */ + keyring = smp_load_acquire(&sb->s_master_keys); if (keyring == NULL) return ERR_PTR(-ENOKEY); /* No keyring yet, so no keys yet. */ -- cgit From 453431a54934d917153c65211b2dabf45562ca88 Mon Sep 17 00:00:00 2001 From: Waiman Long Date: Thu, 6 Aug 2020 23:18:13 -0700 Subject: mm, treewide: rename kzfree() to kfree_sensitive() As said by Linus: A symmetric naming is only helpful if it implies symmetries in use. Otherwise it's actively misleading. In "kzalloc()", the z is meaningful and an important part of what the caller wants. In "kzfree()", the z is actively detrimental, because maybe in the future we really _might_ want to use that "memfill(0xdeadbeef)" or something. The "zero" part of the interface isn't even _relevant_. The main reason that kzfree() exists is to clear sensitive information that should not be leaked to other future users of the same memory objects. Rename kzfree() to kfree_sensitive() to follow the example of the recently added kvfree_sensitive() and make the intention of the API more explicit. In addition, memzero_explicit() is used to clear the memory to make sure that it won't get optimized away by the compiler. The renaming is done by using the command sequence: git grep -w --name-only kzfree |\ xargs sed -i 's/kzfree/kfree_sensitive/' followed by some editing of the kfree_sensitive() kerneldoc and adding a kzfree backward compatibility macro in slab.h. [akpm@linux-foundation.org: fs/crypto/inline_crypt.c needs linux/slab.h] [akpm@linux-foundation.org: fix fs/crypto/inline_crypt.c some more] Suggested-by: Joe Perches Signed-off-by: Waiman Long Signed-off-by: Andrew Morton Acked-by: David Howells Acked-by: Michal Hocko Acked-by: Johannes Weiner Cc: Jarkko Sakkinen Cc: James Morris Cc: "Serge E. Hallyn" Cc: Joe Perches Cc: Matthew Wilcox Cc: David Rientjes Cc: Dan Carpenter Cc: "Jason A . Donenfeld" Link: http://lkml.kernel.org/r/20200616154311.12314-3-longman@redhat.com Signed-off-by: Linus Torvalds --- fs/crypto/keyring.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'fs/crypto/keyring.c') diff --git a/fs/crypto/keyring.c b/fs/crypto/keyring.c index 71d56f8e2870..e74f239c4428 100644 --- a/fs/crypto/keyring.c +++ b/fs/crypto/keyring.c @@ -51,7 +51,7 @@ static void free_master_key(struct fscrypt_master_key *mk) } key_put(mk->mk_users); - kzfree(mk); + kfree_sensitive(mk); } static inline bool valid_key_spec(const struct fscrypt_key_specifier *spec) @@ -531,7 +531,7 @@ static int fscrypt_provisioning_key_preparse(struct key_preparsed_payload *prep) static void fscrypt_provisioning_key_free_preparse( struct key_preparsed_payload *prep) { - kzfree(prep->payload.data[0]); + kfree_sensitive(prep->payload.data[0]); } static void fscrypt_provisioning_key_describe(const struct key *key, @@ -548,7 +548,7 @@ static void fscrypt_provisioning_key_describe(const struct key *key, static void fscrypt_provisioning_key_destroy(struct key *key) { - kzfree(key->payload.data[0]); + kfree_sensitive(key->payload.data[0]); } static struct key_type key_type_fscrypt_provisioning = { -- cgit