linux-ext4 - Re: [PATCH v3 0/7] Support negative dentries on case-insensitive ext4 and f2fs

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <87y1ja4hau.fsf@suse.de>
Date:   Thu, 20 Jul 2023 13:35:37 -0400
From:   Gabriel Krisman Bertazi <krisman@...e.de>
To:     Eric Biggers <ebiggers@...nel.org>
Cc:     viro@...iv.linux.org.uk, brauner@...nel.org, tytso@....edu,
        jaegeuk@...nel.org, linux-fsdevel@...r.kernel.org,
        linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net
Subject: Re: [PATCH v3 0/7] Support negative dentries on case-insensitive
 ext4 and f2fs

Eric Biggers <ebiggers@...nel.org> writes:

>> Another problem exists when turning a negative dentry to positive.  If
>> the negative dentry has a different case than what is currently being
>> used for lookup, the dentry cannot be reused without changing its name,
>> in order to guarantee filename-preserving semantics to userspace.  We
>> need to either change the name or invalidate the dentry. This issue is
>> currently avoided in mainline, since the negative dentry mechanism is
>> disabled.
>
> Are you sure this problem even needs to be solved?

Yes, because we promise name-preserving semantics.  If we don't do it,
the name on the disk might be different than what was asked for, and tools
that rely on it (they exist) will break.  During initial testing, I've
had tools breaking with case-insensitive ext4 because they created a
file, did getdents and wanted to see exactly the name they used.

> It actually isn't specific to negative dentries.  If you have a file "foo"
> that's not in the dcache, and you open it (or look it up in any other way) as
> "FOO", then the positive dentry that gets created is named "FOO".
>
> As a result, the name that shows up in /proc/$pid/fd/ for anyone who has the
> file open is "FOO", not the true name "foo".  This is true even for processes
> that open it as "foo", as long as the dentry remains in the dcache.
>
> No negative dentries involved at all!

I totally agree it is goes beyond negative dentries, but this case is
particularly important because it is the only one (that I know of) where
the incorrect case might actually be written to the disk.  other cases,
like /proc/<pid>/fd/ can just display a different case to userspace,
which is confusing.  Still, the disk has the right version, exactly as
originally created.

I see the current /proc/$pid/fd/ semantics as a bug. In fact, I have/had
a bug report for bwrap/flatkpak breaking because it was mounting
something and immediately checking /proc/mounts to confirm it worked.  A
former team member tried fixing it a while ago, but we didn't follow up,
and I don't really love the way they did it.  I need to look into that.

> Or, it looks like the positive dentry case is solvable using d_add_ci().
> So maybe you are planning to do that?  It's not clear to me.

I want to use d_add_ci for the future, yes. It is not hard, but not
trivial, because there is an infinite recursion if d_add_ci uses
->d_compare() when doing the lookup for a duplicate.  We sent a patch to
fix d_add_ci a while ago, but it was rejected.  I need to revisit.

-- 
Gabriel Krisman Bertazi