2024-05-31 01:05:43

by Eric Biggers

[permalink] [raw]
Subject: Re: [PATCH] ext4: add casefolded file check

On Thu, May 30, 2024 at 03:41:50PM +0800, 'Lizhi Xu' via syzkaller-bugs wrote:
> The file name that needs to calculate the siphash must have both flags casefolded
> and dir at the same time, so before calculating it, confirm that the flag meets
> the conditions.
>
> Reported-by: [email protected]
> Signed-off-by: Lizhi Xu <[email protected]>
> ---
> fs/ext4/hash.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/ext4/hash.c b/fs/ext4/hash.c
> index deabe29da7fb..c8840cfc01dd 100644
> --- a/fs/ext4/hash.c
> +++ b/fs/ext4/hash.c
> @@ -265,6 +265,10 @@ static int __ext4fs_dirhash(const struct inode *dir, const char *name, int len,
> __u64 combined_hash;
>
> if (fscrypt_has_encryption_key(dir)) {
> + if (!IS_CASEFOLDED(dir)) {
> + ext4_warning_inode(dir, "Siphash requires Casefolded file");
> + return -2;
> + }
> combined_hash = fscrypt_fname_siphash(dir, &qname);
> } else {
> ext4_warning_inode(dir, "Siphash requires key");

First, this needs to be sent to the ext4 mailing list (and not to irrelevant
mailing lists such as netdev). Please use ./scripts/get_maintainer.pl, as is
recommended by Documentation/process/submitting-patches.rst.

Second, ext4 already checks for the directory being casefolded before allowing
siphash. This is done by dx_probe(). Evidently syzbot found some way around
that, so what needs to be done is figure out why that happened and what is the
best fix to prevent it. This is not necessarily the patch you've proposed, as
the real issue might actually be a missing check at some earlier time like when
reading the inode from disk or when mounting the filesystem.

- Eric