Hello,
This patchset fixes a bug in case-insensitive directories. When I
submitted a patchset for adding case-insensitive support for tmpfs[0],
Al Viro noted that my implementation didn't take in account previous
dentries that the directory could have created before being changed.
Further investigation showed that neither ext4 or f2fs also doesn't take
this case in consideration as well.
* Why can't we have negative dentries with casefold?
The assumption that the directory has no dentries can lead to a buggy
behavior (note that since the directory must be empty when setting the
casefold flag, all dentries there are negative). Imagine the following
operation on a mounted ext4 with casefold support enabled:
mkdir dir
mkdir dir/C # creates a dentry for `C` (dentry D)
rm -r dir/C # makes dentry D a negative one
Now, let's make it case-insensitive:
chattr +F dir/ # now dir/ is a casefold directory
mkdir dir/c # if hash for `c` collides with dentry D
# d_compare does a case-insensitive compare
# and assumes that dentry D is the one to be used
ls dir/ # VFS uses the name at dentry D for the final file
C # and here's the bug
In that way, all negative dentries at dir/ will become dangling dentries
that can't be trusted to be used an will just waste memory.
The problem with negative dentries is well-know, and both the current
code and commits documents it, but this case hasn't been taken in
consideration so far.
* Reproducing
Given that the bug only happens with a hash collision, I added the
following snippet at the beginning of generic_ci_d_hash():
str->hash = 0;
return 0;
This means that all dentries will have the same hash. This is not good
for performance, but it should not break anything AFAIK. Then, just run
the example showed in the latter section.
* Fixing
To fix this bug, I added a function that, given an inode, for each alias
of it, will remove all the sub-dentries at that directory. Given that
they are all negative dentries, we don't need to do the whole d_walk,
since they don't have children and are also ready to be d_droped and
dputed.
Then, at ext4 and f2fs, when a dir is going to turn on the casefold
flag, we call this function.
Thanks,
André
[0] https://lore.kernel.org/linux-fsdevel/[email protected]/T/#m3265579197095b792ee8b8e8b7f84a58c25c456b
André Almeida (3):
fs/dcache: Add d_clear_dir_neg_dentries()
ext4: Prevent dangling dentries on casefold directories
f2fs: Prevent dangling dentries on casefold directories
fs/dcache.c | 27 +++++++++++++++++++++++++++
fs/ext4/ioctl.c | 3 +++
fs/f2fs/file.c | 4 ++++
include/linux/dcache.h | 1 +
4 files changed, 35 insertions(+)
--
2.31.0
Before making a folder a case-insensitive one, this folder could have
been used before and created some negative dentries (given that the
folder needs to be empty before making it case-insensitive, all detries
there are negative ones). During a new file creation, if a d_hash()
collision happens and the name matches a negative dentry, the new file
might have a name different than the specified by user.
To prevent this from happening, remove all negative dentries in a
directory before making it a case-folded one.
Fixes: 2c2eb7a300cd ("f2fs: Support case-insensitive file name lookups")
Signed-off-by: André Almeida <[email protected]>
---
fs/f2fs/file.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
index d26ff2ae3f5e..616b7eb43795 100644
--- a/fs/f2fs/file.c
+++ b/fs/f2fs/file.c
@@ -1826,6 +1826,10 @@ static int f2fs_setflags_common(struct inode *inode, u32 iflags, u32 mask)
return -EOPNOTSUPP;
if (!f2fs_empty_dir(inode))
return -ENOTEMPTY;
+
+ if (!(masked_flags & F2FS_CASEFOLD_FL) &&
+ (iflags & F2FS_CASEFOLD_FL))
+ d_clear_dir_neg_dentries(inode);
}
if (iflags & (F2FS_COMPR_FL | F2FS_NOCOMP_FL)) {
--
2.31.0
Before making a folder a case-insensitive one, this folder could have
been used before and created some negative dentries (given that the
folder needs to be empty before making it case-insensitive, all detries
there are negative ones). During a new file creation, if a d_hash()
collision happens and the name matches a negative dentry, the new file
might have a name different than the specified by user.
To prevent this from happening, remove all negative dentries in a
directory before making it a case-folded one.
Fixes: b886ee3e778e ("ext4: Support case-insensitive file name lookups")
Signed-off-by: André Almeida <[email protected]>
---
fs/ext4/ioctl.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/ext4/ioctl.c b/fs/ext4/ioctl.c
index a2cf35066f46..0eede4c93c22 100644
--- a/fs/ext4/ioctl.c
+++ b/fs/ext4/ioctl.c
@@ -381,6 +381,9 @@ static int ext4_ioctl_setflags(struct inode *inode,
err = -ENOTEMPTY;
goto flags_out;
}
+
+ if (!(oldflags & EXT4_CASEFOLD_FL) && (flags & EXT4_CASEFOLD_FL))
+ d_clear_dir_neg_dentries(inode);
}
/*
--
2.31.0