lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <CAOQ4uxj+42O07HxnKrp5A2A-Gk0Z_WLDk=62Y9WZ7RRypdju2w@mail.gmail.com>
Date: Sat, 9 Aug 2025 13:50:50 +0200
From: Amir Goldstein <amir73il@...il.com>
To: André Almeida <andrealmeid@...lia.com>
Cc: Miklos Szeredi <miklos@...redi.hu>, Theodore Tso <tytso@....edu>, 
	Gabriel Krisman Bertazi <krisman@...nel.org>, linux-unionfs@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org, 
	Alexander Viro <viro@...iv.linux.org.uk>, Christian Brauner <brauner@...nel.org>, Jan Kara <jack@...e.cz>, 
	kernel-dev@...lia.com
Subject: Re: [PATCH RFC v3 5/7] ovl: Set case-insensitive dentry operations
 for ovl sb

On Fri, Aug 8, 2025 at 10:59 PM André Almeida <andrealmeid@...lia.com> wrote:
>
> For filesystems with encoding (i.e. with case-insensitive support), set
> the dentry operations for the super block as ovl_dentry_ci_operations.
> Also, use the first layer encoding as the ovl super block encoding.
>
> Signed-off-by: André Almeida <andrealmeid@...lia.com>
> ---
> Changes from v2:
> - Create ovl_dentry_ci_operations to not override dentry ops set by
>   ovl_dentry_operations
> - Create a new function for this
> - Instead of setting encoding just when there's a upper layer, set it
>   for any first layer (ofs->fs[0].sb), regardless of it being upper or
>   not.
> ---
>  fs/overlayfs/super.c | 28 ++++++++++++++++++++++++++++
>  1 file changed, 28 insertions(+)
>
> diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
> index bcb7f5dbf9a32e4aa09bc41596be443851e21200..68091bf8368a880d62d9425552613497d6e90b6b 100644
> --- a/fs/overlayfs/super.c
> +++ b/fs/overlayfs/super.c
> @@ -161,6 +161,16 @@ static const struct dentry_operations ovl_dentry_operations = {
>         .d_weak_revalidate = ovl_dentry_weak_revalidate,
>  };
>
> +#if IS_ENABLED(CONFIG_UNICODE)
> +static const struct dentry_operations ovl_dentry_ci_operations = {
> +       .d_real = ovl_d_real,
> +       .d_revalidate = ovl_dentry_revalidate,
> +       .d_weak_revalidate = ovl_dentry_weak_revalidate,
> +       .d_hash = generic_ci_d_hash,
> +       .d_compare = generic_ci_d_compare,
> +};
> +#endif
> +
>  static struct kmem_cache *ovl_inode_cachep;
>
>  static struct inode *ovl_alloc_inode(struct super_block *sb)
> @@ -1318,6 +1328,21 @@ static struct dentry *ovl_get_root(struct super_block *sb,
>         return root;
>  }
>
> +/*
> + * Set the ovl sb encoding as the same one used by the first layer
> + */
> +static void ovl_set_sb_ci_ops(struct super_block *ovl_sb, struct super_block *fs_sb)
> +{
> +#if IS_ENABLED(CONFIG_UNICODE)
> +       if (sb_has_encoding(fs_sb)) {
> +               ovl_sb->s_encoding = fs_sb->s_encoding;
> +               ovl_sb->s_encoding_flags = fs_sb->s_encoding_flags;
> +       }
> +
> +       set_default_d_op(ovl_sb, &ovl_dentry_ci_operations);

I don't like it that set_default_d_op() is called twice and if anything this
helper should have been called only for the ofs->casefold enabled case.

what I suggest it to split to two helpers, first set dentry_ops based on
ofs->casefold determined before ovl_fill_super() is called:

diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c
index df85a76597e9..00647440a566 100644
--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -1307,6 +1307,19 @@ static struct dentry *ovl_get_root(struct
super_block *sb,
        return root;
 }

+static void ovl_set_d_op(struct super_block *sb)
+{
+       struct ovl_fs *ofs = sb->s_fs_info;
+
+#if IS_ENABLED(CONFIG_UNICODE)
+       if (ofs->casefold) {
+               set_default_d_op(ovl_sb, &ovl_dentry_ci_operations);
+               return;
+       }
+#endif
+       set_default_d_op(sb, &ovl_dentry_operations);
+}
+
 int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
 {
        struct ovl_fs *ofs = sb->s_fs_info;
@@ -1322,7 +1335,7 @@ int ovl_fill_super(struct super_block *sb,
struct fs_context *fc)
        if (WARN_ON(fc->user_ns != current_user_ns()))
                goto out_err;

-       set_default_d_op(sb, &ovl_dentry_operations);
+       ovl_set_d_op(sb);


> +#endif
> +}
> +
>  int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
>  {
>         struct ovl_fs *ofs = sb->s_fs_info;
> @@ -1423,12 +1448,15 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
>
>                 sb->s_stack_depth = upper_sb->s_stack_depth;
>                 sb->s_time_gran = upper_sb->s_time_gran;
> +

stray new line.

>         }
>         oe = ovl_get_lowerstack(sb, ctx, ofs, layers);
>         err = PTR_ERR(oe);
>         if (IS_ERR(oe))
>                 goto out_err;
>
> +       ovl_set_sb_ci_ops(sb, ofs->fs[0].sb);
> +

This is wrong because ofs->fs[0].sb is NULL on overlay without an upper dir.

Please consider doing this as part of patch 4 instead of using the
local sb1 var:

--- a/fs/overlayfs/super.c
+++ b/fs/overlayfs/super.c
@@ -991,6 +991,19 @@ static int ovl_get_data_fsid(struct ovl_fs *ofs)
 }

+/*
+ * Set the ovl sb encoding as the same one used by the first layer
+ */
+static void ovl_set_encoding(struct super_block *sb, struct super_block *fs_sb)
+{
+#if IS_ENABLED(CONFIG_UNICODE)
+      if (sb_has_encoding(fs_sb)) {
+              sb->s_encoding = fs_sb->s_encoding;
+              sb->s_encoding_flags = fs_sb->s_encoding_flags;
+      }
+#endif
+}
+
 static int ovl_get_layers(struct super_block *sb, struct ovl_fs *ofs,
                          struct ovl_fs_context *ctx, struct ovl_layer *layers)
 {
@@ -1024,6 +1036,8 @@ static int ovl_get_layers(struct super_block
*sb, struct ovl_fs *ofs,
        if (ovl_upper_mnt(ofs)) {
                ofs->fs[0].sb = ovl_upper_mnt(ofs)->mnt_sb;
                ofs->fs[0].is_lower = false;
+               if (ofs->casefold)
+                       ovl_set_encoding(sb, ofs->fs[0].sb);
        }

        nr_merged_lower = ctx->nr - ctx->nr_data;
@@ -1083,6 +1097,17 @@ static int ovl_get_layers(struct super_block
*sb, struct ovl_fs *ofs,
                l->name = NULL;
                ofs->numlayer++;
                ofs->fs[fsid].is_lower = true;
+
+               if (ofs->casefold) {
+                       if (!ovl_upper_mnt(ofs) && !sb_has_encoding(sb))
+                               ovl_set_encoding(sb, ofs->fs[fsid].sb);
+
+                       if (!sb_has_encoding(sb) ||
+                           !sb_same_encoding(sb, mnt->mnt_sb)) {
+                               pr_err("all layers must have the same
encoding\n");
+                               return -EINVAL;
+                       }
+               }
        }


Thanks,
Amir.

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ