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
| ||
|
Date: Thu, 19 May 2022 15:52:10 -0400 From: Gabriel Krisman Bertazi <krisman@...labora.com> To: Eric Biggers <ebiggers@...nel.org> 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 Eric Biggers <ebiggers@...nel.org> writes: > 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(). Hm, I see! I didn't understand it would be possible to add a key during a lookup from your previous explanation, thanks for clarifying. > 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. Either way, no point in setting it for failure. I will restore the fname->cf_name != NULL check. > 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. Yes, this is quite confusing. Are these implementation documented anywhere? Thank you for the review! -- Gabriel Krisman Bertazi
Powered by blists - more mailing lists