Backport four commits from v5.11-rc1. I resolved conflicts in the first two.
Eric Biggers (4): fscrypt: add fscrypt_is_nokey_name() ext4: prevent creating duplicate encrypted filenames f2fs: prevent creating duplicate encrypted filenames ubifs: prevent creating duplicate encrypted filenames
fs/crypto/hooks.c | 10 +++++----- fs/ext4/namei.c | 3 +++ fs/f2fs/f2fs.h | 2 ++ fs/ubifs/dir.c | 17 +++++++++++++---- include/linux/fscrypt_notsupp.h | 5 +++++ include/linux/fscrypt_supp.h | 29 +++++++++++++++++++++++++++++ 6 files changed, 57 insertions(+), 9 deletions(-)
From: Eric Biggers ebiggers@google.com
commit 159e1de201b6fca10bfec50405a3b53a561096a8 upstream.
It's possible to create a duplicate filename in an encrypted directory by creating a file concurrently with adding the encryption key.
Specifically, sys_open(O_CREAT) (or sys_mkdir(), sys_mknod(), or sys_symlink()) can lookup the target filename while the directory's encryption key hasn't been added yet, resulting in a negative no-key dentry. The VFS then calls ->create() (or ->mkdir(), ->mknod(), or ->symlink()) because the dentry is negative. Normally, ->create() would return -ENOKEY due to the directory's key being unavailable. However, if the key was added between the dentry lookup and ->create(), then the filesystem will go ahead and try to create the file.
If the target filename happens to already exist as a normal name (not a no-key name), a duplicate filename may be added to the directory.
In order to fix this, we need to fix the filesystems to prevent ->create(), ->mkdir(), ->mknod(), and ->symlink() on no-key names. (->rename() and ->link() need it too, but those are already handled correctly by fscrypt_prepare_rename() and fscrypt_prepare_link().)
In preparation for this, add a helper function fscrypt_is_nokey_name() that filesystems can use to do this check. Use this helper function for the existing checks that fs/crypto/ does for rename and link.
Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20201118075609.120337-2-ebiggers@kernel.org Signed-off-by: Eric Biggers ebiggers@google.com --- fs/crypto/hooks.c | 10 +++++----- include/linux/fscrypt_notsupp.h | 5 +++++ include/linux/fscrypt_supp.h | 29 +++++++++++++++++++++++++++++ 3 files changed, 39 insertions(+), 5 deletions(-)
diff --git a/fs/crypto/hooks.c b/fs/crypto/hooks.c index 042d5b44f4ed9..aa86cb2db8236 100644 --- a/fs/crypto/hooks.c +++ b/fs/crypto/hooks.c @@ -58,8 +58,8 @@ 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) + /* ... in case we looked up no-key name before key was added */ + if (fscrypt_is_nokey_name(dentry)) return -ENOKEY;
if (!fscrypt_has_permitted_context(dir, inode)) @@ -83,9 +83,9 @@ 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) + /* ... in case we looked up no-key name(s) before key was added */ + if (fscrypt_is_nokey_name(old_dentry) || + fscrypt_is_nokey_name(new_dentry)) return -ENOKEY;
if (old_dir != new_dir) { diff --git a/include/linux/fscrypt_notsupp.h b/include/linux/fscrypt_notsupp.h index 24b261e49dc14..93304cfeb6016 100644 --- a/include/linux/fscrypt_notsupp.h +++ b/include/linux/fscrypt_notsupp.h @@ -24,6 +24,11 @@ static inline bool fscrypt_dummy_context_enabled(struct inode *inode) return false; }
+static inline bool fscrypt_is_nokey_name(const struct dentry *dentry) +{ + return false; +} + /* crypto.c */ static inline void fscrypt_enqueue_decrypt_work(struct work_struct *work) { diff --git a/include/linux/fscrypt_supp.h b/include/linux/fscrypt_supp.h index 8641e20694ce9..0409c14ae1de4 100644 --- a/include/linux/fscrypt_supp.h +++ b/include/linux/fscrypt_supp.h @@ -58,6 +58,35 @@ static inline bool fscrypt_dummy_context_enabled(struct inode *inode) inode->i_sb->s_cop->dummy_context(inode); }
+/** + * fscrypt_is_nokey_name() - test whether a dentry is a no-key name + * @dentry: the dentry to check + * + * This returns true if the dentry is a no-key dentry. A no-key dentry is a + * dentry that was created in an encrypted directory that hasn't had its + * encryption key added yet. Such dentries may be either positive or negative. + * + * When a filesystem is asked to create a new filename in an encrypted directory + * and the new filename's dentry is a no-key dentry, it must fail the operation + * with ENOKEY. This includes ->create(), ->mkdir(), ->mknod(), ->symlink(), + * ->rename(), and ->link(). (However, ->rename() and ->link() are already + * handled by fscrypt_prepare_rename() and fscrypt_prepare_link().) + * + * This is necessary because creating a filename requires the directory's + * encryption key, but just checking for the key on the directory inode during + * the final filesystem operation doesn't guarantee that the key was available + * during the preceding dentry lookup. And the key must have already been + * available during the dentry lookup in order for it to have been checked + * whether the filename already exists in the directory and for the new file's + * dentry not to be invalidated due to it incorrectly having the no-key flag. + * + * Return: %true if the dentry is a no-key name + */ +static inline bool fscrypt_is_nokey_name(const struct dentry *dentry) +{ + return dentry->d_flags & DCACHE_ENCRYPTED_NAME; +} + /* crypto.c */ extern void fscrypt_enqueue_decrypt_work(struct work_struct *); extern struct fscrypt_ctx *fscrypt_get_ctx(const struct inode *, gfp_t);
From: Eric Biggers ebiggers@google.com
commit 75d18cd1868c2aee43553723872c35d7908f240f upstream.
As described in "fscrypt: add fscrypt_is_nokey_name()", it's possible to create a duplicate filename in an encrypted directory by creating a file concurrently with adding the directory's encryption key.
Fix this bug on ext4 by rejecting no-key dentries in ext4_add_entry().
Note that the duplicate check in ext4_find_dest_de() sometimes prevented this bug. However in many cases it didn't, since ext4_find_dest_de() doesn't examine every dentry.
Fixes: 4461471107b7 ("ext4 crypto: enable filename encryption") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20201118075609.120337-3-ebiggers@kernel.org Signed-off-by: Eric Biggers ebiggers@google.com --- fs/ext4/namei.c | 3 +++ 1 file changed, 3 insertions(+)
diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 4191552880bd7..3c238006870d3 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -2106,6 +2106,9 @@ static int ext4_add_entry(handle_t *handle, struct dentry *dentry, if (!dentry->d_name.len) return -EINVAL;
+ if (fscrypt_is_nokey_name(dentry)) + return -ENOKEY; + retval = ext4_fname_setup_filename(dir, &dentry->d_name, 0, &fname); if (retval) return retval;
From: Eric Biggers ebiggers@google.com
commit bfc2b7e8518999003a61f91c1deb5e88ed77b07d upstream.
As described in "fscrypt: add fscrypt_is_nokey_name()", it's possible to create a duplicate filename in an encrypted directory by creating a file concurrently with adding the directory's encryption key.
Fix this bug on f2fs by rejecting no-key dentries in f2fs_add_link().
Note that the weird check for the current task in f2fs_do_add_link() seems to make this bug difficult to reproduce on f2fs.
Fixes: 9ea97163c6da ("f2fs crypto: add filename encryption for f2fs_add_link") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20201118075609.120337-4-ebiggers@kernel.org Signed-off-by: Eric Biggers ebiggers@google.com --- fs/f2fs/f2fs.h | 2 ++ 1 file changed, 2 insertions(+)
diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h index 53ffa6fe207a3..aacd8e11758ca 100644 --- a/fs/f2fs/f2fs.h +++ b/fs/f2fs/f2fs.h @@ -2857,6 +2857,8 @@ bool f2fs_empty_dir(struct inode *dir);
static inline int f2fs_add_link(struct dentry *dentry, struct inode *inode) { + if (fscrypt_is_nokey_name(dentry)) + return -ENOKEY; return f2fs_do_add_link(d_inode(dentry->d_parent), &dentry->d_name, inode, inode->i_ino, inode->i_mode); }
From: Eric Biggers ebiggers@google.com
commit 76786a0f083473de31678bdb259a3d4167cf756d upstream.
As described in "fscrypt: add fscrypt_is_nokey_name()", it's possible to create a duplicate filename in an encrypted directory by creating a file concurrently with adding the directory's encryption key.
Fix this bug on ubifs by rejecting no-key dentries in ubifs_create(), ubifs_mkdir(), ubifs_mknod(), and ubifs_symlink().
Note that ubifs doesn't actually report the duplicate filenames from readdir, but rather it seems to replace the original dentry with a new one (which is still wrong, just a different effect from ext4).
On ubifs, this fixes xfstest generic/595 as well as the new xfstest I wrote specifically for this bug.
Fixes: f4f61d2cc6d8 ("ubifs: Implement encrypted filenames") Cc: stable@vger.kernel.org Link: https://lore.kernel.org/r/20201118075609.120337-5-ebiggers@kernel.org Signed-off-by: Eric Biggers ebiggers@google.com --- fs/ubifs/dir.c | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-)
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c index 10aab5dccaee4..8fe2ee5462a0b 100644 --- a/fs/ubifs/dir.c +++ b/fs/ubifs/dir.c @@ -290,6 +290,15 @@ static struct dentry *ubifs_lookup(struct inode *dir, struct dentry *dentry, return d_splice_alias(inode, dentry); }
+static int ubifs_prepare_create(struct inode *dir, struct dentry *dentry, + struct fscrypt_name *nm) +{ + if (fscrypt_is_nokey_name(dentry)) + return -ENOKEY; + + return fscrypt_setup_filename(dir, &dentry->d_name, 0, nm); +} + static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode, bool excl) { @@ -313,7 +322,7 @@ static int ubifs_create(struct inode *dir, struct dentry *dentry, umode_t mode, if (err) return err;
- err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm); + err = ubifs_prepare_create(dir, dentry, &nm); if (err) goto out_budg;
@@ -977,7 +986,7 @@ static int ubifs_mkdir(struct inode *dir, struct dentry *dentry, umode_t mode) if (err) return err;
- err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm); + err = ubifs_prepare_create(dir, dentry, &nm); if (err) goto out_budg;
@@ -1062,7 +1071,7 @@ static int ubifs_mknod(struct inode *dir, struct dentry *dentry, return err; }
- err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm); + err = ubifs_prepare_create(dir, dentry, &nm); if (err) { kfree(dev); goto out_budg; @@ -1146,7 +1155,7 @@ static int ubifs_symlink(struct inode *dir, struct dentry *dentry, if (err) return err;
- err = fscrypt_setup_filename(dir, &dentry->d_name, 0, &nm); + err = ubifs_prepare_create(dir, dentry, &nm); if (err) goto out_budg;
On Mon, Dec 28, 2020 at 11:12:07AM -0800, Eric Biggers wrote:
Backport four commits from v5.11-rc1. I resolved conflicts in the first two.
Eric Biggers (4): fscrypt: add fscrypt_is_nokey_name() ext4: prevent creating duplicate encrypted filenames f2fs: prevent creating duplicate encrypted filenames ubifs: prevent creating duplicate encrypted filenames
fs/crypto/hooks.c | 10 +++++----- fs/ext4/namei.c | 3 +++ fs/f2fs/f2fs.h | 2 ++ fs/ubifs/dir.c | 17 +++++++++++++---- include/linux/fscrypt_notsupp.h | 5 +++++ include/linux/fscrypt_supp.h | 29 +++++++++++++++++++++++++++++ 6 files changed, 57 insertions(+), 9 deletions(-)
-- 2.29.2
All now queued up, thanks.
greg k-h
linux-stable-mirror@lists.linaro.org