Drop the restriction for casefold dentries lookup to enable support for
case-insensitive layers in overlayfs.
Support case-insensitive layers with the condition that they should be
uniformly enabled across the stack and (i.e. if the root mount dir has
casefold enabled, so should all the dirs bellow for every layer).
Reviewed-by: Amir Goldstein <amir73il@gmail.com>
Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
Changes from v5:
- Fix mounting layers without casefold flag
---
fs/overlayfs/namei.c | 17 +++++++++--------
fs/overlayfs/util.c | 10 ++++++----
2 files changed, 15 insertions(+), 12 deletions(-)
diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c
index 76d6248b625e7c58e09685e421aef616aadea40a..e93bcc5727bcafdc18a499b47a7609fd41ecaec8 100644
--- a/fs/overlayfs/namei.c
+++ b/fs/overlayfs/namei.c
@@ -239,13 +239,14 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
char val;
/*
- * We allow filesystems that are case-folding capable but deny composing
- * ovl stack from case-folded directories. If someone has enabled case
- * folding on a directory on underlying layer, the warranty of the ovl
- * stack is voided.
+ * We allow filesystems that are case-folding capable as long as the
+ * layers are consistently enabled in the stack, enabled for every dir
+ * or disabled in all dirs. If someone has modified case folding on a
+ * directory on underlying layer, the warranty of the ovl stack is
+ * voided.
*/
- if (ovl_dentry_casefolded(base)) {
- warn = "case folded parent";
+ if (ofs->casefold != ovl_dentry_casefolded(base)) {
+ warn = "parent wrong casefold";
err = -ESTALE;
goto out_warn;
}
@@ -259,8 +260,8 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d,
goto out_err;
}
- if (ovl_dentry_casefolded(this)) {
- warn = "case folded child";
+ if (ofs->casefold != ovl_dentry_casefolded(this)) {
+ warn = "child wrong casefold";
err = -EREMOTE;
goto out_warn;
}
diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c
index a33115e7384c129c543746326642813add63f060..52582b1da52598fbb14866f8c33eb27e36adda36 100644
--- a/fs/overlayfs/util.c
+++ b/fs/overlayfs/util.c
@@ -203,6 +203,8 @@ void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry,
bool ovl_dentry_weird(struct dentry *dentry)
{
+ struct ovl_fs *ofs = OVL_FS(dentry->d_sb);
+
if (!d_can_lookup(dentry) && !d_is_file(dentry) && !d_is_symlink(dentry))
return true;
@@ -210,11 +212,11 @@ bool ovl_dentry_weird(struct dentry *dentry)
return true;
/*
- * Allow filesystems that are case-folding capable but deny composing
- * ovl stack from case-folded directories.
+ * Exceptionally for layers with casefold, we accept that they have
+ * their own hash and compare operations
*/
- if (sb_has_encoding(dentry->d_sb))
- return IS_CASEFOLDED(d_inode(dentry));
+ if (ofs->casefold)
+ return false;
return dentry->d_flags & (DCACHE_OP_HASH | DCACHE_OP_COMPARE);
}
--
2.50.1
On Fri, Aug 22, 2025 at 4:17 PM André Almeida <andrealmeid@igalia.com> wrote: > > Drop the restriction for casefold dentries lookup to enable support for > case-insensitive layers in overlayfs. > > Support case-insensitive layers with the condition that they should be > uniformly enabled across the stack and (i.e. if the root mount dir has > casefold enabled, so should all the dirs bellow for every layer). > > Reviewed-by: Amir Goldstein <amir73il@gmail.com> > Signed-off-by: André Almeida <andrealmeid@igalia.com> > --- > Changes from v5: > - Fix mounting layers without casefold flag > --- > fs/overlayfs/namei.c | 17 +++++++++-------- > fs/overlayfs/util.c | 10 ++++++---- > 2 files changed, 15 insertions(+), 12 deletions(-) > > diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c > index 76d6248b625e7c58e09685e421aef616aadea40a..e93bcc5727bcafdc18a499b47a7609fd41ecaec8 100644 > --- a/fs/overlayfs/namei.c > +++ b/fs/overlayfs/namei.c > @@ -239,13 +239,14 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d, > char val; > > /* > - * We allow filesystems that are case-folding capable but deny composing > - * ovl stack from case-folded directories. If someone has enabled case > - * folding on a directory on underlying layer, the warranty of the ovl > - * stack is voided. > + * We allow filesystems that are case-folding capable as long as the > + * layers are consistently enabled in the stack, enabled for every dir > + * or disabled in all dirs. If someone has modified case folding on a > + * directory on underlying layer, the warranty of the ovl stack is > + * voided. > */ > - if (ovl_dentry_casefolded(base)) { > - warn = "case folded parent"; > + if (ofs->casefold != ovl_dentry_casefolded(base)) { > + warn = "parent wrong casefold"; > err = -ESTALE; > goto out_warn; > } > @@ -259,8 +260,8 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d, > goto out_err; > } > > - if (ovl_dentry_casefolded(this)) { > - warn = "case folded child"; > + if (ofs->casefold != ovl_dentry_casefolded(this)) { > + warn = "child wrong casefold"; > err = -EREMOTE; > goto out_warn; > } > diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c > index a33115e7384c129c543746326642813add63f060..52582b1da52598fbb14866f8c33eb27e36adda36 100644 > --- a/fs/overlayfs/util.c > +++ b/fs/overlayfs/util.c > @@ -203,6 +203,8 @@ void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry, > > bool ovl_dentry_weird(struct dentry *dentry) > { > + struct ovl_fs *ofs = OVL_FS(dentry->d_sb); > + > if (!d_can_lookup(dentry) && !d_is_file(dentry) && !d_is_symlink(dentry)) > return true; > > @@ -210,11 +212,11 @@ bool ovl_dentry_weird(struct dentry *dentry) > return true; > > /* > - * Allow filesystems that are case-folding capable but deny composing > - * ovl stack from case-folded directories. > + * Exceptionally for layers with casefold, we accept that they have > + * their own hash and compare operations > */ > - if (sb_has_encoding(dentry->d_sb)) > - return IS_CASEFOLDED(d_inode(dentry)); > + if (ofs->casefold) > + return false; I think this is better as: if (sb_has_encoding(dentry->d_sb)) return false; I don't think there is a reason to test ofs->casefold here. a "weird" dentry is one that overlayfs doesn't know how to handle. Now it known how to handle dentries with hash()/compare() on casefolding capable filesysytems. Can you please push v6 after this fix to your gitlab branch? Thanks, Amir.
Em 22/08/2025 13:34, Amir Goldstein escreveu: > On Fri, Aug 22, 2025 at 4:17 PM André Almeida <andrealmeid@igalia.com> wrote: >> >> Drop the restriction for casefold dentries lookup to enable support for >> case-insensitive layers in overlayfs. >> >> Support case-insensitive layers with the condition that they should be >> uniformly enabled across the stack and (i.e. if the root mount dir has >> casefold enabled, so should all the dirs bellow for every layer). >> >> Reviewed-by: Amir Goldstein <amir73il@gmail.com> >> Signed-off-by: André Almeida <andrealmeid@igalia.com> >> --- >> Changes from v5: >> - Fix mounting layers without casefold flag >> --- >> fs/overlayfs/namei.c | 17 +++++++++-------- >> fs/overlayfs/util.c | 10 ++++++---- >> 2 files changed, 15 insertions(+), 12 deletions(-) >> >> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c >> index 76d6248b625e7c58e09685e421aef616aadea40a..e93bcc5727bcafdc18a499b47a7609fd41ecaec8 100644 >> --- a/fs/overlayfs/namei.c >> +++ b/fs/overlayfs/namei.c >> @@ -239,13 +239,14 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d, >> char val; >> >> /* >> - * We allow filesystems that are case-folding capable but deny composing >> - * ovl stack from case-folded directories. If someone has enabled case >> - * folding on a directory on underlying layer, the warranty of the ovl >> - * stack is voided. >> + * We allow filesystems that are case-folding capable as long as the >> + * layers are consistently enabled in the stack, enabled for every dir >> + * or disabled in all dirs. If someone has modified case folding on a >> + * directory on underlying layer, the warranty of the ovl stack is >> + * voided. >> */ >> - if (ovl_dentry_casefolded(base)) { >> - warn = "case folded parent"; >> + if (ofs->casefold != ovl_dentry_casefolded(base)) { >> + warn = "parent wrong casefold"; >> err = -ESTALE; >> goto out_warn; >> } >> @@ -259,8 +260,8 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d, >> goto out_err; >> } >> >> - if (ovl_dentry_casefolded(this)) { >> - warn = "case folded child"; >> + if (ofs->casefold != ovl_dentry_casefolded(this)) { >> + warn = "child wrong casefold"; >> err = -EREMOTE; >> goto out_warn; >> } >> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c >> index a33115e7384c129c543746326642813add63f060..52582b1da52598fbb14866f8c33eb27e36adda36 100644 >> --- a/fs/overlayfs/util.c >> +++ b/fs/overlayfs/util.c >> @@ -203,6 +203,8 @@ void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry, >> >> bool ovl_dentry_weird(struct dentry *dentry) >> { >> + struct ovl_fs *ofs = OVL_FS(dentry->d_sb); >> + >> if (!d_can_lookup(dentry) && !d_is_file(dentry) && !d_is_symlink(dentry)) >> return true; >> >> @@ -210,11 +212,11 @@ bool ovl_dentry_weird(struct dentry *dentry) >> return true; >> >> /* >> - * Allow filesystems that are case-folding capable but deny composing >> - * ovl stack from case-folded directories. >> + * Exceptionally for layers with casefold, we accept that they have >> + * their own hash and compare operations >> */ >> - if (sb_has_encoding(dentry->d_sb)) >> - return IS_CASEFOLDED(d_inode(dentry)); >> + if (ofs->casefold) >> + return false; > > I think this is better as: > if (sb_has_encoding(dentry->d_sb)) > return false; > > I don't think there is a reason to test ofs->casefold here. > a "weird" dentry is one that overlayfs doesn't know how to > handle. Now it known how to handle dentries with hash()/compare() > on casefolding capable filesysytems. > > Can you please push v6 after this fix to your gitlab branch? > Ok, it's done
On Fri, Aug 22, 2025 at 6:47 PM André Almeida <andrealmeid@igalia.com> wrote: > > Em 22/08/2025 13:34, Amir Goldstein escreveu: > > On Fri, Aug 22, 2025 at 4:17 PM André Almeida <andrealmeid@igalia.com> wrote: > >> > >> Drop the restriction for casefold dentries lookup to enable support for > >> case-insensitive layers in overlayfs. > >> > >> Support case-insensitive layers with the condition that they should be > >> uniformly enabled across the stack and (i.e. if the root mount dir has > >> casefold enabled, so should all the dirs bellow for every layer). > >> > >> Reviewed-by: Amir Goldstein <amir73il@gmail.com> > >> Signed-off-by: André Almeida <andrealmeid@igalia.com> > >> --- > >> Changes from v5: > >> - Fix mounting layers without casefold flag > >> --- > >> fs/overlayfs/namei.c | 17 +++++++++-------- > >> fs/overlayfs/util.c | 10 ++++++---- > >> 2 files changed, 15 insertions(+), 12 deletions(-) > >> > >> diff --git a/fs/overlayfs/namei.c b/fs/overlayfs/namei.c > >> index 76d6248b625e7c58e09685e421aef616aadea40a..e93bcc5727bcafdc18a499b47a7609fd41ecaec8 100644 > >> --- a/fs/overlayfs/namei.c > >> +++ b/fs/overlayfs/namei.c > >> @@ -239,13 +239,14 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d, > >> char val; > >> > >> /* > >> - * We allow filesystems that are case-folding capable but deny composing > >> - * ovl stack from case-folded directories. If someone has enabled case > >> - * folding on a directory on underlying layer, the warranty of the ovl > >> - * stack is voided. > >> + * We allow filesystems that are case-folding capable as long as the > >> + * layers are consistently enabled in the stack, enabled for every dir > >> + * or disabled in all dirs. If someone has modified case folding on a > >> + * directory on underlying layer, the warranty of the ovl stack is > >> + * voided. > >> */ > >> - if (ovl_dentry_casefolded(base)) { > >> - warn = "case folded parent"; > >> + if (ofs->casefold != ovl_dentry_casefolded(base)) { > >> + warn = "parent wrong casefold"; > >> err = -ESTALE; > >> goto out_warn; > >> } > >> @@ -259,8 +260,8 @@ static int ovl_lookup_single(struct dentry *base, struct ovl_lookup_data *d, > >> goto out_err; > >> } > >> > >> - if (ovl_dentry_casefolded(this)) { > >> - warn = "case folded child"; > >> + if (ofs->casefold != ovl_dentry_casefolded(this)) { > >> + warn = "child wrong casefold"; > >> err = -EREMOTE; > >> goto out_warn; > >> } > >> diff --git a/fs/overlayfs/util.c b/fs/overlayfs/util.c > >> index a33115e7384c129c543746326642813add63f060..52582b1da52598fbb14866f8c33eb27e36adda36 100644 > >> --- a/fs/overlayfs/util.c > >> +++ b/fs/overlayfs/util.c > >> @@ -203,6 +203,8 @@ void ovl_dentry_init_flags(struct dentry *dentry, struct dentry *upperdentry, > >> > >> bool ovl_dentry_weird(struct dentry *dentry) > >> { > >> + struct ovl_fs *ofs = OVL_FS(dentry->d_sb); > >> + FWIW this was a bug that hits WARN_ON_ONCE(sb->s_type != &ovl_fs_type) because dentry is NOT an ovl dentry. > >> if (!d_can_lookup(dentry) && !d_is_file(dentry) && !d_is_symlink(dentry)) > >> return true; > >> > >> @@ -210,11 +212,11 @@ bool ovl_dentry_weird(struct dentry *dentry) > >> return true; > >> > >> /* > >> - * Allow filesystems that are case-folding capable but deny composing > >> - * ovl stack from case-folded directories. > >> + * Exceptionally for layers with casefold, we accept that they have > >> + * their own hash and compare operations > >> */ > >> - if (sb_has_encoding(dentry->d_sb)) > >> - return IS_CASEFOLDED(d_inode(dentry)); > >> + if (ofs->casefold) > >> + return false; > > > > I think this is better as: > > if (sb_has_encoding(dentry->d_sb)) > > return false; > > And this still fails the test "Casefold enabled" for me. Maybe you are confused because this does not look like a test failure. It looks like this: generic/999 5s ... [19:10:21][ 150.667994] overlayfs: failed lookup in lower (ovl-lower/casefold, name='subdir', err=-116): parent wrong casefold [ 150.669741] overlayfs: failed lookup in lower (ovl-lower/casefold, name='subdir', err=-116): parent wrong casefold [ 150.760644] overlayfs: failed lookup in lower (/ovl-lower, name='casefold', err=-66): child wrong casefold [19:10:24] [not run] generic/999 -- overlayfs does not support casefold enabled layers Ran: generic/999 Not run: generic/999 Passed all 1 tests I'm not sure I will keep the test this way. This is not very standard nor good practice, to run half of the test and then skip it. I would probably split it into two tests. The first one as it is now will run to completion on kenrels >= v6.17 and the Casefold enable test will run on kernels >= v6.18. In any case, please make sure that the test is not skipped when testing Casefold enabled layers And then continue with the missing test cases. When you have a test that passes please send the test itself or a fstest branch for me to test. Thanks, Amir.
Hi Amir, Em 22/08/2025 16:17, Amir Goldstein escreveu: [...] /* >>>> - * Allow filesystems that are case-folding capable but deny composing >>>> - * ovl stack from case-folded directories. >>>> + * Exceptionally for layers with casefold, we accept that they have >>>> + * their own hash and compare operations >>>> */ >>>> - if (sb_has_encoding(dentry->d_sb)) >>>> - return IS_CASEFOLDED(d_inode(dentry)); >>>> + if (ofs->casefold) >>>> + return false; >>> >>> I think this is better as: >>> if (sb_has_encoding(dentry->d_sb)) >>> return false; >>> > > And this still fails the test "Casefold enabled" for me. > > Maybe you are confused because this does not look like > a test failure. It looks like this: > > generic/999 5s ... [19:10:21][ 150.667994] overlayfs: failed lookup > in lower (ovl-lower/casefold, name='subdir', err=-116): parent wrong > casefold > [ 150.669741] overlayfs: failed lookup in lower (ovl-lower/casefold, > name='subdir', err=-116): parent wrong casefold > [ 150.760644] overlayfs: failed lookup in lower (/ovl-lower, > name='casefold', err=-66): child wrong casefold > [19:10:24] [not run] > generic/999 -- overlayfs does not support casefold enabled layers > Ran: generic/999 > Not run: generic/999 > Passed all 1 tests > This is how the test output looks before my changes[1] to the test: $ ./run.sh FSTYP -- ext4 PLATFORM -- Linux/x86_64 archlinux 6.17.0-rc1+ #1174 SMP PREEMPT_DYNAMIC Mon Aug 25 10:18:09 -03 2025 MKFS_OPTIONS -- -F /dev/vdc MOUNT_OPTIONS -- -o acl,user_xattr /dev/vdc /tmp/dir2 generic/999 1s ... [not run] overlayfs does not support casefold enabled layers Ran: generic/999 Not run: generic/999 Passed all 1 tests And this is how it looks after my changes[1] to the test: $ ./run.sh FSTYP -- ext4 PLATFORM -- Linux/x86_64 archlinux 6.17.0-rc1+ #1174 SMP PREEMPT_DYNAMIC Mon Aug 25 10:18:09 -03 2025 MKFS_OPTIONS -- -F /dev/vdc MOUNT_OPTIONS -- -o acl,user_xattr /dev/vdc /tmp/dir2 generic/999 1s Ran: generic/999 Passed all 1 tests So, as far as I can tell, the casefold enabled is not being skipped after the fix to the test. [1] https://lore.kernel.org/lkml/5da6b0f4-2730-4783-9c57-c46c2d13e848@igalia.com/ > I'm not sure I will keep the test this way. This is not very standard nor > good practice, to run half of the test and then skip it. > I would probably split it into two tests. > The first one as it is now will run to completion on kenrels >= v6.17 > and the Casefold enable test will run on kernels >= v6.18. > > In any case, please make sure that the test is not skipped when testing > Casefold enabled layers > > And then continue with the missing test cases. > > When you have a test that passes please send the test itself or > a fstest branch for me to test. Ok! > > Thanks, > Amir.
On Mon, Aug 25, 2025 at 3:31 PM André Almeida <andrealmeid@igalia.com> wrote: > > Hi Amir, > > Em 22/08/2025 16:17, Amir Goldstein escreveu: > > [...] > > /* > >>>> - * Allow filesystems that are case-folding capable but deny composing > >>>> - * ovl stack from case-folded directories. > >>>> + * Exceptionally for layers with casefold, we accept that they have > >>>> + * their own hash and compare operations > >>>> */ > >>>> - if (sb_has_encoding(dentry->d_sb)) > >>>> - return IS_CASEFOLDED(d_inode(dentry)); > >>>> + if (ofs->casefold) > >>>> + return false; > >>> > >>> I think this is better as: > >>> if (sb_has_encoding(dentry->d_sb)) > >>> return false; > >>> > > > > And this still fails the test "Casefold enabled" for me. > > > > Maybe you are confused because this does not look like > > a test failure. It looks like this: > > > > generic/999 5s ... [19:10:21][ 150.667994] overlayfs: failed lookup > > in lower (ovl-lower/casefold, name='subdir', err=-116): parent wrong > > casefold > > [ 150.669741] overlayfs: failed lookup in lower (ovl-lower/casefold, > > name='subdir', err=-116): parent wrong casefold > > [ 150.760644] overlayfs: failed lookup in lower (/ovl-lower, > > name='casefold', err=-66): child wrong casefold > > [19:10:24] [not run] > > generic/999 -- overlayfs does not support casefold enabled layers > > Ran: generic/999 > > Not run: generic/999 > > Passed all 1 tests > > > > This is how the test output looks before my changes[1] to the test: > > $ ./run.sh > FSTYP -- ext4 > PLATFORM -- Linux/x86_64 archlinux 6.17.0-rc1+ #1174 SMP > PREEMPT_DYNAMIC Mon Aug 25 10:18:09 -03 2025 > MKFS_OPTIONS -- -F /dev/vdc > MOUNT_OPTIONS -- -o acl,user_xattr /dev/vdc /tmp/dir2 > > generic/999 1s ... [not run] overlayfs does not support casefold enabled > layers > Ran: generic/999 > Not run: generic/999 > Passed all 1 tests > > > And this is how it looks after my changes[1] to the test: > > $ ./run.sh > FSTYP -- ext4 > PLATFORM -- Linux/x86_64 archlinux 6.17.0-rc1+ #1174 SMP > PREEMPT_DYNAMIC Mon Aug 25 10:18:09 -03 2025 > MKFS_OPTIONS -- -F /dev/vdc > MOUNT_OPTIONS -- -o acl,user_xattr /dev/vdc /tmp/dir2 > > generic/999 1s > Ran: generic/999 > Passed all 1 tests > > So, as far as I can tell, the casefold enabled is not being skipped > after the fix to the test. Is this how it looks with your v6 or after fixing the bug: https://lore.kernel.org/linux-unionfs/68a8c4d7.050a0220.37038e.005c.GAE@google.com/ Because for me this skipping started after fixing this bug Maybe we fixed the bug incorrectly, but I did not see what the problem was from a quick look. Can you test with my branch: https://github.com/amir73il/linux/commits/ovl_casefold/ > > [1] > https://lore.kernel.org/lkml/5da6b0f4-2730-4783-9c57-c46c2d13e848@igalia.com/ > > > > I'm not sure I will keep the test this way. This is not very standard nor > > good practice, to run half of the test and then skip it. > > I would probably split it into two tests. > > The first one as it is now will run to completion on kenrels >= v6.17 > > and the Casefold enable test will run on kernels >= v6.18. > > > > In any case, please make sure that the test is not skipped when testing > > Casefold enabled layers > > > > And then continue with the missing test cases. > > > > When you have a test that passes please send the test itself or > > a fstest branch for me to test. > > Ok! I assume you are testing with ext4 layers? If we are both testing the same code and same test and getting different results I would like to get to the bottom of this, so please share as much information on your test setup as you can. Thanks, Amir.
Em 26/08/2025 04:31, Amir Goldstein escreveu: > On Mon, Aug 25, 2025 at 3:31 PM André Almeida <andrealmeid@igalia.com> wrote: >> >> Hi Amir, >> >> Em 22/08/2025 16:17, Amir Goldstein escreveu: >> >> [...] >> >> /* >>>>>> - * Allow filesystems that are case-folding capable but deny composing >>>>>> - * ovl stack from case-folded directories. >>>>>> + * Exceptionally for layers with casefold, we accept that they have >>>>>> + * their own hash and compare operations >>>>>> */ >>>>>> - if (sb_has_encoding(dentry->d_sb)) >>>>>> - return IS_CASEFOLDED(d_inode(dentry)); >>>>>> + if (ofs->casefold) >>>>>> + return false; >>>>> >>>>> I think this is better as: >>>>> if (sb_has_encoding(dentry->d_sb)) >>>>> return false; >>>>> >>> >>> And this still fails the test "Casefold enabled" for me. >>> >>> Maybe you are confused because this does not look like >>> a test failure. It looks like this: >>> >>> generic/999 5s ... [19:10:21][ 150.667994] overlayfs: failed lookup >>> in lower (ovl-lower/casefold, name='subdir', err=-116): parent wrong >>> casefold >>> [ 150.669741] overlayfs: failed lookup in lower (ovl-lower/casefold, >>> name='subdir', err=-116): parent wrong casefold >>> [ 150.760644] overlayfs: failed lookup in lower (/ovl-lower, >>> name='casefold', err=-66): child wrong casefold >>> [19:10:24] [not run] >>> generic/999 -- overlayfs does not support casefold enabled layers >>> Ran: generic/999 >>> Not run: generic/999 >>> Passed all 1 tests >>> >> >> This is how the test output looks before my changes[1] to the test: >> >> $ ./run.sh >> FSTYP -- ext4 >> PLATFORM -- Linux/x86_64 archlinux 6.17.0-rc1+ #1174 SMP >> PREEMPT_DYNAMIC Mon Aug 25 10:18:09 -03 2025 >> MKFS_OPTIONS -- -F /dev/vdc >> MOUNT_OPTIONS -- -o acl,user_xattr /dev/vdc /tmp/dir2 >> >> generic/999 1s ... [not run] overlayfs does not support casefold enabled >> layers >> Ran: generic/999 >> Not run: generic/999 >> Passed all 1 tests >> >> >> And this is how it looks after my changes[1] to the test: >> >> $ ./run.sh >> FSTYP -- ext4 >> PLATFORM -- Linux/x86_64 archlinux 6.17.0-rc1+ #1174 SMP >> PREEMPT_DYNAMIC Mon Aug 25 10:18:09 -03 2025 >> MKFS_OPTIONS -- -F /dev/vdc >> MOUNT_OPTIONS -- -o acl,user_xattr /dev/vdc /tmp/dir2 >> >> generic/999 1s >> Ran: generic/999 >> Passed all 1 tests >> >> So, as far as I can tell, the casefold enabled is not being skipped >> after the fix to the test. > > Is this how it looks with your v6 or after fixing the bug: > https://lore.kernel.org/linux-unionfs/68a8c4d7.050a0220.37038e.005c.GAE@google.com/ > > Because for me this skipping started after fixing this bug > Maybe we fixed the bug incorrectly, but I did not see what the problem > was from a quick look. > > Can you test with my branch: > https://github.com/amir73il/linux/commits/ovl_casefold/ > Right, our branches have a different base, mine is older and based on the tag vfs/vfs-6.18.mount. I have now tested with your branch, and indeed the test fails with "overlayfs does not support casefold enabled". I did some debugging and the missing commit from my branch that is making this difference here is e8bd877fb76bb9f3 ("ovl: fix possible double unlink"). After reverting it on top of your branch, the test works. I'm not sure yet why this prevents the mount, but this is the call trace when the error happens: TID/PID 860/860 (mount/mount): entry_SYSCALL_64_after_hwframe+0x77 do_syscall_64+0xa2 x64_sys_call+0x1bc3 __x64_sys_fsconfig+0x46c vfs_cmd_create+0x60 vfs_get_tree+0x2e ovl_get_tree+0x19 get_tree_nodev+0x70 ovl_fill_super+0x53b ! 0us [-EINVAL] ovl_parent_lock And for the ovl_parent_lock() arguments, *parent="work", *child="#7". So right now I'm trying to figure out why the dentry for #7 is not hashed. >> >> [1] >> https://lore.kernel.org/lkml/5da6b0f4-2730-4783-9c57-c46c2d13e848@igalia.com/ >> >> >>> I'm not sure I will keep the test this way. This is not very standard nor >>> good practice, to run half of the test and then skip it. >>> I would probably split it into two tests. >>> The first one as it is now will run to completion on kenrels >= v6.17 >>> and the Casefold enable test will run on kernels >= v6.18. >>> >>> In any case, please make sure that the test is not skipped when testing >>> Casefold enabled layers >>> >>> And then continue with the missing test cases. >>> >>> When you have a test that passes please send the test itself or >>> a fstest branch for me to test. >> >> Ok! > > I assume you are testing with ext4 layers? > > If we are both testing the same code and same test and getting different > results I would like to get to the bottom of this, so please share as much > information on your test setup as you can. > > Thanks, > Amir.
On Tue, Aug 26, 2025 at 9:01 PM André Almeida <andrealmeid@igalia.com> wrote: > > > > Em 26/08/2025 04:31, Amir Goldstein escreveu: > > On Mon, Aug 25, 2025 at 3:31 PM André Almeida <andrealmeid@igalia.com> wrote: > >> > >> Hi Amir, > >> > >> Em 22/08/2025 16:17, Amir Goldstein escreveu: > >> > >> [...] > >> > >> /* > >>>>>> - * Allow filesystems that are case-folding capable but deny composing > >>>>>> - * ovl stack from case-folded directories. > >>>>>> + * Exceptionally for layers with casefold, we accept that they have > >>>>>> + * their own hash and compare operations > >>>>>> */ > >>>>>> - if (sb_has_encoding(dentry->d_sb)) > >>>>>> - return IS_CASEFOLDED(d_inode(dentry)); > >>>>>> + if (ofs->casefold) > >>>>>> + return false; > >>>>> > >>>>> I think this is better as: > >>>>> if (sb_has_encoding(dentry->d_sb)) > >>>>> return false; > >>>>> > >>> > >>> And this still fails the test "Casefold enabled" for me. > >>> > >>> Maybe you are confused because this does not look like > >>> a test failure. It looks like this: > >>> > >>> generic/999 5s ... [19:10:21][ 150.667994] overlayfs: failed lookup > >>> in lower (ovl-lower/casefold, name='subdir', err=-116): parent wrong > >>> casefold > >>> [ 150.669741] overlayfs: failed lookup in lower (ovl-lower/casefold, > >>> name='subdir', err=-116): parent wrong casefold > >>> [ 150.760644] overlayfs: failed lookup in lower (/ovl-lower, > >>> name='casefold', err=-66): child wrong casefold > >>> [19:10:24] [not run] > >>> generic/999 -- overlayfs does not support casefold enabled layers > >>> Ran: generic/999 > >>> Not run: generic/999 > >>> Passed all 1 tests > >>> > >> > >> This is how the test output looks before my changes[1] to the test: > >> > >> $ ./run.sh > >> FSTYP -- ext4 > >> PLATFORM -- Linux/x86_64 archlinux 6.17.0-rc1+ #1174 SMP > >> PREEMPT_DYNAMIC Mon Aug 25 10:18:09 -03 2025 > >> MKFS_OPTIONS -- -F /dev/vdc > >> MOUNT_OPTIONS -- -o acl,user_xattr /dev/vdc /tmp/dir2 > >> > >> generic/999 1s ... [not run] overlayfs does not support casefold enabled > >> layers > >> Ran: generic/999 > >> Not run: generic/999 > >> Passed all 1 tests > >> > >> > >> And this is how it looks after my changes[1] to the test: > >> > >> $ ./run.sh > >> FSTYP -- ext4 > >> PLATFORM -- Linux/x86_64 archlinux 6.17.0-rc1+ #1174 SMP > >> PREEMPT_DYNAMIC Mon Aug 25 10:18:09 -03 2025 > >> MKFS_OPTIONS -- -F /dev/vdc > >> MOUNT_OPTIONS -- -o acl,user_xattr /dev/vdc /tmp/dir2 > >> > >> generic/999 1s > >> Ran: generic/999 > >> Passed all 1 tests > >> > >> So, as far as I can tell, the casefold enabled is not being skipped > >> after the fix to the test. > > > > Is this how it looks with your v6 or after fixing the bug: > > https://lore.kernel.org/linux-unionfs/68a8c4d7.050a0220.37038e.005c.GAE@google.com/ > > > > Because for me this skipping started after fixing this bug > > Maybe we fixed the bug incorrectly, but I did not see what the problem > > was from a quick look. > > > > Can you test with my branch: > > https://github.com/amir73il/linux/commits/ovl_casefold/ > > > > Right, our branches have a different base, mine is older and based on > the tag vfs/vfs-6.18.mount. > > I have now tested with your branch, and indeed the test fails with > "overlayfs does not support casefold enabled". I did some debugging and > the missing commit from my branch that is making this difference here is > e8bd877fb76bb9f3 ("ovl: fix possible double unlink"). After reverting it > on top of your branch, the test works. I'm not sure yet why this > prevents the mount, but this is the call trace when the error happens: Wow, that is an interesting development race... > > TID/PID 860/860 (mount/mount): > > entry_SYSCALL_64_after_hwframe+0x77 > do_syscall_64+0xa2 > x64_sys_call+0x1bc3 > __x64_sys_fsconfig+0x46c > vfs_cmd_create+0x60 > vfs_get_tree+0x2e > ovl_get_tree+0x19 > get_tree_nodev+0x70 > ovl_fill_super+0x53b > ! 0us [-EINVAL] ovl_parent_lock > > And for the ovl_parent_lock() arguments, *parent="work", *child="#7". So > right now I'm trying to figure out why the dentry for #7 is not hashed. > 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. commit 5dfcd10378038637648f3f422e3d5097eb6faa5f Author: Amir Goldstein <amir73il@gmail.com> 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 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@igalia.com> Link: https://lore.kernel.org/r/18704e8c-c734-43f3-bc7c-b8be345e1bf5@igalia.com/ Signed-off-by: Amir Goldstein <amir73il@gmail.com> 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);
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@gmail.com> > 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@igalia.com> > Link: https://lore.kernel.org/r/18704e8c-c734-43f3-bc7c-b8be345e1bf5@igalia.com/ I think the correct chain here is: Reported-by: André Almeida <andrealmeid@igalia.com> Closes: https://lore.kernel.org/r/18704e8c-c734-43f3-bc7c-b8be345e1bf5@igalia.com/ Fixes: e8bd877fb76b ("ovl: fix possible double unlink") > Signed-off-by: Amir Goldstein <amir73il@gmail.com> > Reviewed-by: André Almeida <andrealmeid@igalia.com> > 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);
© 2016 - 2025 Red Hat, Inc.