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  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]
Date:   Wed, 11 Jul 2018 16:30:46 -0400
From:   Gabriel Krisman Bertazi <>
To:     "Theodore Y. Ts'o" <>
Subject: Re: [PATCH 20/20] ext4: Implement encoding-aware dcache hooks

"Theodore Y. Ts'o" <> 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;
>> +


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

Powered by blists - more mailing lists