[PATCH RFC v3 6/7] ovl: Add S_CASEFOLD as part of the inode flag to be copied

André Almeida posted 7 patches 1 month, 3 weeks ago
There is a newer version of this series
[PATCH RFC v3 6/7] ovl: Add S_CASEFOLD as part of the inode flag to be copied
Posted by André Almeida 1 month, 3 weeks ago
To keep ovl's inodes consistent with their real inodes, add the
S_CASEFOLD flag as part of the flags that need to be copied.

Signed-off-by: André Almeida <andrealmeid@igalia.com>
---
Changes from v2:
- Instead of manually setting the flag if the realpath dentry is
  casefolded, just add this flag as part of the flags that need to be
  copied.
---
 fs/overlayfs/overlayfs.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index bb0d7ded8e763a4a7a6fc506d966ed2f3bdb4f06..8a9a67d2933173c61b0fa0af5634d91e092e00b2 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -822,7 +822,7 @@ struct inode *ovl_get_inode(struct super_block *sb,
 void ovl_copyattr(struct inode *to);
 
 /* vfs inode flags copied from real to ovl inode */
-#define OVL_COPY_I_FLAGS_MASK	(S_SYNC | S_NOATIME | S_APPEND | S_IMMUTABLE)
+#define OVL_COPY_I_FLAGS_MASK	(S_SYNC | S_NOATIME | S_APPEND | S_IMMUTABLE | S_CASEFOLD)
 /* vfs inode flags read from overlay.protattr xattr to ovl inode */
 #define OVL_PROT_I_FLAGS_MASK	(S_APPEND | S_IMMUTABLE)
 

-- 
2.50.1

Re: [PATCH RFC v3 6/7] ovl: Add S_CASEFOLD as part of the inode flag to be copied
Posted by Amir Goldstein 1 month, 3 weeks ago
On Fri, Aug 8, 2025 at 10:59 PM André Almeida <andrealmeid@igalia.com> wrote:
>
> To keep ovl's inodes consistent with their real inodes, add the
> S_CASEFOLD flag as part of the flags that need to be copied.
>
> Signed-off-by: André Almeida <andrealmeid@igalia.com>
> ---
> Changes from v2:
> - Instead of manually setting the flag if the realpath dentry is
>   casefolded, just add this flag as part of the flags that need to be
>   copied.
> ---
>  fs/overlayfs/overlayfs.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> index bb0d7ded8e763a4a7a6fc506d966ed2f3bdb4f06..8a9a67d2933173c61b0fa0af5634d91e092e00b2 100644
> --- a/fs/overlayfs/overlayfs.h
> +++ b/fs/overlayfs/overlayfs.h
> @@ -822,7 +822,7 @@ struct inode *ovl_get_inode(struct super_block *sb,
>  void ovl_copyattr(struct inode *to);
>
>  /* vfs inode flags copied from real to ovl inode */
> -#define OVL_COPY_I_FLAGS_MASK  (S_SYNC | S_NOATIME | S_APPEND | S_IMMUTABLE)
> +#define OVL_COPY_I_FLAGS_MASK  (S_SYNC | S_NOATIME | S_APPEND | S_IMMUTABLE | S_CASEFOLD)
>  /* vfs inode flags read from overlay.protattr xattr to ovl inode */
>  #define OVL_PROT_I_FLAGS_MASK  (S_APPEND | S_IMMUTABLE)
>

Ok, this is simpler, but it's too simple.
OVL_COPY_I_FLAGS_MASK is used in copy up with the assumption that
all copied i_flags are related to fileattr flags, so you need something like:

diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
index 27396fe63f6d..66bd43a99d2e 100644
--- a/fs/overlayfs/copy_up.c
+++ b/fs/overlayfs/copy_up.c
@@ -670,7 +670,7 @@ static int ovl_copy_up_metadata(struct
ovl_copy_up_ctx *c, struct dentry *temp)
        if (err)
                return err;

-       if (inode->i_flags & OVL_COPY_I_FLAGS_MASK &&
+       if (inode->i_flags & OVL_FATTR_I_FLAGS_MASK &&
            (S_ISREG(c->stat.mode) || S_ISDIR(c->stat.mode))) {
                /*
                 * Copy the fileattr inode flags that are the source of already
diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
index bb0d7ded8e76..f014802cfe55 100644
--- a/fs/overlayfs/overlayfs.h
+++ b/fs/overlayfs/overlayfs.h
@@ -821,10 +821,14 @@ struct inode *ovl_get_inode(struct super_block *sb,
                            struct ovl_inode_params *oip);
 void ovl_copyattr(struct inode *to);

-/* vfs inode flags copied from real to ovl inode */
-#define OVL_COPY_I_FLAGS_MASK  (S_SYNC | S_NOATIME | S_APPEND | S_IMMUTABLE)
-/* vfs inode flags read from overlay.protattr xattr to ovl inode */
+/* vfs fileattr flags read from overlay.protattr xattr to ovl inode */
 #define OVL_PROT_I_FLAGS_MASK  (S_APPEND | S_IMMUTABLE)
+/* vfs fileattr flags copied from real to ovl inode */
+#define OVL_FATTR_I_FLAGS_MASK (OVL_PROT_I_FLAGS_MASK | \
+                                S_SYNC | S_NOATIME)
+/* vfs inode flags copied from real to ovl inode */
+#define OVL_COPY_I_FLAGS_MASK  (OVL_FATTR_I_FLAGS_MASK | \
+                                S_CASEFOLD)

In addition we want a sanity check that S_CASEFOLD is always consistent
with the ovl global casefold state.
I think a WARN_ON assertion is enough even without failing:

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index df85a76597e9..53914b4039c0 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1300,6 +1300,7 @@ static struct dentry *ovl_get_root(struct super_block *sb,
        ovl_dentry_set_flag(OVL_E_CONNECTED, root);
        ovl_set_upperdata(d_inode(root));
        ovl_inode_init(d_inode(root), &oip, ino, fsid);
+       WARN_ON(IS_CASEFOLDED(d_inode(root)) != ofs->casefold);
        ovl_dentry_init_flags(root, upperdentry, oe, DCACHE_OP_WEAK_REVALIDATE);
        /* root keeps a reference of upperdentry */
        dget(upperdentry);
diff --git a/fs/overlayfs/inode.c b/fs/overlayfs/inode.c
index ecb9f2019395..ad97daf6641b 100644
--- a/fs/overlayfs/inode.c
+++ b/fs/overlayfs/inode.c
@@ -1277,6 +1277,7 @@ struct inode *ovl_get_inode(struct super_block *sb,
        }
        ovl_fill_inode(inode, realinode->i_mode, realinode->i_rdev);
        ovl_inode_init(inode, oip, ino, fsid);
+       WARN_ON_ONCE(IS_CASEFOLDED(inode) != ofs->casefold);

        if (upperdentry && ovl_is_impuredir(sb, upperdentry))
                ovl_set_flag(OVL_IMPURE, inode);

Thanks,
Amir.
Re: [PATCH RFC v3 6/7] ovl: Add S_CASEFOLD as part of the inode flag to be copied
Posted by Amir Goldstein 1 month, 3 weeks ago
On Sat, Aug 9, 2025 at 11:51 AM Amir Goldstein <amir73il@gmail.com> wrote:
>
> On Fri, Aug 8, 2025 at 10:59 PM André Almeida <andrealmeid@igalia.com> wrote:
> >
> > To keep ovl's inodes consistent with their real inodes, add the
> > S_CASEFOLD flag as part of the flags that need to be copied.
> >
> > Signed-off-by: André Almeida <andrealmeid@igalia.com>
> > ---
> > Changes from v2:
> > - Instead of manually setting the flag if the realpath dentry is
> >   casefolded, just add this flag as part of the flags that need to be
> >   copied.
> > ---
> >  fs/overlayfs/overlayfs.h | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/fs/overlayfs/overlayfs.h b/fs/overlayfs/overlayfs.h
> > index bb0d7ded8e763a4a7a6fc506d966ed2f3bdb4f06..8a9a67d2933173c61b0fa0af5634d91e092e00b2 100644
> > --- a/fs/overlayfs/overlayfs.h
> > +++ b/fs/overlayfs/overlayfs.h
> > @@ -822,7 +822,7 @@ struct inode *ovl_get_inode(struct super_block *sb,
> >  void ovl_copyattr(struct inode *to);
> >
> >  /* vfs inode flags copied from real to ovl inode */
> > -#define OVL_COPY_I_FLAGS_MASK  (S_SYNC | S_NOATIME | S_APPEND | S_IMMUTABLE)
> > +#define OVL_COPY_I_FLAGS_MASK  (S_SYNC | S_NOATIME | S_APPEND | S_IMMUTABLE | S_CASEFOLD)
> >  /* vfs inode flags read from overlay.protattr xattr to ovl inode */
> >  #define OVL_PROT_I_FLAGS_MASK  (S_APPEND | S_IMMUTABLE)
> >
>
> Ok, this is simpler, but it's too simple.
> OVL_COPY_I_FLAGS_MASK is used in copy up with the assumption that
> all copied i_flags are related to fileattr flags, so you need something like:
>
> diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c
> index 27396fe63f6d..66bd43a99d2e 100644
> --- a/fs/overlayfs/copy_up.c
> +++ b/fs/overlayfs/copy_up.c
> @@ -670,7 +670,7 @@ static int ovl_copy_up_metadata(struct
> ovl_copy_up_ctx *c, struct dentry *temp)
>         if (err)
>                 return err;
>
> -       if (inode->i_flags & OVL_COPY_I_FLAGS_MASK &&
> +       if (inode->i_flags & OVL_FATTR_I_FLAGS_MASK &&
>             (S_ISREG(c->stat.mode) || S_ISDIR(c->stat.mode))) {
>                 /*
>                  * Copy the fileattr inode flags that are the source of already

Which reminds me that you also need to verify that a copied up directory
conforms to the ofs->casefold expectation, because there is no code
to make the copied up directory casefolded.

We can assume that layers check has already verified that upperdir/workdir
are casefold correct, but we need to verify that $workdir/work/$tmpdir created
by ovl_create_temp() has inherited the expected casefolding.

Same goes for ovl_mkdir(), we must verify that the new created dentry/inode
conform to the expected ofs->casefold.

I think this check in ovl_create_real() should cover both cases:

diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c
index 70b8687dc45e..be8c5d02302d 100644
--- a/fs/overlayfs/dir.c
+++ b/fs/overlayfs/dir.c
@@ -187,6 +187,11 @@ struct dentry *ovl_create_real(struct ovl_fs
*ofs, struct dentry *parent,
                        /* mkdir is special... */
                        newdentry =  ovl_do_mkdir(ofs, dir, newdentry,
attr->mode);
                        err = PTR_ERR_OR_ZERO(newdentry);
+                       /* expect to inherit casefolding from
workdir/upperdir */
+                       if (!err && ofs->casefold !=
ovl_dentry_casefolded(newdentry)) {
+                               dput(newdentry);
+                               err = -EINVAL;
+                       }
                        break;

Thanks,
Amir.