Re: [PATCH v2] libfs: Attempt exact-match comparison first during casefold lookup

[Gabriel Krisman Bertazi <krisman@suse.de>]

Eric Biggers <ebiggers@kernel.org> writes:

> On Wed, Jan 17, 2024 at 09:46:18PM -0300, Gabriel Krisman Bertazi wrote:
>> Note that, for strict mode, generic_ci_d_compare used to reject an
>> invalid UTF-8 string, which would now be considered valid if it
>> exact-matches the disk-name.  But, if that is the case, the filesystem
>> is corrupt.  More than that, it really doesn't matter in practice,
>> because the name-under-lookup will have already been rejected by
>> generic_ci_d_hash and we won't even get here.
>
> Can you make the code comment explain this, please?

will do, once we settle the other discussion about this point.
>
>> +	/*
>> +	 * Attempt a case-sensitive match first. It is cheaper and
>> +	 * should cover most lookups, including all the sane
>> +	 * applications that expect a case-sensitive filesystem.
>> +	 */
>> +	if (len == name->len && !memcmp(str, name->name, len))
>> +		return 0;
>
> Is the memcmp() safe, considering that the 'str' can be concurrently
> modified?

You wrote quite a bit of the data race documentation, so you definitely
know better.

But I don't see how this could be a problem.  the str pointer itself
can't change since it's already a copy of the dentry->d_name pointer; if
the string is external, it is guaranteed to exist until the end of the
lookup; Finally, If it's inlined, the string can change concurrently
which, in the worst case scenario, gives us a false result. But then,
VFS will retry, like we do for the case-insensitive comparison right
below.

..Right? :)

-- 
Gabriel Krisman Bertazi