Re: [PATCH v3] ext4: validate extent root during fast commit replay

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential
issues to consider:
- [Medium] ext4: ignored extent check errors in ext4_fc_replay_inode()
- [Medium] ext4: missing EXT4_INODE_EXTENTS check in fast commit range
  replay
--

commit c7e3408ccbd89c7e858695dd9961091a529765cf
Author: Jérémy Jean <Jeremy.Jean@oss.cyber.gouv.fr>

ext4: validate extent root during fast commit replay

This commit validates the root extent header in ext4_find_extent() during
fast commit replay, skips extent replay helpers when EXT4_INODE_EXTENTS is
not set, and clears EXT4_FC_REPLAY before loading the root inode.

> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 0cac890cf3702..b883ce015c303 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -2002,7 +2002,7 @@ static int ext4_fc_replay_inode(struct super_block *sb,
>  	 * crashing. This should be fixed but until then, we calculate
>  	 * the number of blocks the inode.
>  	 */
> -	if (!ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA))
> +	if (ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS))
>  		ext4_ext_replay_set_iblocks(inode);
>  
>  	inode->i_generation = le32_to_cpu(ext4_raw_inode(&iloc)->i_generation);

[Severity: Medium]
Should ext4_fc_replay_inode() check the return values of
ext4_ext_clear_bb() and ext4_ext_replay_set_iblocks()?

When the new ext4_ext_check() in ext4_find_extent() rejects a corrupt root
extent header with valid eh_magic (such as eh_entries = 4 and eh_max = 5),
both helpers return -EFSCORRUPTED, but ext4_fc_replay_inode() ignores the
error, computes a valid checksum over the corrupt i_block via
ext4_inode_csum_set(), and writes the inode to disk.

> @@ -2334,7 +2334,7 @@ static void ext4_fc_set_bitmaps_and_counters(struct super_block *sb)
>  		}
>  		cur = 0;
>  		end = EXT_MAX_BLOCKS;
> -		if (ext4_test_inode_flag(inode, EXT4_INODE_INLINE_DATA)) {
> +		if (!ext4_test_inode_flag(inode, EXT4_INODE_EXTENTS)) {
>  			iput(inode);
>  			continue;
>  		}

[Severity: Medium]
Do ext4_fc_replay_add_range() and ext4_fc_replay_del_range() also need to
verify EXT4_INODE_EXTENTS before replaying range tags?

If an EXT4_FC_TAG_ADD_RANGE or EXT4_FC_TAG_DEL_RANGE tag targets an inode
without EXT4_INODE_EXTENTS set, ext4_map_blocks() dispatches to
ext4_ind_map_blocks() while the subsequent calls (ext4_find_extent(),
ext4_ext_insert_extent(), ext4_ext_replay_update_ex(),
ext4_ext_remove_space(), and ext4_ext_replay_shrink_inode()) treat i_data
as an extent tree.

In ext4_fc_replay_del_range(), any blocks reported mapped by
ext4_ind_map_blocks() are cleared in the block bitmap via
ext4_mb_mark_bb() and are no longer restored by
ext4_fc_set_bitmaps_and_counters() now that it skips non-extent inodes.
In ext4_fc_replay_add_range(), if ext4_ind_map_blocks() reports a hole for
a logical block that is already present in an extent header in i_data,
ext4_ext_insert_extent() hits BUG_ON(newext->ee_block == nearex->ee_block).

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261109221211.1793790-2-Jeremy.Jean@oss.cyber.gouv.fr?part=1