Re: [PATCH] jbd2: check for aborted handle in jbd2_journal_dirty_metadata()

[Andreas Dilger]

On May 6, 2026, at 23:06, Deepanshu Kartikey <kartikey406@xxxxxxxxx> wrote:
> 
> jbd2_journal_dirty_metadata() unconditionally dereferences
> handle->h_transaction at function entry to obtain the journal pointer:
> 
> transaction_t *transaction = handle->h_transaction;
> journal_t *journal = transaction->t_journal;
> 
> However, h_transaction may legitimately be NULL for an aborted handle.
> The is_handle_aborted() helper in include/linux/jbd2.h explicitly
> treats !h_transaction as one of the aborted states:
> 
> if (handle->h_aborted || !handle->h_transaction)
> return 1;
> 
> Every other entry point in fs/jbd2/transaction.c
> (jbd2_journal_get_{write,undo,create}_access, jbd2_journal_extend,
> jbd2_journal_restart, jbd2_journal_stop, etc.) guards against this
> with an is_handle_aborted() check before any dereference of
> h_transaction. jbd2_journal_dirty_metadata() was missing this guard.
> 
> This is reachable from ocfs2's xattr code. ocfs2_xa_set() intentionally
> falls through to ocfs2_xa_journal_dirty() even after
> ocfs2_xa_prepare_entry() fails, on the assumption that the buffer
> needs to be journaled to record any partial modifications (see the
> comment above the out_dirty label in fs/ocfs2/xattr.c). If the failure
> was caused by the journal being aborted -- e.g. an underlying I/O
> error during a sub-operation such as __ocfs2_remove_xattr_range() --
> the handle's h_transaction has been cleared by the abort path, and
> the unconditional deref in jbd2_journal_dirty_metadata() becomes a
> NULL deref.
> 
> Reproduced by syzbot with a crafted ocfs2 image where I/O against the
> loop device backing the mount is sabotaged via LOOP_SET_STATUS64
> between two setxattr() calls, causing the second setxattr (which
> truncates an external xattr value) to abort the journal mid-flight:
> 
>  Oops: general protection fault, probably for non-canonical
>        address 0xdffffc0000000000
>  KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
>  RIP: jbd2_journal_dirty_metadata+0x4a/0xd30 fs/jbd2/transaction.c:1520
>  Call Trace:
>   ocfs2_journal_dirty+0x130/0x700 fs/ocfs2/journal.c:831
>   ocfs2_xa_journal_dirty fs/ocfs2/xattr.c:1483 [inline]
>   ocfs2_xa_set+0x15e3/0x2ec0 fs/ocfs2/xattr.c:2294
>   ocfs2_xattr_block_set+0x3e0/0x33c0 fs/ocfs2/xattr.c:3016
>   __ocfs2_xattr_set_handle+0x6b3/0xf50 fs/ocfs2/xattr.c:3418
>   ocfs2_xattr_set+0xf3f/0x13e0 fs/ocfs2/xattr.c:3681
>   __vfs_setxattr+0x43c/0x480 fs/xattr.c:218
>   ...
> 
> Fix by adding the standard is_handle_aborted() guard at the top of
> jbd2_journal_dirty_metadata() and returning -EROFS, matching the
> pattern used by every other entry point in this file.
> ocfs2_journal_dirty() already handles a non-zero return from
> jbd2_journal_dirty_metadata() correctly.
> 
> Reported-by: syzbot+98f651460e558a21baae@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=98f651460e558a21baae
> Tested-by: syzbot+98f651460e558a21baae@xxxxxxxxxxxxxxxxxxxxxxxxx
> Signed-off-by: Deepanshu Kartikey <kartikey406@xxxxxxxxx>

LGTM.

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

> ---
> fs/jbd2/transaction.c | 9 +++++++--
> 1 file changed, 7 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/jbd2/transaction.c b/fs/jbd2/transaction.c
> index 4885903bbd10..aa0be9e9c876 100644
> --- a/fs/jbd2/transaction.c
> +++ b/fs/jbd2/transaction.c
> @@ -1516,14 +1516,19 @@ void jbd2_buffer_abort_trigger(struct journal_head *jh,
>  */
> int jbd2_journal_dirty_metadata(handle_t *handle, struct buffer_head *bh)
> {
> - transaction_t *transaction = handle->h_transaction;
> - journal_t *journal = transaction->t_journal;
> + transaction_t *transaction;
> + journal_t *journal;
> struct journal_head *jh;
> int ret = 0;
> 
> + if (is_handle_aborted(handle))
> + return -EROFS;
> if (!buffer_jbd(bh))
> return -EUCLEAN;
> 
> + transaction = handle->h_transaction;
> + journal = transaction->t_journal;
> +
> /*
> * We don't grab jh reference here since the buffer must be part
> * of the running transaction.
> -- 
> 2.43.0
> 
> 


Cheers, Andreas