Em 27/08/2025 15:06, Amir Goldstein escreveu:
[...]
The reason is this:
static struct dentry *ext4_lookup(...
{
...
if (IS_ENABLED(CONFIG_UNICODE) && !inode && IS_CASEFOLDED(dir)) {
/* Eventually we want to call d_add_ci(dentry, NULL)
* for negative dentries in the encoding case as
* well. For now, prevent the negative dentry
* from being cached.
*/
return NULL;
}
return d_splice_alias(inode, dentry);
}
Neil,
Apparently, the assumption that
ovl_lookup_temp() => ovl_lookup_upper() => lookup_one()
returns a hashed dentry is not always true.
It may be always true for all the filesystems that are currently
supported as an overlayfs
upper layer fs (?), but it does not look like you can count on this
for the wider vfs effort
and we should try to come up with a solution for ovl_parent_lock()
that will allow enabling
casefolding on overlayfs layers.
This patch seems to work. WDYT?
Thanks,
Amir.
Thank you for the fix!
commit 5dfcd10378038637648f3f422e3d5097eb6faa5f
Author: Amir Goldstein <amir73il@xxxxxxxxx>
Date: Wed Aug 27 19:55:26 2025 +0200
ovl: adapt ovl_parent_lock() to casefolded directories
e8bd877fb76bb9f3 ("ovl: fix possible double unlink") added a sanity
Just to make checkpatch happy, this should be
Commit e8bd877fb76b ("ovl: fix possible double unlink") added a sanity
check of !d_unhashed(child) to try to verify that child dentry was not
unlinked while parent dir was unlocked.
This "was not unlink" check has a false positive result in the case of
casefolded parent dir, because in that case, ovl_create_temp() returns
an unhashed dentry.
Change the "was not unlinked" check to use cant_mount(child).
cant_mount(child) means that child was unlinked while we have been
holding a reference to child, so it could not have become negative.
This fixes the error in ovl_parent_lock() in ovl_check_rename_whiteout()
after ovl_create_temp() and allows mount of overlayfs with casefolding
enabled layers.
Reported-by: André Almeida <andrealmeid@xxxxxxxxxx>
Link: https://lore.kernel.org/r/18704e8c-c734-43f3-bc7c-b8be345e1bf5@xxxxxxxxxx/
I think the correct chain here is:
Reported-by: André Almeida <andrealmeid@xxxxxxxxxx>
Closes:
https://lore.kernel.org/r/18704e8c-c734-43f3-bc7c-b8be345e1bf5@xxxxxxxxxx/
Fixes: e8bd877fb76b ("ovl: fix possible double unlink")
Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
Reviewed-by: André Almeida <andrealmeid@xxxxxxxxxx>
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index bec4a39d1b97c..bffbb59776720 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -1551,9 +1551,23 @@ void ovl_copyattr(struct inode *inode)
int ovl_parent_lock(struct dentry *parent, struct dentry *child)
{
+ bool is_unlinked;
+
inode_lock_nested(parent->d_inode, I_MUTEX_PARENT);
- if (!child ||
- (!d_unhashed(child) && child->d_parent == parent))
+ if (!child)
+ return 0;
+
+ /*
+ * After re-acquiring parent dir lock, verify that child was not moved
+ * to another parent and that it was not unlinked. cant_mount() means
+ * that child was unlinked while parent was unlocked. Since we are
+ * holding a reference to child, it could not have become negative.
+ * d_unhashed(child) is not a strong enough indication for unlinked,
+ * because with casefolded parent dir, ovl_create_temp() returns an
+ * unhashed dentry.
+ */
+ is_unlinked = cant_mount(child) || WARN_ON_ONCE(d_is_negative(child));
+ if (!is_unlinked && child->d_parent == parent)
return 0;
inode_unlock(parent->d_inode);
