Re: [PATCH v4 1/7] fs: Expose name under lookup to d_revalidate hook

[Gabriel Krisman Bertazi <krisman@suse.de>]

Christian Brauner <brauner@kernel.org> writes:

>> -static inline int d_revalidate(struct dentry *dentry, unsigned int flags)
>> +static inline int d_revalidate(struct dentry *dentry,
>> +			       const struct qstr *name,
>> +			       unsigned int flags)
>>  {
>> -	if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE))
>> +
>> +	if (unlikely(dentry->d_flags & DCACHE_OP_REVALIDATE)) {
>> +		if (dentry->d_op->d_revalidate_name)
>> +			return dentry->d_op->d_revalidate_name(dentry, name, flags);
>>  		return dentry->d_op->d_revalidate(dentry, flags);
>
> This whole sequence got me thinking.
>

...

> ubuntu@imp1-vm:~$ sudo mount -t ecryptfs /mnt/test/casefold-dir /opt
> ubuntu@imp1-vm:/opt$ findmnt | grep opt
> └─/opt  /mnt/test/casefold-dir ecryptfs rw,relatime,ecryptfs_sig=8567ee2ae5880f2d,ecryptfs_cipher=aes,ecryptfs_key_bytes=16,ecryptfs_unlink_sigs

That's interesting.  I was aware of overlayfs and wanted to eventually
get it to work together with casefold, but never considered an ecryptfs combo.

> So it doesn't even seem to care if the underlying filesytem uses a
> custom dentry hash function which seems problematic (So unrelated to
> this change someone should likely explain why that doesn't matter.).
>
> Afaict with your series this will be even more broken because ecryptfs
> and overlayfs call ->d_revalidate() directly.
>
> So this suggests that really you want to extend ->d_revalidate() and we
> should at least similar to overlayfs make ecryptfs reject being mounted
> on casefolding directories and refuse lookup requests for casefolding
> directories.
>
> Ideally we'd explicitly reject by having such fses detect casefolding
> unless it's really enough to reject based on DCACHE_OP_HASH.

Thanks for finding this issue. I'll follow up with merging d_revalidate
and d_revalidate_name and adding a patch to explicitly reject
combinations of ecryptfs/overlayfs with casefolding filesystems, and
safeguard the lookup.

-- 
Gabriel Krisman Bertazi