| 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
| ||
|
Message-ID: <CAOQ4uxjdE4A14cwkhm+QjZS-5ANhc1568nbNU=KtF-oh_fE3VQ@mail.gmail.com>
Date: Sat, 9 Aug 2025 14:36:14 +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 1/7] ovl: Store casefold name for case-insentive dentries
On Fri, Aug 8, 2025 at 10:59 PM André Almeida <andrealmeid@...lia.com> 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@...lia.com>
> ---
> 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.
Powered by blists - more mailing lists