linux-kernel - Re: [PATCH 1/2] fs/dcache: introduce d_alloc_parallel_check

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 for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87jzg9wjeo.fsf@mailhost.krisman.be>
Date: Wed, 21 Aug 2024 19:22:39 -0400
From: Gabriel Krisman Bertazi <krisman@...e.de>
To: Eugen Hristev <eugen.hristev@...labora.com>
Cc: viro@...iv.linux.org.uk,  brauner@...nel.org,  tytso@....edu,
  linux-ext4@...r.kernel.org,  jack@...e.cz,  adilger.kernel@...ger.ca,
  linux-fsdevel@...r.kernel.org,  linux-kernel@...r.kernel.org,
  kernel@...labora.com,  shreeya.patel@...labora.com
Subject: Re: [PATCH 1/2] fs/dcache: introduce d_alloc_parallel_check_existing

Eugen Hristev <eugen.hristev@...labora.com> writes:

> Yes, but we cannot add another dentry for the same file with a different case.
> That would break everything about dentry lookups, etc.
> We need to have the one dentry in the cache which use the right case. Regardless of
> the case of the lookup.
>
> As Al Viro said here :
> https://lore.kernel.org/lkml/YVmyYP25kgGq9uEy@zeniv-ca.linux.org.uk/
> we cannot have parallel lookups for names that would compare as equals (two
> different dentries for the same file with different case).
>
> So yes, I return the same dentry-under-lookup, because that's the purpose of that
> search, return it, have it use the right case, and then splice it to the cache.

It is not changing the case of the returned dentry.  The patch simply
returns the same dentry you sent to d_alloc_parallel, which is then
spliced into the cache. Exactly as if you had issued d_splice_alias
directly.  You are just doing a hop in d_alloc_parallel and finding the
same dentry.

A quick test case below. You can print the ->d_name through
several methods. I'm doing it by reading /proc/self/cwd.

$ # In a case-insensitive filesystem
$ mkdir cf &&  chattr +F cf
$ mkdir cf/hello
$ echo 3 > /proc/sys/vm/drop_caches    # drop the dentry created above
$ cd cf/HELLO                          # provoke a case-inexact lookup.
$ readlink /proc/self/cwd

If we replaced the dentry with the disk name, it should
print <mnt>/cf/hello.  With your patch, it still prints <mnt>/cf/HELLO

Al,

Would it be acceptable to just change the dentry->d_name here in a new
flavor of d_add_ci used only by these filesystems? We are inside the
creation path, so the dentry has never been hashed.  Concurrent lookups
will be stuck in d_wait_lookup() until we are done and will never become
invalid after the change because the lookup was already done
case-insensitively, so they all match the same dentry, per-definition,
and we know there is no other matching dentries in the directory.  We'd
only need to be careful not to expose partial names to concurrent
parallel lookups.

-- 
Gabriel Krisman Bertazi