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
| ||
|
Message-ID: <20240125030530.GB52073@sol.localdomain> Date: Wed, 24 Jan 2024 19:05:30 -0800 From: Eric Biggers <ebiggers@...nel.org> To: Gabriel Krisman Bertazi <krisman@...e.de> Cc: viro@...iv.linux.org.uk, jaegeuk@...nel.org, tytso@....edu, linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net, linux-fsdevel@...r.kernel.org, amir73il@...il.com Subject: Re: [PATCH v3 02/10] fscrypt: Share code between functions that prepare lookup On Fri, Jan 19, 2024 at 03:47:34PM -0300, Gabriel Krisman Bertazi wrote: > To make the patch simpler, we now call fscrypt_get_encryption_info twice > for fscrypt_prepare_lookup, once inside fscrypt_setup_filename and once > inside fscrypt_prepare_lookup_dentry. It seems safe to do, and > considering it will bail early in the second lookup and most lookups > should go to the dcache anyway, it doesn't seem problematic for > performance. In addition, we add a function call for the unencrypted > case, also during lookup. Unfortunately I don't think it's correct. This is basically undoing my fix b01531db6cec ("fscrypt: fix race where ->lookup() marks plaintext dentry as ciphertext") from several years ago. When a lookup is done, the filesystem needs to either treat the name being looked up as a no-key name *or* as a regular name, depending on whether the directory's key is present. We shouldn't enable race conditions where, due to the key being concurrently added, the name is treated as a no-key name for filename matching purposes but a regular name for dentry validation purposes. That can result in an anomaly where a file that exists ends up with a negative dentry that doesn't get invalidated. Basically, the boolean fscrypt_name::is_nokey_name that's produced by fscrypt_setup_filename() should continue to be propagated to DCACHE_NOKEY_NAME. - Eric
Powered by blists - more mailing lists