[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <5e9c229f-cbd9-4fcb-a349-4605e52f13a1@huaweicloud.com>
Date: Mon, 29 Sep 2025 20:07:20 +0800
From: Zhang Yi <yi.zhang@...weicloud.com>
To: Deepanshu Kartikey <kartikey406@...il.com>
Cc: tytso@....edu, adilger.kernel@...ger.ca, linux-ext4@...r.kernel.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH] ext4: validate extent entries before caching in
ext4_find_extent()
On 9/29/2025 1:27 PM, Deepanshu Kartikey wrote:
> Zhang Yi,
>
> You're correct that ext4_ext_check_inode() should catch this. I investigated and found why it doesn't:
> [ 18.824142] DEBUG: Validating inode 2 (no inline data)
> [ 18.835777] DEBUG: verity inode 15, inline=0, extents=1
> [ 18.836793] DEBUG: Skipping validation for inode 15 (has inline data)
>
> The verity inode reports inline=0 when checking the flag directly, but ext4_has_inline_data() returns true at the validation check, causing validation to be skipped.
This makes me confused, how can this inode report inline=0 when checking
the flag directly, but ext4_has_inline_data() returns true?
Does this mean that this inode has both the EXT4_INODE_EXTENTS and
EXT4_INODE_INLINE_DATA flags set? If so, we should detect this in
ext4_iget() and call ext4_error_inode().
>
> This corrupted filesystem has a verity file that somehow triggers the inline data check to return true, even though verity files should never have inline data. This allows the corrupted out-of-order extents to bypass validation.
>
> My patch adds validation before caching extents, which catches such corruption regardless of why the inline data check fails. This provides necessary defense against corrupted filesystems at the point where extents are actually used.
Generally speaking, we should avoid redundant checks. It is sufficient to
verify the metadata after reading it from the disk, without considering
scenarios which intentionally corrupted the metadata by directly writing
to the bdev. Adding checks in ext4_find_extent() would introduce
unnecessary overhead.
Thanks,
Yi.
Powered by blists - more mailing lists