[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <f5ea7370-c8c3-47c2-88cb-9740d82e87c6@igalia.com>
Date: Tue, 5 Aug 2025 15:40:28 -0300
From: André Almeida <andrealmeid@...lia.com>
To: Gabriel Krisman Bertazi <gabriel@...sman.be>
Cc: Miklos Szeredi <miklos@...redi.hu>, Amir Goldstein <amir73il@...il.com>,
Theodore Tso <tytso@....edu>, 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 v2 2/8] ovl: Create ovl_strcmp() with casefold support
Hi Gabriel!
Em 05/08/2025 11:56, Gabriel Krisman Bertazi escreveu:
> André Almeida <andrealmeid@...lia.com> writes:
>
>> To add overlayfs support casefold filesystems, create a new function
>> ovl_strcmp() with support for casefold names.
>>
>> If the ovl_cache_entry have stored a casefold name, use it and create
>> a casfold version of the name that is going to be compared to.
>>
>> For the casefold support, just comparing the strings does not work
>> because we need the dentry enconding, so make this function find the
>> equivalent dentry for a giving directory, if any.
>>
>> As this function is used for search and insertion in the red-black tree,
>> that means that the tree node keys are going to be the casefolded
>> version of the dentry's names. Otherwise, the search would not work for
>> case-insensitive mount points.
>>
>> For the non-casefold names, nothing changes.
>>
>> Signed-off-by: André Almeida <andrealmeid@...lia.com>
>> ---
>> I wonder what should be done here if kmalloc fails, if the strcmp()
>> should fail as well or just fallback to the normal name?
>> ---
>> fs/overlayfs/readdir.c | 42 ++++++++++++++++++++++++++++++++++++++++--
>> 1 file changed, 40 insertions(+), 2 deletions(-)
>>
>> diff --git a/fs/overlayfs/readdir.c b/fs/overlayfs/readdir.c
>> index 83bca1bcb0488461b08effa70b32ff2fefba134e..1b8eb10e72a229ade40d18795746d3c779797a06 100644
>> --- a/fs/overlayfs/readdir.c
>> +++ b/fs/overlayfs/readdir.c
>> @@ -72,6 +72,44 @@ static struct ovl_cache_entry *ovl_cache_entry_from_node(struct rb_node *n)
>> return rb_entry(n, struct ovl_cache_entry, node);
>> }
>>
>> +/*
>> + * Compare a string with a cache entry, with support for casefold names.
>> + */
>> +static int ovl_strcmp(const char *str, struct ovl_cache_entry *p, int len)
>> +{
>
> Why do you need to re-casefold str on every call to ovl_strcmp? Isn't
> it done in a loop while walking the rbtree with a constant "str" (i.e.,
> the name being added, see ovl_cache_entry_find)? Can't you do it once,
> outside of ovl_strcmp? This way you don't repeatedly allocate/free
> memory for each node of the tree (as Viro mentioned), and you don't have
> to deal with kmalloc failures here.
>
Yes, that's a more reasonable approach, I will do it for v3
>> +
>> + const struct qstr qstr = { .name = str, .len = len };
>> + const char *p_name = p->name, *name = str;
>> + char *dst = NULL;
>> + int cmp, cf_len;
>> +
>> + if (p->cf_name)
>> + p_name = p->cf_name;
>
> This should check IS_ENABLED(CONFIG_UNICODE) so it can be
> compiled out by anyone doing CONFIG_UNICODE=n
>
>> +
>> + if (p->map && !is_dot_dotdot(str, len)) {
>> + dst = kmalloc(OVL_NAME_LEN, GFP_KERNEL);
>> +
>> + /*
>> + * strcmp can't fail, so we fallback to the use the original
>> + * name
>> + */
>> + if (dst) {
>> + cf_len = utf8_casefold(p->map, &qstr, dst, OVL_NAME_LEN);
>
> utf8_casefold can fail, as you know and checked. But if it does, a
> negative cf_len is passed to strncmp and cast to a very high
> value.
>
ops, that's right, thanks for the feedback!
Powered by blists - more mailing lists