linux-ext4 - Re: [PATCH v6 4/8] ext4: Reuse generic_ci

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: <YoW8yx9Fw9Rwiaja@sol.localdomain>
Date:   Wed, 18 May 2022 20:43:07 -0700
From:   Eric Biggers <ebiggers@...nel.org>
To:     Gabriel Krisman Bertazi <krisman@...labora.com>
Cc:     tytso@....edu, adilger.kernel@...ger.ca, jaegeuk@...nel.org,
        linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
        kernel@...labora.com
Subject: Re: [PATCH v6 4/8] ext4: Reuse generic_ci_match for ci comparisons

On Wed, May 18, 2022 at 09:40:40PM -0400, Gabriel Krisman Bertazi wrote:
> Instead of reimplementing ext4_match_ci, use the new libfs helper.
> 
> It should be fine to drop the fname->cf_name in the encrypted directory
> case for the hash verification optimization because the only two ways
> for fname->cf_name to be NULL on a case-insensitive lookup is
> 
>  (1) if name under lookup has an invalid encoding and the FS is not in
>  strict mode; or
> 
>  (2) if the directory is encrypted and we don't have the
>  key.
> 
> For case (1), it doesn't matter, because the lookup hash will be
> generated with fname->usr_name, the same as the disk (fallback to
> invalid encoding behavior on !strict mode).  Case (2) is caught by the
> previous check (!IS_ENCRYPTED(parent) ||
> fscrypt_has_encryption_key(parent)), so we never reach this code.

The code actually can be reached in case (2), because the key could have been
added between ext4_fname_setup_ci_filename() and ext4_match().

I *think* your change doesn't make it any worse, since in such a case the name
comparison is going to be comparing a no-key name to a regular one, which will
very likely fail.  So adding an additional way for the match to fail seems fine.

It's hard to reason about, though.  f2fs does things in a much cleaner way, as
I've mentioned before, since it decides which type of match it wants at the
beginning, when initializing struct f2fs_filename.

- Eric