Re: [PATCH v1] jbd2: check transaction state before stopping handle

[liubaolin]

> Dear maintainers and community,
>
> Our customer reported a kernel crash issue. The kernel crashes in stop_this_handle() with:
> J_ASSERT(atomic_read(&transaction->t_updates) > 0);
>
> Crash stack:
> Call trace:
> stop_this_handle+0x148/0x158
> jbd2_journal_stop+0x198/0x388
> __ext4_journal_stop+0x70/0xf0
> ext4_create+0x12c/0x188
> ...
>
> From the vmcore dump, I found that handle->h_transaction->t_updates is 0 and handle->h_transaction->t_state is T_FINISHED. 
> This means the handle is still bound to a transaction that has already completed.
>
> In the JBD2 commit process, once a transaction enters T_LOCKED state, the commit thread waits for all associated handles to complete (t_updates becomes 0). 
> After T_LOCKED completes, the transaction transitions to T_FLUSH, T_COMMIT, and eventually T_FINISHED. 
> At this point, t_updates is legitimately 0, and there should be no active handles bound to this transaction.
>
> This appears to be a low-probability race condition that occurs under high concurrency scenarios. 
> The crash happened during ext4_create(), and at some rare timing point during the execution, a handle ended up bound to a transaction that had already completed. 
> The symptom is clear: a handle is bound to a T_FINISHED transaction with t_updates == 0.
>
> To prevent this, I added a transaction state check in both jbd2_journal_stop() and jbd2__journal_restart() before calling stop_this_handle().
> If the transaction is not in T_RUNNING or T_LOCKED state (i.e., it's in T_FLUSH or later states), 
> I clear handle->h_transaction, clear current->journal_info if needed, restore the memalloc_nofs context, and skip calling stop_this_handle(). 
> This is a defensive check to handle the edge case where a handle is bound to a transaction in an invalid state.
>
> Please let me know if you have any questions or concerns.
>
> Best regards,
> Baolin Liu



在 2026/3/5 20:57, Baolin Liu 写道:
> Add others
>
>
>
>
>
> At 2026-03-05 20:54:02, "Baolin Liu" <liubaolin12138@xxxxxxx> wrote:
> > From: Baolin Liu <liubaolin@xxxxxxxxxx>
> >
> > When a transaction enters T_FLUSH or later states,
> > handle->h_transaction may still point to it.
> > If jbd2_journal_stop() or jbd2__journal_restart() is called,
> > stop_this_handle() checks t_updates > 0, but t_updates is
> > already 0 for these states, causing a kernel BUG.
> >
> > Fix by checking transaction->t_state in jbd2_journal_stop()
> > and jbd2__journal_restart() before calling stop_this_handle().
> > If the transaction is not in T_RUNNING or T_LOCKED state,
> > clear handle->h_transaction and skip stop_this_handle().
> >
> > Crash stack:
> >  Call trace:
> >  stop_this_handle+0x148/0x158
> >  jbd2_journal_stop+0x198/0x388
> >  __ext4_journal_stop+0x70/0xf0
> >  ext4_create+0x12c/0x188
> >  lookup_open+0x214/0x6d8
> >  do_last+0x364/0x878
> >  path_openat+0x6c/0x280
> >  do_filp_open+0x70/0xe8
> >  do_sys_open+0x178/0x200
> >  sys_openat+0x3c/0x50
> >  el0_svc_naked+0x44/0x48
> >
> > Signed-off-by: Baolin Liu <liubaolin@xxxxxxxxxx>
> > ---
> > fs/jbd2/transaction.c | 25 +++++++++++++++++++++++--
> > 1 file changed, 23 insertions(+), 2 deletions(-)
> >
> > diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> > index dca4b5d8aaaa..3779382dbb80 100644
> > --- a/fs/jbd2/transaction.c
> > +++ b/fs/jbd2/transaction.c
> > @@ -772,14 +772,25 @@ int jbd2__journal_restart(handle_t *handle, int nblocks, int revoke_records,
> > 	journal = transaction->t_journal;
> > 	tid = transaction->t_tid;
> >
> > +	jbd2_debug(2, "restarting handle %p\n", handle);
> > +
> > +	/* Check if transaction is in invalid state */
> > +	if (transaction->t_state != T_RUNNING &&
> > +		transaction->t_state != T_LOCKED) {
> > +		if (current->journal_info == handle)
> > +			current->journal_info = NULL;
> > +		handle->h_transaction = NULL;
> > +		memalloc_nofs_restore(handle->saved_alloc_context);
> > +		goto skip_stop;
> > +	}
> > +
> > 	/*
> > 	 * First unlink the handle from its current transaction, and start the
> > 	 * commit on that.
> > 	 */
> > -	jbd2_debug(2, "restarting handle %p\n", handle);
> > 	stop_this_handle(handle);
> > 	handle->h_transaction = NULL;
> > -
> > +skip_stop:
> > 	/*
> > 	 * TODO: If we use READ_ONCE / WRITE_ONCE for j_commit_request we can
> >  	 * get rid of pointless j_state_lock traffic like this.
> > @@ -1856,6 +1867,16 @@ int jbd2_journal_stop(handle_t *handle)
> > 		memalloc_nofs_restore(handle->saved_alloc_context);
> > 		goto free_and_exit;
> > 	}
> > +	/* Check if transaction is in invalid state */
> > +	if (transaction->t_state != T_RUNNING &&
> > +		transaction->t_state != T_LOCKED) {
> > +		if (current->journal_info == handle)
> > +			current->journal_info = NULL;
> > +		handle->h_transaction = NULL;
> > +		memalloc_nofs_restore(handle->saved_alloc_context);
> > +		goto free_and_exit;
> > +	}
> > +
> > 	journal = transaction->t_journal;
> > 	tid = transaction->t_tid;
> >
> > --
> > 2.39.2