Re: [PATCH 08/27] ext4: zero s_fc_replay_state in ext4_fc_replay_cleanup()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] ext4: missing s_ea_inode_work flush before
  ext4_fc_replay_cleanup() [1]
--

commit a4ccfbc7c2db4e8403230f07464457a51d148eaa
Author: Theodore Ts'o <tytso@mit.edu>

ext4: zero s_fc_replay_state in ext4_fc_replay_cleanup()

This commit zeroes sbi->s_fc_replay_state in ext4_fc_replay_cleanup()
after freeing fc_regions and fc_modified_inodes so that repeated calls to
ext4_fc_replay_cleanup() are idempotent.

> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 0cac890cf3702..15bd040c2329b 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -2398,6 +2398,7 @@ void ext4_fc_replay_cleanup(struct super_block *sb)
>  	sbi->s_mount_state &= ~EXT4_FC_REPLAY;
>  	kfree(sbi->s_fc_replay_state.fc_regions);
>  	kfree(sbi->s_fc_replay_state.fc_modified_inodes);
> +	memset(&sbi->s_fc_replay_state, 0, sizeof(sbi->s_fc_replay_state));
>  }

[Severity: High]
This problem wasn't introduced by this patch, but can
ext4_fc_replay_cleanup() clear EXT4_FC_REPLAY before sbi->s_ea_inode_work
is flushed [1]?

When fast-commit replay unlinks an inode that owns an extended attribute
inode, ext4_xattr_inode_dec_ref_all() -> ext4_put_ea_inode() queues the EA
inode onto sbi->s_ea_inode_to_free and schedules sbi->s_ea_inode_work.
Unlike the failed_mount3a error path, the normal mount path in
__ext4_fill_super() calls ext4_fc_replay_cleanup() without flushing
sbi->s_ea_inode_work first, clearing EXT4_FC_REPLAY before ext4_mb_init()
initializes sbi->s_group_info:

fs/ext4/super.c:__ext4_fill_super() {
	...
	ext4_fc_replay_cleanup(sb);

	ext4_ext_init(sb);

	/* Register extent status tree shrinker */
	err = ext4_es_register_shrinker(sbi);
	if (err)
		goto failed_mount3;

	err = ext4_Inode_table_init(sb);
	...
	err = ext4_mb_init(sb);
	...
failed_mount3a:
	/* Drain deferred EA inode iputs from journal replay */
	flush_delayed_work(&sbi->s_ea_inode_work);
	ext4_fc_replay_cleanup(sb);
	...
}

If ext4_ea_inode_work() runs before ext4_mb_init() populates
sbi->s_group_info (or when mount failure jumps to failed_mount_wq after
ext4_mb_release()), ext4_evict_inode() -> ext4_free_inode() runs with
EXT4_FC_REPLAY cleared and ext4_get_group_info() returning NULL, which
triggers ext4_std_error(sb, -EFSCORRUPTED).

Should ext4_fc_replay_cleanup() (or __ext4_fill_super() right before it)
call flush_delayed_work(&sbi->s_ea_inode_work) before clearing
EXT4_FC_REPLAY?

[1] https://sashiko.dev/#/bug/linux-f893d2ed-6353-413a-8dd1-c6bc439b880a

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261112040638.1576836-1-tytso@mit.edu?part=8