From 1d6217a4f9905917ee63315c8ea3d63833792f51 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Wed, 8 Jul 2020 14:57:22 -0700 Subject: fscrypt: rename FS_KEY_DERIVATION_NONCE_SIZE The name "FS_KEY_DERIVATION_NONCE_SIZE" is a bit outdated since due to the addition of FSCRYPT_POLICY_FLAG_DIRECT_KEY, the file nonce may now be used as a tweak instead of for key derivation. Also, we're now prefixing the fscrypt constants with "FSCRYPT_" instead of "FS_". Therefore, rename this constant to FSCRYPT_FILE_NONCE_SIZE. Link: https://lore.kernel.org/r/20200708215722.147154-1-ebiggers@kernel.org Signed-off-by: Eric Biggers --- fs/crypto/policy.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/crypto/policy.c') diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c index d23ff162c78b..8a8ad0e44bb8 100644 --- a/fs/crypto/policy.c +++ b/fs/crypto/policy.c @@ -529,7 +529,7 @@ int fscrypt_ioctl_get_nonce(struct file *filp, void __user *arg) if (!fscrypt_context_is_valid(&ctx, ret)) return -EINVAL; if (copy_to_user(arg, fscrypt_context_nonce(&ctx), - FS_KEY_DERIVATION_NONCE_SIZE)) + FSCRYPT_FILE_NONCE_SIZE)) return -EFAULT; return 0; } -- cgit From f000223c981a7c75f6f3ab7288f0be7b571c3644 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 21 Jul 2020 11:10:12 -0700 Subject: fscrypt: restrict IV_INO_LBLK_* to AES-256-XTS IV_INO_LBLK_* exist only because of hardware limitations, and currently the only known use case for them involves AES-256-XTS. Therefore, for now only allow them in combination with AES-256-XTS. This way we don't have to worry about them being combined with other encryption modes. (To be clear, combining IV_INO_LBLK_* with other encryption modes *should* work just fine. It's just not being tested, so we can't be 100% sure it works. So with no known use case, it's best to disallow it for now, just like we don't allow other weird combinations like AES-256-XTS contents encryption with Adiantum filenames encryption.) This can be relaxed later if a use case for other combinations arises. Fixes: b103fb7653ff ("fscrypt: add support for IV_INO_LBLK_64 policies") Fixes: e3b1078bedd3 ("fscrypt: add support for IV_INO_LBLK_32 policies") Link: https://lore.kernel.org/r/20200721181012.39308-1-ebiggers@kernel.org Signed-off-by: Eric Biggers --- fs/crypto/policy.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) (limited to 'fs/crypto/policy.c') diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c index 8a8ad0e44bb8..8e667aadf271 100644 --- a/fs/crypto/policy.c +++ b/fs/crypto/policy.c @@ -77,6 +77,20 @@ static bool supported_iv_ino_lblk_policy(const struct fscrypt_policy_v2 *policy, struct super_block *sb = inode->i_sb; int ino_bits = 64, lblk_bits = 64; + /* + * IV_INO_LBLK_* exist only because of hardware limitations, and + * currently the only known use case for them involves AES-256-XTS. + * That's also all we test currently. For these reasons, for now only + * allow AES-256-XTS here. This can be relaxed later if a use case for + * IV_INO_LBLK_* with other encryption modes arises. + */ + if (policy->contents_encryption_mode != FSCRYPT_MODE_AES_256_XTS) { + fscrypt_warn(inode, + "Can't use %s policy with contents mode other than AES-256-XTS", + type); + return false; + } + /* * It's unsafe to include inode numbers in the IVs if the filesystem can * potentially renumber inodes, e.g. via filesystem shrinking. -- cgit From ab673b987488c4fab7a0bc4824a48211f9d910e3 Mon Sep 17 00:00:00 2001 From: Eric Biggers Date: Tue, 21 Jul 2020 15:59:19 -0700 Subject: fscrypt: use smp_load_acquire() for ->i_crypt_info 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. fscrypt_info includes various sub-objects which are internal to and are allocated by other kernel subsystems such as keyrings and crypto. So by using READ_ONCE() for ->i_crypt_info, we're relying on internal implementation details of these other kernel subsystems. 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: e37a784d8b6a ("fscrypt: use READ_ONCE() to access ->i_crypt_info") Link: https://lore.kernel.org/r/20200721225920.114347-5-ebiggers@kernel.org Signed-off-by: Eric Biggers --- fs/crypto/policy.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/crypto/policy.c') diff --git a/fs/crypto/policy.c b/fs/crypto/policy.c index 8e667aadf271..2d73fd39ad96 100644 --- a/fs/crypto/policy.c +++ b/fs/crypto/policy.c @@ -352,7 +352,7 @@ static int fscrypt_get_policy(struct inode *inode, union fscrypt_policy *policy) union fscrypt_context ctx; int ret; - ci = READ_ONCE(inode->i_crypt_info); + ci = fscrypt_get_info(inode); if (ci) { /* key available, use the cached policy */ *policy = ci->ci_policy; @@ -641,7 +641,7 @@ int fscrypt_inherit_context(struct inode *parent, struct inode *child, if (res < 0) return res; - ci = READ_ONCE(parent->i_crypt_info); + ci = fscrypt_get_info(parent); if (ci == NULL) return -ENOKEY; -- cgit