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

[liubaolin <liubaolin12138@163.com>]

> 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@163.com> wrote:
>>From: Baolin Liu <liubaolin@kylinos.cn>
>>
>>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@kylinos.cn>
>>---
>> 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