summaryrefslogtreecommitdiff
path: root/fs/crypto
diff options
context:
space:
mode:
authorEric Biggers <ebiggers@google.com>2019-03-20 11:39:13 -0700
committerTheodore Ts'o <tytso@mit.edu>2019-04-17 10:07:51 -0400
commitb01531db6cec2aa330dbc91bfbfaaef4a0d387a4 (patch)
tree215e407f512e5b48d4f3269c3ab717f3080fcf51 /fs/crypto
parentd456a33f041af4b54f3ce495a86d00c246165032 (diff)
fscrypt: fix race where ->lookup() marks plaintext dentry as ciphertext
->lookup() in an encrypted directory begins as follows: 1. fscrypt_prepare_lookup(): a. Try to load the directory's encryption key. b. If the key is unavailable, mark the dentry as a ciphertext name via d_flags. 2. fscrypt_setup_filename(): a. Try to load the directory's encryption key. b. If the key is available, encrypt the name (treated as a plaintext name) to get the on-disk name. Otherwise decode the name (treated as a ciphertext name) to get the on-disk name. But if the key is concurrently added, it may be found at (2a) but not at (1a). In this case, the dentry will be wrongly marked as a ciphertext name even though it was actually treated as plaintext. This will cause the dentry to be wrongly invalidated on the next lookup, potentially causing problems. For example, if the racy ->lookup() was part of sys_mount(), then the new mount will be detached when anything tries to access it. This is despite the mountpoint having a plaintext path, which should remain valid now that the key was added. Of course, this is only possible if there's a userspace race. Still, the additional kernel-side race is confusing and unexpected. Close the kernel-side race by changing fscrypt_prepare_lookup() to also set the on-disk filename (step 2b), consistent with the d_flags update. Fixes: 28b4c263961c ("ext4 crypto: revalidate dentry after adding or removing the key") Signed-off-by: Eric Biggers <ebiggers@google.com> Signed-off-by: Theodore Ts'o <tytso@mit.edu>
Diffstat (limited to 'fs/crypto')
-rw-r--r--fs/crypto/fname.c1
-rw-r--r--fs/crypto/hooks.c11
2 files changed, 7 insertions, 5 deletions
diff --git a/fs/crypto/fname.c b/fs/crypto/fname.c
index 050384c79f40..eccea3d8f923 100644
--- a/fs/crypto/fname.c
+++ b/fs/crypto/fname.c
@@ -356,6 +356,7 @@ int fscrypt_setup_filename(struct inode *dir, const struct qstr *iname,
}
if (!lookup)
return -ENOKEY;
+ fname->is_ciphertext_name = true;
/*
* We don't have the key and we are doing a lookup; decode the
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c
index 9d8910e86ee5..042d5b44f4ed 100644
--- a/fs/crypto/hooks.c
+++ b/fs/crypto/hooks.c
@@ -104,20 +104,21 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry,
}
EXPORT_SYMBOL_GPL(__fscrypt_prepare_rename);
-int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry)
+int __fscrypt_prepare_lookup(struct inode *dir, struct dentry *dentry,
+ struct fscrypt_name *fname)
{
- int err = fscrypt_get_encryption_info(dir);
+ int err = fscrypt_setup_filename(dir, &dentry->d_name, 1, fname);
- if (err)
+ if (err && err != -ENOENT)
return err;
- if (!fscrypt_has_encryption_key(dir)) {
+ if (fname->is_ciphertext_name) {
spin_lock(&dentry->d_lock);
dentry->d_flags |= DCACHE_ENCRYPTED_NAME;
spin_unlock(&dentry->d_lock);
d_set_d_op(dentry, &fscrypt_d_ops);
}
- return 0;
+ return err;
}
EXPORT_SYMBOL_GPL(__fscrypt_prepare_lookup);