On Fri 14-07-23 10:55:27, Zhang Yi wrote:
> From: Zhihao Cheng <chengzhihao1@xxxxxxxxxx>
>
> Following process will corrupt ext4 image:
> Step 1:
> jbd2_journal_commit_transaction
> __jbd2_journal_insert_checkpoint(jh, commit_transaction)
> // Put jh into trans1->t_checkpoint_list
> journal->j_checkpoint_transactions = commit_transaction
> // Put trans1 into journal->j_checkpoint_transactions
>
> Step 2:
> do_get_write_access
> test_clear_buffer_dirty(bh) // clear buffer dirty，set jbd dirty
> __jbd2_journal_file_buffer(jh, transaction) // jh belongs to trans2
>
> Step 3:
> drop_cache
> journal_shrink_one_cp_list
> jbd2_journal_try_remove_checkpoint
> if (!trylock_buffer(bh)) // lock bh, true
> if (buffer_dirty(bh)) // buffer is not dirty
> __jbd2_journal_remove_checkpoint(jh)
> // remove jh from trans1->t_checkpoint_list
>
> Step 4:
> jbd2_log_do_checkpoint
> trans1 = journal->j_checkpoint_transactions
> // jh is not in trans1->t_checkpoint_list
> jbd2_cleanup_journal_tail(journal) // trans1 is done
>
> Step 5: Power cut, trans2 is not committed, jh is lost in next mounting.
>
> Fix it by checking 'jh->b_transaction' before remove it from checkpoint.
>
> Fixes: 46f881b5b175 ("jbd2: fix a race when checking checkpoint buffer busy")
> Signed-off-by: Zhihao Cheng <chengzhihao1@xxxxxxxxxx>
> Signed-off-by: Zhang Yi <yi.zhang@xxxxxxxxxx>
Indeed! I've missed this difference between __cp_buffer_busy() and
jbd2_journal_try_remove_checkpoint() during my review of 46f881b5b175. The
fix looks good. Feel free to add:
Reviewed-by: Jan Kara <jack@xxxxxxx>
Honza
> ---
> fs/jbd2/checkpoint.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/jbd2/checkpoint.c b/fs/jbd2/checkpoint.c
> index 936c6d758a65..f033ac807013 100644
> --- a/fs/jbd2/checkpoint.c
> +++ b/fs/jbd2/checkpoint.c
> @@ -639,6 +639,8 @@ int jbd2_journal_try_remove_checkpoint(struct journal_head *jh)
> {
> struct buffer_head *bh = jh2bh(jh);
>
> + if (jh->b_transaction)
> + return -EBUSY;
> if (!trylock_buffer(bh))
> return -EBUSY;
> if (buffer_dirty(bh)) {
> --
> 2.39.2
>
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
