| From ec772f01307a2c06ebf6cdd221e6b518a71ddae7 Mon Sep 17 00:00:00 2001 |
| From: Eric Biggers <ebiggers@google.com> |
| Date: Thu, 23 Jan 2020 20:12:34 -0800 |
| Subject: ext4: fix race conditions in ->d_compare() and ->d_hash() |
| |
| From: Eric Biggers <ebiggers@google.com> |
| |
| commit ec772f01307a2c06ebf6cdd221e6b518a71ddae7 upstream. |
| |
| Since ->d_compare() and ->d_hash() can be called in RCU-walk mode, |
| ->d_parent and ->d_inode can be concurrently modified, and in |
| particular, ->d_inode may be changed to NULL. For ext4_d_hash() this |
| resulted in a reproducible NULL dereference if a lookup is done in a |
| directory being deleted, e.g. with: |
| |
| int main() |
| { |
| if (fork()) { |
| for (;;) { |
| mkdir("subdir", 0700); |
| rmdir("subdir"); |
| } |
| } else { |
| for (;;) |
| access("subdir/file", 0); |
| } |
| } |
| |
| ... or by running the 't_encrypted_d_revalidate' program from xfstests. |
| Both repros work in any directory on a filesystem with the encoding |
| feature, even if the directory doesn't actually have the casefold flag. |
| |
| I couldn't reproduce a crash in ext4_d_compare(), but it appears that a |
| similar crash is possible there. |
| |
| Fix these bugs by reading ->d_parent and ->d_inode using READ_ONCE() and |
| falling back to the case sensitive behavior if the inode is NULL. |
| |
| Reported-by: Al Viro <viro@zeniv.linux.org.uk> |
| Fixes: b886ee3e778e ("ext4: Support case-insensitive file name lookups") |
| Cc: <stable@vger.kernel.org> # v5.2+ |
| Signed-off-by: Eric Biggers <ebiggers@google.com> |
| Link: https://lore.kernel.org/r/20200124041234.159740-1-ebiggers@kernel.org |
| Signed-off-by: Theodore Ts'o <tytso@mit.edu> |
| Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> |
| |
| --- |
| fs/ext4/dir.c | 9 ++++++--- |
| 1 file changed, 6 insertions(+), 3 deletions(-) |
| |
| --- a/fs/ext4/dir.c |
| +++ b/fs/ext4/dir.c |
| @@ -672,9 +672,11 @@ static int ext4_d_compare(const struct d |
| const char *str, const struct qstr *name) |
| { |
| struct qstr qstr = {.name = str, .len = len }; |
| - struct inode *inode = dentry->d_parent->d_inode; |
| + const struct dentry *parent = READ_ONCE(dentry->d_parent); |
| + const struct inode *inode = READ_ONCE(parent->d_inode); |
| |
| - if (!IS_CASEFOLDED(inode) || !EXT4_SB(inode->i_sb)->s_encoding) { |
| + if (!inode || !IS_CASEFOLDED(inode) || |
| + !EXT4_SB(inode->i_sb)->s_encoding) { |
| if (len != name->len) |
| return -1; |
| return memcmp(str, name->name, len); |
| @@ -687,10 +689,11 @@ static int ext4_d_hash(const struct dent |
| { |
| const struct ext4_sb_info *sbi = EXT4_SB(dentry->d_sb); |
| const struct unicode_map *um = sbi->s_encoding; |
| + const struct inode *inode = READ_ONCE(dentry->d_inode); |
| unsigned char *norm; |
| int len, ret = 0; |
| |
| - if (!IS_CASEFOLDED(dentry->d_inode) || !um) |
| + if (!inode || !IS_CASEFOLDED(inode) || !um) |
| return 0; |
| |
| norm = kmalloc(PATH_MAX, GFP_ATOMIC); |