Re: [PATCH v4 2/2] jbd2: gracefully abort on transaction state corruptions

[Andreas Dilger]

On Mar 3, 2026, at 11:01, Milos Nikic <nikic.milos@xxxxxxxxx> wrote:
> 
> Auditing the jbd2 codebase reveals several legacy J_ASSERT calls
> that enforce internal state machine invariants (e.g., verifying
> jh->b_transaction or jh->b_next_transaction pointers).
> 
> When these invariants are broken, the journal is in a corrupted
> state. However, triggering a fatal panic brings down the entire
> system for a localized filesystem error.
> 
> This patch targets a specific class of these asserts: those
> residing inside functions that natively return integer error codes,
> booleans, or error pointers. It replaces the hard J_ASSERTs with
> WARN_ON_ONCE to capture the offending stack trace, safely drops
> any held locks, gracefully aborts the journal, and returns -EINVAL.
> 
> This prevents a catastrophic kernel panic while ensuring the
> corrupted journal state is safely contained and upstream callers
> (like ext4 or ocfs2) can gracefully handle the aborted handle.
> 
> Functions modified in fs/jbd2/transaction.c:
> - jbd2__journal_start()
> - do_get_write_access()
> - jbd2_journal_dirty_metadata()
> - jbd2_journal_forget()
> - jbd2_journal_try_to_free_buffers()
> - jbd2_journal_file_inode()
> 
> Signed-off-by: Milos Nikic <nikic.milos@xxxxxxxxx>

Looks good, with one minor suggestion.  Either way you could add:

Reviewed-by: Andreas Dilger <adilger@xxxxxxxxx <mailto:adilger@xxxxxxxxx>>

> @@ -1531,13 +1558,15 @@ int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
>  			spin_lock(&jh->b_state_lock);
>  			if (jh->b_transaction == transaction &&
>  			    jh->b_jlist != BJ_Metadata)
> -				pr_err("JBD2: assertion failure: h_type=%u "
> - 				       "h_line_no=%u block_no=%llu jlist=%u\n",
> +				pr_err("JBD2: assertion failure: h_type=%u h_line_no=%u block_no=%llu jlist=%u\n",
>  				       handle->h_type, handle->h_line_no,
>  				       (unsigned long long) bh->b_blocknr,
>  				       jh->b_jlist);
> -			J_ASSERT_JH(jh, jh->b_transaction != transaction ||
> -				    jh->b_jlist == BJ_Metadata);
> +			if (WARN_ON_ONCE(jh->b_transaction == transaction &&
> +					 jh->b_jlist != BJ_Metadata)) {
> +				ret = -EINVAL;
> +				goto out_unlock_bh;
> +			}
>  			spin_unlock(&jh->b_state_lock);
>  		}

It doesn't make sense to check these conditions twice.  That was needed with
the J_ASSERT_JH() calls, but it is now possible to put the pr_err() calls
inside "if (WARN_ON_ONCE(...))" as it is done in other parts of the patch.

Cheers, Andreas