Re: [PATCH] ext4: propagate errors from fast commit i_blocks replay

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch fails to fix error handling in the second loop of ext4_ext_replay_set_iblocks(), leading to partial block counts being applied to the inode and masked errors.
--

commit 561c6a78544da466ba6d1525d42b474f54d2d473
Author: lty <781735889@qq.com>

ext4: propagate errors from fast commit i_blocks replay

This patch stops ext4_ext_replay_set_iblocks() from calculating partial
block counts when ext4_map_blocks() fails. It leaves i_blocks unchanged
and aborts inode replay on errors.

> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index 76038b6c36552..d67e30cf44adb 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
[ ... ]
> @@ -6329,11 +6329,11 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
>  out:
>  	inode->i_blocks = numblks << (inode->i_sb->s_blocksize_bits - 9);
>  	ext4_mark_inode_dirty(NULL, inode);

[Severity: High]
This isn't a bug introduced by this patch, but does the second loop in
ext4_ext_replay_set_iblocks() still suffer from the same issue?

While this patch correctly captures errors from ext4_map_blocks() in the
first loop, failures during the extent tree block counting phase still jump
to the out label:

	while (cur < end) {
		path = ext4_find_extent(inode, cur, path, 0);
		if (IS_ERR(path))
			break;
		ex = path[path->p_depth].p_ext;
		...
		ret = skip_hole(inode, &cur);
		if (ret < 0)
			break;

If any of these break conditions occur, we hit the out label shown in the
diff context above and blindly overwrite inode->i_blocks with an incomplete
numblks tally.

Additionally, if ext4_find_extent() fails, we don't propagate the error code
because ret might still be 0 from a previous skip_hole() success. This allows
fast commit replay to silently corrupt metadata while returning success.

Should the error paths in the second loop also jump to cleanup instead of
out, and ensure ret is updated to PTR_ERR(path) on lookup failures?

>  cleanup:
>  	ext4_free_ext_path(path);
>  	ext4_free_ext_path(path2);
> -	return 0;
> +	return ret;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/tencent_46F42EDFC0F504046B160E939DB76255EE08@qq.com?part=1