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>

1 files changed, 47 insertions, 15 deletions

diff --git a/fs/ext4/ext4.h b/fs/ext4/ext4.h
index 82ffdacdc7fa..e64a4ee96d30 100644
--- a/fs/ext4/ext4.h
+++ b/fs/ext4/ext4.h
@@ -2287,23 +2287,47 @@ extern unsigned ext4_free_clusters_after_init(struct super_block *sb,
 ext4_fsblk_t ext4_inode_to_goal_block(struct inode *);
 
 #ifdef CONFIG_FS_ENCRYPTION
+static inline void ext4_fname_from_fscrypt_name(struct ext4_filename *dst,
+                                                const struct fscrypt_name *src)
+{
+        memset(dst, 0, sizeof(*dst));
+
+        dst->usr_fname = src->usr_fname;
+        dst->disk_name = src->disk_name;
+        dst->hinfo.hash = src->hash;
+        dst->hinfo.minor_hash = src->minor_hash;
+        dst->crypto_buf = src->crypto_buf;
+}
+
 static inline int ext4_fname_setup_filename(struct inode *dir,
-                        const struct qstr *iname,
-                        int lookup, struct ext4_filename *fname)
+                                            const struct qstr *iname,
+                                            int lookup,
+                                            struct ext4_filename *fname)
 {
         struct fscrypt_name name;
         int err;
 
-        memset(fname, 0, sizeof(struct ext4_filename));
-
         err = fscrypt_setup_filename(dir, iname, lookup, &name);
+        if (err)
+                return err;
 
-        fname->usr_fname = name.usr_fname;
-        fname->disk_name = name.disk_name;
-        fname->hinfo.hash = name.hash;
-        fname->hinfo.minor_hash = name.minor_hash;
-        fname->crypto_buf = name.crypto_buf;
-        return err;
+        ext4_fname_from_fscrypt_name(fname, &name);
+        return 0;
+}
+
+static inline int ext4_fname_prepare_lookup(struct inode *dir,
+                                            struct dentry *dentry,
+                                            struct ext4_filename *fname)
+{
+        struct fscrypt_name name;
+        int err;
+
+        err = fscrypt_prepare_lookup(dir, dentry, &name);
+        if (err)
+                return err;
+
+        ext4_fname_from_fscrypt_name(fname, &name);
+        return 0;
 }
 
 static inline void ext4_fname_free_filename(struct ext4_filename *fname)
@@ -2317,19 +2341,27 @@ static inline void ext4_fname_free_filename(struct ext4_filename *fname)
         fname->usr_fname = NULL;
         fname->disk_name.name = NULL;
 }
-#else
+#else /* !CONFIG_FS_ENCRYPTION */
 static inline int ext4_fname_setup_filename(struct inode *dir,
-                const struct qstr *iname,
-                int lookup, struct ext4_filename *fname)
+                                            const struct qstr *iname,
+                                            int lookup,
+                                            struct ext4_filename *fname)
 {
         fname->usr_fname = iname;
         fname->disk_name.name = (unsigned char *) iname->name;
         fname->disk_name.len = iname->len;
         return 0;
 }
-static inline void ext4_fname_free_filename(struct ext4_filename *fname) { }
 
-#endif
+static inline int ext4_fname_prepare_lookup(struct inode *dir,
+                                            struct dentry *dentry,
+                                            struct ext4_filename *fname)
+{
+        return ext4_fname_setup_filename(dir, &dentry->d_name, 1, fname);
+}
+
+static inline void ext4_fname_free_filename(struct ext4_filename *fname) { }
+#endif /* !CONFIG_FS_ENCRYPTION */
 
 /* dir.c */
 extern int __ext4_check_dir_entry(const char *, unsigned int, struct inode *,