If the call to ext4_map_blocks() fails due to an corrupted file
system, ext4_ext_replay_set_iblocks() can get stuck in an infinite
loop. This could be reproduced by running generic/526 with a file
system that has inline_data and fast_commit enabled. The system will
repeatedly log to the console:
EXT4-fs warning (device dm-3): ext4_block_to_path:105: block 1074800922 > max in inode 131076
and the stack that it gets stuck in is:
ext4_block_to_path+0xe3/0x130
ext4_ind_map_blocks+0x93/0x690
ext4_map_blocks+0x100/0x660
skip_hole+0x47/0x70
ext4_ext_replay_set_iblocks+0x223/0x440
ext4_fc_replay_inode+0x29e/0x3b0
ext4_fc_replay+0x278/0x550
do_one_pass+0x646/0xc10
jbd2_journal_recover+0x14a/0x270
jbd2_journal_load+0xc4/0x150
ext4_load_journal+0x1f3/0x490
ext4_fill_super+0x22d4/0x2c00
With this patch, generic/526 still fails, but system is no longer
locking up in a tight loop. It's likely the root casue is that
fast_commit replay is corrupting file systems with inline_data, and we
probably need to add better error handling in the fast commit replay
code path beyond what is done here, which essentially just breaks the
infinite loop without reporting the to the higher levels of the code.
Fixes: 8016E29F4362 ("ext4: fast commit recovery path")
Cc: [email protected]
Cc: Harshad Shirwadkar <[email protected]>
Signed-off-by: Theodore Ts'o <[email protected]>
---
fs/ext4/extents.c | 19 ++++++++++++++-----
1 file changed, 14 insertions(+), 5 deletions(-)
diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
index eb1dd4f024f2..e57019cc3601 100644
--- a/fs/ext4/extents.c
+++ b/fs/ext4/extents.c
@@ -5913,7 +5913,7 @@ void ext4_ext_replay_shrink_inode(struct inode *inode, ext4_lblk_t end)
}
/* Check if *cur is a hole and if it is, skip it */
-static void skip_hole(struct inode *inode, ext4_lblk_t *cur)
+static int skip_hole(struct inode *inode, ext4_lblk_t *cur)
{
int ret;
struct ext4_map_blocks map;
@@ -5922,9 +5922,12 @@ static void skip_hole(struct inode *inode, ext4_lblk_t *cur)
map.m_len = ((inode->i_size) >> inode->i_sb->s_blocksize_bits) - *cur;
ret = ext4_map_blocks(NULL, inode, &map, 0);
+ if (ret < 0)
+ return ret;
if (ret != 0)
- return;
+ return 0;
*cur = *cur + map.m_len;
+ return 0;
}
/* Count number of blocks used by this inode and update i_blocks */
@@ -5973,7 +5976,9 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
* iblocks by total number of differences found.
*/
cur = 0;
- skip_hole(inode, &cur);
+ ret = skip_hole(inode, &cur);
+ if (ret < 0)
+ goto out;
path = ext4_find_extent(inode, cur, NULL, 0);
if (IS_ERR(path))
goto out;
@@ -5992,8 +5997,12 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
}
cur = max(cur + 1, le32_to_cpu(ex->ee_block) +
ext4_ext_get_actual_len(ex));
- skip_hole(inode, &cur);
-
+ ret = skip_hole(inode, &cur);
+ if (ret < 0) {
+ ext4_ext_drop_refs(path);
+ kfree(path);
+ break;
+ }
path2 = ext4_find_extent(inode, cur, NULL, 0);
if (IS_ERR(path2)) {
ext4_ext_drop_refs(path);
--
2.31.0
Thanks for the patch Ted. Looks good to me.
Reviewed-by: Harshad Shirwadkar <[email protected]>
On Thu, Sep 2, 2021 at 8:51 AM Theodore Ts'o <[email protected]> wrote:
>
> If the call to ext4_map_blocks() fails due to an corrupted file
> system, ext4_ext_replay_set_iblocks() can get stuck in an infinite
> loop. This could be reproduced by running generic/526 with a file
> system that has inline_data and fast_commit enabled. The system will
> repeatedly log to the console:
>
> EXT4-fs warning (device dm-3): ext4_block_to_path:105: block 1074800922 > max in inode 131076
>
> and the stack that it gets stuck in is:
>
> ext4_block_to_path+0xe3/0x130
> ext4_ind_map_blocks+0x93/0x690
> ext4_map_blocks+0x100/0x660
> skip_hole+0x47/0x70
> ext4_ext_replay_set_iblocks+0x223/0x440
> ext4_fc_replay_inode+0x29e/0x3b0
> ext4_fc_replay+0x278/0x550
> do_one_pass+0x646/0xc10
> jbd2_journal_recover+0x14a/0x270
> jbd2_journal_load+0xc4/0x150
> ext4_load_journal+0x1f3/0x490
> ext4_fill_super+0x22d4/0x2c00
>
> With this patch, generic/526 still fails, but system is no longer
> locking up in a tight loop. It's likely the root casue is that
> fast_commit replay is corrupting file systems with inline_data, and we
> probably need to add better error handling in the fast commit replay
> code path beyond what is done here, which essentially just breaks the
> infinite loop without reporting the to the higher levels of the code.
>
> Fixes: 8016E29F4362 ("ext4: fast commit recovery path")
> Cc: [email protected]
> Cc: Harshad Shirwadkar <[email protected]>
> Signed-off-by: Theodore Ts'o <[email protected]>
> ---
> fs/ext4/extents.c | 19 ++++++++++++++-----
> 1 file changed, 14 insertions(+), 5 deletions(-)
>
> diff --git a/fs/ext4/extents.c b/fs/ext4/extents.c
> index eb1dd4f024f2..e57019cc3601 100644
> --- a/fs/ext4/extents.c
> +++ b/fs/ext4/extents.c
> @@ -5913,7 +5913,7 @@ void ext4_ext_replay_shrink_inode(struct inode *inode, ext4_lblk_t end)
> }
>
> /* Check if *cur is a hole and if it is, skip it */
> -static void skip_hole(struct inode *inode, ext4_lblk_t *cur)
> +static int skip_hole(struct inode *inode, ext4_lblk_t *cur)
> {
> int ret;
> struct ext4_map_blocks map;
> @@ -5922,9 +5922,12 @@ static void skip_hole(struct inode *inode, ext4_lblk_t *cur)
> map.m_len = ((inode->i_size) >> inode->i_sb->s_blocksize_bits) - *cur;
>
> ret = ext4_map_blocks(NULL, inode, &map, 0);
> + if (ret < 0)
> + return ret;
> if (ret != 0)
> - return;
> + return 0;
> *cur = *cur + map.m_len;
> + return 0;
> }
>
> /* Count number of blocks used by this inode and update i_blocks */
> @@ -5973,7 +5976,9 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
> * iblocks by total number of differences found.
> */
> cur = 0;
> - skip_hole(inode, &cur);
> + ret = skip_hole(inode, &cur);
> + if (ret < 0)
> + goto out;
> path = ext4_find_extent(inode, cur, NULL, 0);
> if (IS_ERR(path))
> goto out;
> @@ -5992,8 +5997,12 @@ int ext4_ext_replay_set_iblocks(struct inode *inode)
> }
> cur = max(cur + 1, le32_to_cpu(ex->ee_block) +
> ext4_ext_get_actual_len(ex));
> - skip_hole(inode, &cur);
> -
> + ret = skip_hole(inode, &cur);
> + if (ret < 0) {
> + ext4_ext_drop_refs(path);
> + kfree(path);
> + break;
> + }
> path2 = ext4_find_extent(inode, cur, NULL, 0);
> if (IS_ERR(path2)) {
> ext4_ext_drop_refs(path);
> --
> 2.31.0
>