From: Ritesh Harjani riteshh@linux.ibm.com
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.164449710... Fixes: 4f98186848707f53 ("jbd2: refactor wait logic for transaction updates into a common function") Cc: stable@kernel.org Reported-and-tested-by: syzbot+afa2ca5171d93e44b348@syzkaller.appspotmail.com Reviewed-by: Jan Kara jack@suse.cz Signed-off-by: Ritesh Harjani riteshh@linux.ibm.com Signed-off-by: Theodore Ts'o tytso@mit.edu [backport to 5.4-5.16 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@amazon.com Fixes: 1da177e4c3f41524 Cc: stable@kernel.org # 5.4.x - 5.16.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.
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 d188fa913a07..e9ed8a71ff25 100644 --- a/fs/jbd2/commit.c +++ b/fs/jbd2/commit.c @@ -408,6 +408,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); @@ -484,22 +485,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); commit_transaction->t_state = T_SWITCH; write_unlock(&journal->j_state_lock);