Re: [PATCH RFC v3 1/7] ovl: Store casefold name for case-insentive dentries

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Fri, Aug 8, 2025 at 10:59 PM André Almeida <andrealmeid@xxxxxxxxxx> wrote:
>
> In order to make case-insentive mounting points work, overlayfs needs

Terminology:
not case-insensitive mounting points
case-insensitive layers.

> the casefolded version of its dentries so the search and insertion in

Terminology:
overlayfs needs the casefolded names
the objects in the rb tree correspond to dirent readdir results not to dentries
which are dcache objects.

> the struct ovl_readdir_data's red-black compares the dentry names in a
> case-insentive fashion.
>
> If a dentry is casefolded, compute and store it's casefolded name and

We are not doing per-dir casefolding so should say:
"If overlay mount is casefolded, compute and store the casefolded name..."

> it's Unicode map. If utf8_casefold() fails, set it's name pointer as
> NULL so it can be ignored and fallback to the original name.
>
> Signed-off-by: André Almeida <andrealmeid@xxxxxxxxxx>
> ---
> Changes from v3:
>  - Guard all the casefolding inside of IS_ENABLED(UNICODE)
> ---
>  fs/overlayfs/readdir.c | 41 ++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 40 insertions(+), 1 deletion(-)
>
> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
> index b65cdfce31ce27172d28d879559f1008b9c87320..2f42fec97f76c2000f76e15c60975db567b2c6d6 100644
> --- a/fs/overlayfs/readdir.c
> +++ b/fs/overlayfs/readdir.c
> @@ -16,6 +16,8 @@
>  #include <linux/overflow.h>
>  #include "overlayfs.h"
>
> +#define OVL_NAME_LEN 255
> +

This is very arbitrary.
Either use the existing constant NAME_MAX or
use the max of all layers ofs->namelen.
I think ofs->namelen will always be <= NAME_MAX,
so it's fine to use NAME_MAX if we never expose
the cf_name to userspace.

>  struct ovl_cache_entry {
>         unsigned int len;
>         unsigned int type;
> @@ -27,6 +29,8 @@ struct ovl_cache_entry {
>         bool is_upper;
>         bool is_whiteout;
>         bool check_xwhiteout;
> +       char *cf_name;
> +       int cf_len;
>         char name[];
>  };
>
> @@ -45,6 +49,7 @@ struct ovl_readdir_data {
>         struct list_head *list;
>         struct list_head middle;
>         struct ovl_cache_entry *first_maybe_whiteout;
> +       struct unicode_map *map;
>         int count;
>         int err;
>         bool is_upper;
> @@ -166,6 +171,31 @@ static struct ovl_cache_entry *ovl_cache_entry_new(struct ovl_readdir_data *rdd,
>         p->is_whiteout = false;
>         /* Defer check for overlay.whiteout to ovl_iterate() */
>         p->check_xwhiteout = rdd->in_xwhiteouts_dir && d_type == DT_REG;
> +       p->cf_name = NULL;
> +       p->cf_len = 0;
> +
> +#if IS_ENABLED(CONFIG_UNICODE)

Whenever possible in C code you should use:

if (IS_ENABLED(CONFIG_UNICODE) && rdd->map && ...

> +       if (rdd->map && !is_dot_dotdot(name, len)) {
> +               const struct qstr str = { .name = name, .len = len };
> +               int ret;
> +
> +               p->cf_name = kmalloc(OVL_NAME_LEN, GFP_KERNEL);
> +
> +               if (!p->cf_name) {
> +                       kfree(p);
> +                       return NULL;
> +               }
> +
> +               ret = utf8_casefold(rdd->map, &str, p->cf_name, OVL_NAME_LEN);
> +
> +               if (ret < 0) {
> +                       kfree(p->cf_name);
> +                       p->cf_name = NULL;
> +               } else {
> +                       p->cf_len = ret;
> +               }
> +       }
> +#endif
>
>         if (d_type == DT_CHR) {
>                 p->next_maybe_whiteout = rdd->first_maybe_whiteout;
> @@ -223,8 +253,10 @@ void ovl_cache_free(struct list_head *list)
>         struct ovl_cache_entry *p;
>         struct ovl_cache_entry *n;
>
> -       list_for_each_entry_safe(p, n, list, l_node)
> +       list_for_each_entry_safe(p, n, list, l_node) {
> +               kfree(p->cf_name);
>                 kfree(p);
> +       }
>
>         INIT_LIST_HEAD(list);
>  }
> @@ -357,12 +389,19 @@ static int ovl_dir_read_merged(struct dentry *dentry, struct list_head *list,
>                 .list = list,
>                 .root = root,
>                 .is_lowest = false,
> +               .map = NULL,
>         };
>         int idx, next;
>         const struct ovl_layer *layer;
>
>         for (idx = 0; idx != -1; idx = next) {
>                 next = ovl_path_next(idx, dentry, &realpath, &layer);
> +
> +#if IS_ENABLED(CONFIG_UNICODE)
> +               if (ovl_dentry_casefolded(realpath.dentry))
> +                       rdd.map = realpath.dentry->d_sb->s_encoding;
> +#endif
> +

We are not doing per-dir casefolding, so this should be
if (ofs->casefold)

and I'd rather avoid this ifdef, so how about another vfs helper:

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 2ec4807d4ea8..3f4c89367908 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -3738,15 +3738,20 @@ static inline bool
generic_ci_validate_strict_name(struct inode *dir, struct qst
 }
 #endif

-static inline bool sb_has_encoding(const struct super_block *sb)
+static inline struct unicode_map *sb_encoding(const struct super_block *sb)
 {
 #if IS_ENABLED(CONFIG_UNICODE)
-       return !!sb->s_encoding;
+       return sb->s_encoding;
 #else
-       return false;
+       return NULL;
 #endif
 }

+static inline bool sb_has_encoding(const struct super_block *sb)
+{
+       return !!sb_encoding(sb);
+}
+

Thanks,
Amir.





[Index of Archives]     [Linux Ext4 Filesystem]     [Union Filesystem]     [Filesystem Testing]     [Ceph Users]     [Ecryptfs]     [NTFS 3]     [AutoFS]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux Cachefs]     [Reiser Filesystem]     [Linux RAID]     [NTFS 3]     [Samba]     [Device Mapper]     [CEPH Development]

  Powered by Linux