fscrypt: fix race allowing rename() and link() of ciphertext dentries
authorEric Biggers <ebiggers@google.com>
Wed, 20 Mar 2019 18:39:10 +0000 (11:39 -0700)
committerTheodore Ts'o <tytso@mit.edu>
Wed, 17 Apr 2019 13:51:20 +0000 (09:51 -0400)
Close some race conditions where fscrypt allowed rename() and link() on
ciphertext dentries that had been looked up just prior to the key being
concurrently added.  It's better to return -ENOKEY in this case.

This avoids doing the nonsensical thing of encrypting the names a second
time when searching for the actual on-disk dir entries.  It also
guarantees that DCACHE_ENCRYPTED_NAME dentries are never rename()d, so
the dcache won't have support all possible combinations of moving
DCACHE_ENCRYPTED_NAME around during __d_move().

Signed-off-by: Eric Biggers <ebiggers@google.com>
Signed-off-by: Theodore Ts'o <tytso@mit.edu>
fs/crypto/hooks.c
include/linux/fscrypt.h

index a9492f7..2e7498a 100644 (file)
@@ -49,7 +49,8 @@ int fscrypt_file_open(struct inode *inode, struct file *filp)
 }
 EXPORT_SYMBOL_GPL(fscrypt_file_open);
 
-int __fscrypt_prepare_link(struct inode *inode, struct inode *dir)
+int __fscrypt_prepare_link(struct inode *inode, struct inode *dir,
+                          struct dentry *dentry)
 {
        int err;
 
@@ -57,6 +58,10 @@ int __fscrypt_prepare_link(struct inode *inode, struct inode *dir)
        if (err)
                return err;
 
+       /* ... in case we looked up ciphertext name before key was added */
+       if (dentry->d_flags & DCACHE_ENCRYPTED_NAME)
+               return -ENOKEY;
+
        if (!fscrypt_has_permitted_context(dir, inode))
                return -EXDEV;
 
@@ -78,6 +83,11 @@ int __fscrypt_prepare_rename(struct inode *old_dir, struct dentry *old_dentry,
        if (err)
                return err;
 
+       /* ... in case we looked up ciphertext name(s) before key was added */
+       if ((old_dentry->d_flags | new_dentry->d_flags) &
+           DCACHE_ENCRYPTED_NAME)
+               return -ENOKEY;
+
        if (old_dir != new_dir) {
                if (IS_ENCRYPTED(new_dir) &&
                    !fscrypt_has_permitted_context(new_dir,
index 09e368a..855f743 100644 (file)
@@ -215,7 +215,8 @@ extern int fscrypt_zeroout_range(const struct inode *, pgoff_t, sector_t,
 
 /* hooks.c */
 extern int fscrypt_file_open(struct inode *inode, struct file *filp);
-extern int __fscrypt_prepare_link(struct inode *inode, struct inode *dir);
+extern int __fscrypt_prepare_link(struct inode *inode, struct inode *dir,
+                                 struct dentry *dentry);
 extern int __fscrypt_prepare_rename(struct inode *old_dir,
                                    struct dentry *old_dentry,
                                    struct inode *new_dir,
@@ -401,8 +402,8 @@ static inline int fscrypt_file_open(struct inode *inode, struct file *filp)
        return 0;
 }
 
-static inline int __fscrypt_prepare_link(struct inode *inode,
-                                        struct inode *dir)
+static inline int __fscrypt_prepare_link(struct inode *inode, struct inode *dir,
+                                        struct dentry *dentry)
 {
        return -EOPNOTSUPP;
 }
@@ -497,7 +498,7 @@ static inline int fscrypt_prepare_link(struct dentry *old_dentry,
                                       struct dentry *dentry)
 {
        if (IS_ENCRYPTED(dir))
-               return __fscrypt_prepare_link(d_inode(old_dentry), dir);
+               return __fscrypt_prepare_link(d_inode(old_dentry), dir, dentry);
        return 0;
 }