Re: [PATCH] ext4: fix OOB write in fast-commit replay

[Jan Kara]

On Sun 08-11-26 20:43:46, Jérémy Jean wrote:
> Fast-commit recovery checks record bounds and lengths during scan.
> Replay reads the records again without these checks. A corrupt filesystem
> can make an earlier replay operation overwrite a later record. Its new

This fact looks like the real problem. I think we should validate the
replay does not corrupt the journal. How does this happen? Otherwise we
have to fully redo the validation of each record done in
ext4_fc_replay_scan().

								Honza

> length can make ext4_fc_replay_inode() copy past the end of an inode buffer.
> The OOB write can trigger KASAN, which reports it as a UAF:
> 
>   BUG: KASAN: slab-use-after-free in ext4_fc_replay+0x1725/0x2de0
>   Write of size 200 at addr ffff888008598f64 by task busybox/62
>   Call Trace:
>    __asan_memcpy+0x3c/0x60
>    ext4_fc_replay+0x1725/0x2de0
>    do_one_pass+0x113c/0x2370
>    jbd2_journal_recover+0x528/0x7e0
>    jbd2_journal_load+0x7c/0x810
>    ext4_fill_super+0xaa54/0xc270
> 
> Check record bounds and lengths again before replaying each record.
> 
> Fixes: 8016e29f4362 ("ext4: fast commit recovery path")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Jérémy Jean <Jeremy.Jean@xxxxxxxxxxxxxxxxx>
> ---
>  fs/ext4/fast_commit.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/fs/ext4/fast_commit.c b/fs/ext4/fast_commit.c
> index 0cac890..9edd26a 100644
> --- a/fs/ext4/fast_commit.c
> +++ b/fs/ext4/fast_commit.c
> @@ -2613,6 +2613,11 @@ static int ext4_fc_replay(journal_t *journal, struct buffer_head *bh,
>  			ext4_fc_set_bitmaps_and_counters(sb);
>  			break;
>  		}
> +		if (tl.fc_len > end - val ||
> +		    !ext4_fc_value_len_isvalid(sbi, tl.fc_tag, tl.fc_len)) {
> +			ret = -EFSCORRUPTED;
> +			break;
> +		}
>  
>  		ext4_debug("Replay phase, tag:%s\n", tag2str(tl.fc_tag));
>  		state->fc_replay_num_tags--;
> -- 
> 2.47.3
> 
-- 
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR