linux-ext4 - Re: [PATCH v4 1/7] fs: Expose name under lookup to d

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87mszg11g3.fsf@suse.de>
Date:   Fri, 28 Jul 2023 11:49:00 -0400
From:   Gabriel Krisman Bertazi <krisman@...e.de>
To:     Christian Brauner <brauner@...nel.org>
Cc:     viro@...iv.linux.org.uk, tytso@....edu, ebiggers@...nel.org,
        jaegeuk@...nel.org, linux-fsdevel@...r.kernel.org,
        linux-ext4@...r.kernel.org, linux-f2fs-devel@...ts.sourceforge.net,
        Gabriel Krisman Bertazi <krisman@...labora.com>
Subject: Re: [PATCH v4 1/7] fs: Expose name under lookup to d_revalidate hook

Christian Brauner <brauner@...nel.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@...1-vm:~$ sudo mount -t ecryptfs /mnt/test/casefold-dir /opt
> ubuntu@...1-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