On Tue, Apr 26, 2022 at 11:27:01AM -0700, Samuel Mendoza-Jonas wrote: > From: Ritesh Harjani <riteshh@xxxxxxxxxxxxx> > > commit cc16eecae687912238ee6efbff71ad31e2bc414e upstream. > > jbd2_journal_wait_updates() is called with j_state_lock held. But if > there is a commit in progress, then this transaction might get committed > and freed via jbd2_journal_commit_transaction() -> > jbd2_journal_free_transaction(), when we release j_state_lock. > So check for journal->j_running_transaction everytime we release and > acquire j_state_lock to avoid use-after-free issue. > > Link: https://lore.kernel.org/r/948c2fed518ae739db6a8f7f83f1d58b504f87d0.1644497105.git.ritesh.list@xxxxxxxxx > Fixes: 4f98186848707f53 ("jbd2: refactor wait logic for transaction updates into a common function") > Cc: stable@xxxxxxxxxx > Reported-and-tested-by: syzbot+afa2ca5171d93e44b348@xxxxxxxxxxxxxxxxxxxxxxxxx > Reviewed-by: Jan Kara <jack@xxxxxxx> > Signed-off-by: Ritesh Harjani <riteshh@xxxxxxxxxxxxx> > Signed-off-by: Theodore Ts'o <tytso@xxxxxxx> > [backport to 4.9-4.19 in original jbd2_journal_commit_transaction() > location before the refactor in > 4f9818684870 "jbd2: refactor wait logic for transaction updates into a > common function"] > Signed-off-by: Samuel Mendoza-Jonas <samjonas@xxxxxxxxxx> > Fixes: 1da177e4c3f41524 > Cc: stable@xxxxxxxxxx # 4.9.x - 4.19.x > --- > While marked for 5.17 stable, it looks like this fix also applies to the > original location in jbd2_journal_commit_transaction() before it was > refactored to use jbd2_journal_wait_updates(). This applies the same > change there. Jan kindly pointed out this was a false alarm: https://lore.kernel.org/all/20220427111726.3wdyxbqoxs7skdzf@xxxxxxxxxx/ So the existing patch is fine and these can be ignored! Cheers, Sam > > fs/jbd2/commit.c | 22 +++++++++++++--------- > 1 file changed, 13 insertions(+), 9 deletions(-) > > diff --git a/fs/jbd2/commit.c b/fs/jbd2/commit.c > index 97760cb9bcd7..66e776eb5ea7 100644 > --- a/fs/jbd2/commit.c > +++ b/fs/jbd2/commit.c > @@ -382,6 +382,7 @@ void jbd2_journal_commit_transaction(journal_t *journal) > int csum_size = 0; > LIST_HEAD(io_bufs); > LIST_HEAD(log_bufs); > + DEFINE_WAIT(wait); > > if (jbd2_journal_has_csum_v2or3(journal)) > csum_size = sizeof(struct jbd2_journal_block_tail); > @@ -434,22 +435,25 @@ void jbd2_journal_commit_transaction(journal_t *journal) > stats.run.rs_running = jbd2_time_diff(commit_transaction->t_start, > stats.run.rs_locked); > > - spin_lock(&commit_transaction->t_handle_lock); > - while (atomic_read(&commit_transaction->t_updates)) { > - DEFINE_WAIT(wait); > + while (1) { > + commit_transaction = journal->j_running_transaction; > + if (!commit_transaction) > + break; > > + spin_lock(&commit_transaction->t_handle_lock); > prepare_to_wait(&journal->j_wait_updates, &wait, > TASK_UNINTERRUPTIBLE); > - if (atomic_read(&commit_transaction->t_updates)) { > + if (!atomic_read(&commit_transaction->t_updates)) { > spin_unlock(&commit_transaction->t_handle_lock); > - write_unlock(&journal->j_state_lock); > - schedule(); > - write_lock(&journal->j_state_lock); > - spin_lock(&commit_transaction->t_handle_lock); > + finish_wait(&journal->j_wait_updates, &wait); > + break; > } > + spin_unlock(&commit_transaction->t_handle_lock); > + write_unlock(&journal->j_state_lock); > + schedule(); > finish_wait(&journal->j_wait_updates, &wait); > + write_lock(&journal->j_state_lock); > } > - spin_unlock(&commit_transaction->t_handle_lock); > > J_ASSERT (atomic_read(&commit_transaction->t_outstanding_credits) <= > journal->j_max_transaction_buffers); > -- > 2.25.1 >
