Received-SPF: pass (google.com: domain of linux-ext4-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Gabriel Krisman Bertazi <krisman@suse.de>
To:     Christian Brauner <brauner@kernel.org>
Cc:     viro@zeniv.linux.org.uk, tytso@mit.edu, ebiggers@kernel.org,
        jaegeuk@kernel.org, linux-fsdevel@vger.kernel.org,
        linux-ext4@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net,
        Gabriel Krisman Bertazi <krisman@collabora.com>
Subject: Re: [PATCH v4 1/7] fs: Expose name under lookup to d_revalidate hook
Organization: SUSE
References: <20230727172843.20542-1-krisman@suse.de>
        <20230727172843.20542-2-krisman@suse.de>
        <20230728-unrentabel-volumen-1500701f2524@brauner>
Date:   Fri, 28 Jul 2023 11:49:00 -0400
In-Reply-To: <20230728-unrentabel-volumen-1500701f2524@brauner> (Christian
        Brauner's message of "Fri, 28 Jul 2023 16:00:10 +0200")
Message-ID: <87mszg11g3.fsf@suse.de>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/28.2 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Christian Brauner <brauner@kernel.org> writes:

>> -static inline int d_revalidate(struct dentry *dentry, unsigned int flag=
s)
>> +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
> =E2=94=94=E2=94=80/opt  /mnt/test/casefold-dir ecryptfs rw,relatime,ecryp=
tfs_sig=3D8567ee2ae5880f2d,ecryptfs_cipher=3Daes,ecryptfs_key_bytes=3D16,ec=
ryptfs_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 com=
bo.

> 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.

--=20
Gabriel Krisman Bertazi