* [PATCH 4.19 0/4] fscrypt: prevent creating duplicate encrypted filenames
@ 2020-12-28 19:12 Eric Biggers
2020-12-28 19:12 ` [PATCH 4.19 1/4] fscrypt: add fscrypt_is_nokey_name() Eric Biggers
` (4 more replies)
0 siblings, 5 replies; 6+ messages in thread
From: Eric Biggers @ 2020-12-28 19:12 UTC (permalink / raw)
To: stable; +Cc: linux-fscrypt, linux-ext4, linux-f2fs-devel, linux-mtd
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
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 4.19 1/4] fscrypt: add fscrypt_is_nokey_name()
2020-12-28 19:12 [PATCH 4.19 0/4] fscrypt: prevent creating duplicate encrypted filenames Eric Biggers
@ 2020-12-28 19:12 ` Eric Biggers
2020-12-28 19:12 ` [PATCH 4.19 2/4] ext4: prevent creating duplicate encrypted filenames Eric Biggers
` (3 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2020-12-28 19:12 UTC (permalink / raw)
To: stable; +Cc: linux-fscrypt, linux-ext4, linux-f2fs-devel, linux-mtd
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);
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4.19 2/4] ext4: prevent creating duplicate encrypted filenames
2020-12-28 19:12 [PATCH 4.19 0/4] fscrypt: prevent creating duplicate encrypted filenames Eric Biggers
2020-12-28 19:12 ` [PATCH 4.19 1/4] fscrypt: add fscrypt_is_nokey_name() Eric Biggers
@ 2020-12-28 19:12 ` Eric Biggers
2020-12-28 19:12 ` [PATCH 4.19 3/4] f2fs: " Eric Biggers
` (2 subsequent siblings)
4 siblings, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2020-12-28 19:12 UTC (permalink / raw)
To: stable; +Cc: linux-fscrypt, linux-ext4, linux-f2fs-devel, linux-mtd
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;
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4.19 3/4] f2fs: prevent creating duplicate encrypted filenames
2020-12-28 19:12 [PATCH 4.19 0/4] fscrypt: prevent creating duplicate encrypted filenames Eric Biggers
2020-12-28 19:12 ` [PATCH 4.19 1/4] fscrypt: add fscrypt_is_nokey_name() Eric Biggers
2020-12-28 19:12 ` [PATCH 4.19 2/4] ext4: prevent creating duplicate encrypted filenames Eric Biggers
@ 2020-12-28 19:12 ` Eric Biggers
2020-12-28 19:12 ` [PATCH 4.19 4/4] ubifs: " Eric Biggers
2020-12-30 15:42 ` [PATCH 4.19 0/4] fscrypt: " Greg KH
4 siblings, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2020-12-28 19:12 UTC (permalink / raw)
To: stable; +Cc: linux-fscrypt, linux-ext4, linux-f2fs-devel, linux-mtd
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);
}
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 4.19 4/4] ubifs: prevent creating duplicate encrypted filenames
2020-12-28 19:12 [PATCH 4.19 0/4] fscrypt: prevent creating duplicate encrypted filenames Eric Biggers
` (2 preceding siblings ...)
2020-12-28 19:12 ` [PATCH 4.19 3/4] f2fs: " Eric Biggers
@ 2020-12-28 19:12 ` Eric Biggers
2020-12-30 15:42 ` [PATCH 4.19 0/4] fscrypt: " Greg KH
4 siblings, 0 replies; 6+ messages in thread
From: Eric Biggers @ 2020-12-28 19:12 UTC (permalink / raw)
To: stable; +Cc: linux-fscrypt, linux-ext4, linux-f2fs-devel, linux-mtd
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;
--
2.29.2
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 4.19 0/4] fscrypt: prevent creating duplicate encrypted filenames
2020-12-28 19:12 [PATCH 4.19 0/4] fscrypt: prevent creating duplicate encrypted filenames Eric Biggers
` (3 preceding siblings ...)
2020-12-28 19:12 ` [PATCH 4.19 4/4] ubifs: " Eric Biggers
@ 2020-12-30 15:42 ` Greg KH
4 siblings, 0 replies; 6+ messages in thread
From: Greg KH @ 2020-12-30 15:42 UTC (permalink / raw)
To: Eric Biggers
Cc: stable, linux-fscrypt, linux-ext4, linux-f2fs-devel, linux-mtd
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
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2020-12-30 15:41 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-12-28 19:12 [PATCH 4.19 0/4] fscrypt: prevent creating duplicate encrypted filenames Eric Biggers
2020-12-28 19:12 ` [PATCH 4.19 1/4] fscrypt: add fscrypt_is_nokey_name() Eric Biggers
2020-12-28 19:12 ` [PATCH 4.19 2/4] ext4: prevent creating duplicate encrypted filenames Eric Biggers
2020-12-28 19:12 ` [PATCH 4.19 3/4] f2fs: " Eric Biggers
2020-12-28 19:12 ` [PATCH 4.19 4/4] ubifs: " Eric Biggers
2020-12-30 15:42 ` [PATCH 4.19 0/4] fscrypt: " Greg KH
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).