mbox series

[0/5] fscrypt: prevent creating duplicate encrypted filenames

Message ID 20201118075609.120337-1-ebiggers@kernel.org
Headers show
Series fscrypt: prevent creating duplicate encrypted filenames | expand

Message

Eric Biggers Nov. 18, 2020, 7:56 a.m. UTC
This series fixes a longstanding race condition where a duplicate
filename can be created in an encrypted directory if a syscall that
creates a new filename (e.g. open() or mkdir()) races with the
directory's encryption key being added.

To close this race, we need to prevent creating files if the dentry is
still marked as a no-key name.  I.e. we need to fail the ->create() (or
other operation that creates a new filename) if the key wasn't available
when doing the dentry lookup earlier in the syscall, even if the key was
concurrently added between the dentry lookup and ->create().

See patch 1 for a more detailed explanation.

Patch 1 introduces a helper function required for the fix.  Patches 2-4
fix the bug on ext4, f2fs, and ubifs.  Patch 5 is a cleanup.

This fixes xfstest generic/595 on ubifs, but that test was hitting this
bug only accidentally.  I've also written a new xfstest which reproduces
this bug on both ext4 and ubifs.

Eric Biggers (5):
  fscrypt: add fscrypt_is_nokey_name()
  ext4: prevent creating duplicate encrypted filenames
  f2fs: prevent creating duplicate encrypted filenames
  ubifs: prevent creating duplicate encrypted filenames
  fscrypt: remove unnecessary calls to fscrypt_require_key()

 fs/crypto/hooks.c       | 31 +++++++++++--------------------
 fs/ext4/namei.c         |  3 +++
 fs/f2fs/f2fs.h          |  2 ++
 fs/ubifs/dir.c          | 17 +++++++++++++----
 include/linux/fscrypt.h | 37 +++++++++++++++++++++++++++++++++++--
 5 files changed, 64 insertions(+), 26 deletions(-)


base-commit: 3ceb6543e9cf6ed87cc1fbc6f23ca2db903564cd

Comments

Eric Biggers Nov. 24, 2020, 11:28 p.m. UTC | #1
On Tue, Nov 17, 2020 at 11:56:04PM -0800, Eric Biggers wrote:
> This series fixes a longstanding race condition where a duplicate
> filename can be created in an encrypted directory if a syscall that
> creates a new filename (e.g. open() or mkdir()) races with the
> directory's encryption key being added.
> 
> To close this race, we need to prevent creating files if the dentry is
> still marked as a no-key name.  I.e. we need to fail the ->create() (or
> other operation that creates a new filename) if the key wasn't available
> when doing the dentry lookup earlier in the syscall, even if the key was
> concurrently added between the dentry lookup and ->create().
> 
> See patch 1 for a more detailed explanation.
> 
> Patch 1 introduces a helper function required for the fix.  Patches 2-4
> fix the bug on ext4, f2fs, and ubifs.  Patch 5 is a cleanup.
> 
> This fixes xfstest generic/595 on ubifs, but that test was hitting this
> bug only accidentally.  I've also written a new xfstest which reproduces
> this bug on both ext4 and ubifs.
> 
> Eric Biggers (5):
>   fscrypt: add fscrypt_is_nokey_name()
>   ext4: prevent creating duplicate encrypted filenames
>   f2fs: prevent creating duplicate encrypted filenames
>   ubifs: prevent creating duplicate encrypted filenames
>   fscrypt: remove unnecessary calls to fscrypt_require_key()
> 
>  fs/crypto/hooks.c       | 31 +++++++++++--------------------
>  fs/ext4/namei.c         |  3 +++
>  fs/f2fs/f2fs.h          |  2 ++
>  fs/ubifs/dir.c          | 17 +++++++++++++----
>  include/linux/fscrypt.h | 37 +++++++++++++++++++++++++++++++++++--
>  5 files changed, 64 insertions(+), 26 deletions(-)
> 
> 
> base-commit: 3ceb6543e9cf6ed87cc1fbc6f23ca2db903564cd

All applied to fscrypt.git#master for 5.11.

I'd still appreciate acks for ext4, f2fs, and ubifs though.

- Eric