[PATCH v2] ext4: propagate errors from fast commit block cleanup

[PATCH v2] ext4: propagate errors from fast commit block cleanup

[781735889]

From: lty <781735889@qq.com>

ext4_ext_clear_bb() stops scanning when ext4_map_blocks() fails, but it
returns success to its caller. It also ignores failures from extent lookup
and replay-region allocation. ext4_fc_replay_inode() ignores the helper
result and continues replay with stale block bitmap accounting.

Propagate all cleanup errors from ext4_ext_clear_bb() and abort inode
replay when cleanup fails.

Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
Cc: stable@vger.kernel.org

Testing: QEMU fast-commit replay with an injected -EIO at the
ext4_ext_clear_bb() mapping call. The unpatched kernel completed recovery
and e2fsck reported a free-block count mismatch; the patched kernel aborted
recovery with EIO. Also built the ext4 target objects with the expanded
error paths.

Signed-off-by: lty <781735889@qq.com>
---
Changes in v2:
- Propagate errors from extent lookup and replay-region recording.
- Abort inode replay when block cleanup fails.

 fs/ext4/extents.c     | 31 ++++++++++++++++++-------------
 fs/ext4/fast_commit.c |  5 ++++-
 2 files changed, 22 insertions(+), 14 deletions(-)

diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index f8c805d80c33..ba6dee8829a5 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -6369,29 +6369,34 @@ int ext4_ext_clear_bb(struct inode *inode)
 		map.m_len = end - cur;
 		ret = ext4_map_blocks(NULL, inode, &map, 0);
 		if (ret < 0)
-			break;
+			goto out;
 		if (ret > 0) {
 			path = ext4_find_extent(inode, map.m_lblk, path, 0);
-			if (!IS_ERR(path)) {
-				for (j = 0; j < path->p_depth; j++) {
-					ext4_mb_mark_bb(inode->i_sb,
-							path[j].p_block, 1, false);
-					ext4_fc_record_regions(inode->i_sb, inode->i_ino,
-							0, path[j].p_block, 1, 1);
-				}
-			} else {
-				path = NULL;
+			if (IS_ERR(path)) {
+				ret = PTR_ERR(path);
+				goto out;
+			}
+			for (j = 0; j < path->p_depth; j++) {
+				ext4_mb_mark_bb(inode->i_sb,
+						path[j].p_block, 1, false);
+				ret = ext4_fc_record_regions(inode->i_sb,
+							     inode->i_ino, 0,
+							     path[j].p_block, 1, 1);
+				if (ret)
+					goto out;
 			}
 			ext4_mb_mark_bb(inode->i_sb, map.m_pblk, map.m_len, false);
-			ext4_fc_record_regions(inode->i_sb, inode->i_ino,
-					map.m_lblk, map.m_pblk, map.m_len, 1);
+			ret = ext4_fc_record_regions(inode->i_sb, inode->i_ino,
+						     map.m_lblk, map.m_pblk, map.m_len, 1);
+			if (ret)
+				goto out;
 		}
 		cur = cur + map.m_len;
 	}
 
 out:
 	ext4_free_ext_path(path);
-	return 0;
+	return ret < 0 ? ret : 0;
 }
 
 #if IS_ENABLED(CONFIG_EXT4_KUNIT_TESTS)
diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
index 8110b01e36c1..8c24e882aee8 100644
--- a/fs/ext4/fast_commit.c
+++ b/fs/ext4/fast_commit.c
@@ -1941,8 +1941,11 @@ static int ext4_fc_replay_inode(struct super_block *sb,
 
 	inode = ext4_iget(sb, ino, EXT4_IGET_NORMAL);
 	if (!IS_ERR(inode)) {
-		ext4_ext_clear_bb(inode);
+		ret = ext4_ext_clear_bb(inode);
 		iput(inode);
+		inode = NULL;
+		if (ret)
+			goto out;
 	}
 	inode = NULL;

-- 
2.34.1
 

Re: [PATCH v2] ext4: propagate errors from fast commit block cleanup

[sashiko-bot]

> From: lty <781735889@qq.com>
> 
> ext4_ext_clear_bb() stops scanning when ext4_map_blocks() fails, but it
> returns success to its caller. It also ignores failures from extent lookup
> and replay-region allocation. ext4_fc_replay_inode() ignores the helper
> result and continues replay with stale block bitmap accounting.
> 
> Propagate all cleanup errors from ext4_ext_clear_bb() and abort inode
> replay when cleanup fails.
> 
> Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
> Cc: stable@vger.kernel.org
> 
> Testing: QEMU fast-commit replay with an injected -EIO at the
> ext4_ext_clear_bb() mapping call. The unpatched kernel completed recovery
> and e2fsck reported a free-block count mismatch; the patched kernel aborted
> recovery with EIO. Also built the ext4 target objects with the expanded
> error paths.
> 
> Signed-off-by: lty <781735889@qq.com>

Sashiko has reviewed this patch and found no issues. It looks great!

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