Re: [PATCH v6 9/9] ovl: Support mounting case-insensitive enabled layers

[Amir Goldstein <amir73il@gmail.com>]

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.