Commit cc16eeca authored by Ritesh Harjani's avatar Ritesh Harjani Committed by Theodore Ts'o
Browse files

jbd2: fix use-after-free of transaction_t race 

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@gmail.com


Fixes: 4f981868 ("jbd2: refactor wait logic for transaction updates into a common function")
Cc: stable@kernel.org
Reported-and-tested-by: default avatar <syzbot+afa2ca5171d93e44b348@syzkaller.appspotmail.com>
Reviewed-by: default avatarJan Kara <jack@suse.cz>
Signed-off-by: default avatarRitesh Harjani <riteshh@linux.ibm.com>
Signed-off-by: default avatarTheodore Ts'o <tytso@mit.edu>
parent e3952fcc

fs/jbd2/transaction.c

+25 −16
Original line number Diff line number Diff line
@@ -842,27 +842,38 @@ EXPORT_SYMBOL(jbd2_journal_restart); 
 */

void jbd2_journal_wait_updates(journal_t *journal)

{

	transaction_t *commit_transaction = journal->j_running_transaction;

	DEFINE_WAIT(wait);



	if (!commit_transaction)

		return;

	while (1) {

		/*

		 * Note that the running transaction can get freed under us if

		 * this transaction is getting committed in

		 * jbd2_journal_commit_transaction() ->

		 * jbd2_journal_free_transaction(). This can only happen when we

		 * release j_state_lock -> schedule() -> acquire j_state_lock.

		 * Hence we should everytime retrieve new j_running_transaction

		 * value (after j_state_lock release acquire cycle), else it may

		 * lead to use-after-free of old freed transaction.

		 */

		transaction_t *transaction = journal->j_running_transaction;



	spin_lock(&commit_transaction->t_handle_lock);

	while (atomic_read(&commit_transaction->t_updates)) {

		DEFINE_WAIT(wait);

		if (!transaction)

			break;



		spin_lock(&transaction->t_handle_lock);

		prepare_to_wait(&journal->j_wait_updates, &wait,

				TASK_UNINTERRUPTIBLE);

		if (atomic_read(&commit_transaction->t_updates)) {

			spin_unlock(&commit_transaction->t_handle_lock);

		if (!atomic_read(&transaction->t_updates)) {

			spin_unlock(&transaction->t_handle_lock);

			finish_wait(&journal->j_wait_updates, &wait);

			break;

		}

		spin_unlock(&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);

		write_lock(&journal->j_state_lock);

	}

	spin_unlock(&commit_transaction->t_handle_lock);

}



/**
@@ -877,8 +888,6 @@ void jbd2_journal_wait_updates(journal_t *journal) 
 */

void jbd2_journal_lock_updates(journal_t *journal)

{

	DEFINE_WAIT(wait);



	jbd2_might_wait_for_commit(journal);



	write_lock(&journal->j_state_lock);