"Theodore Y. Ts'o" <tytso@xxxxxxx> writes: > On Tue, Jul 03, 2018 at 01:07:00PM -0400, Gabriel Krisman Bertazi wrote: >> diff --git a/fs/ext4/super.c b/fs/ext4/super.c >> index 53db9b6c7e33..f292cc5bacda 100644 >> --- a/fs/ext4/super.c >> +++ b/fs/ext4/super.c >> @@ -4358,6 +4358,10 @@ static int ext4_fill_super(struct super_block *sb, void *data, int silent) >> iput(root); >> goto failed_mount4; >> } >> + >> + if (sbi->encoding) >> + sb->s_d_op = &ext4_dentry_ops; >> + Hello, Thanks for the review. > > This is going to be potentially problematic for fscrypt (e.g., when > ext4's encryption is enabled, as will be the case in Android's File > Based Encryption). See the call to d_set_d_op() in > __fscrypt_prepare_lookup in fs/crypto/hooks.c, since d_set_d_op is > going to overwrite sb->s_d_op. > > This probably means that the fscrypt code is going to have to be > case-sensitivity aware. Or we would have to make case folding and fs > encryption to be mutually exclusive, which would be unfortunate, since > Android is going to be a potential user of case folding. Yes, I am aware of that issue with fscrypt's d_revalidate, the smoke tests helped me catch it beforehand. I worked around it on a different branch by implementing a wrapper around my own d_revalidate, and removing it from fscrypt. I didn't send it along with this patchset because, as you mentioned, fscrypt needs to become case-sensitive aware (in fact, encoding-aware), which means we need to either also store the normalized version of the encrypted file name in the dentry, or do a much more expensive search in the directory, by decrypting each name before comparison. I'm doing the second option now, (decrypting the filename at comparison time), but I considered this non-trivial change to be a future step, which is not ready for submission. My approach in the current patchset is make these features mutually exclusive for now, as you can see in patch 17. I'm refusing to mount with encoding any superblock that has the encryption feature enabled. I know it is not a final solution for Android and Valve usecases, but I think this support can be integrated at a later moment without breaking the abi. -- 9Gabriel Krisman Bertazi
