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@syzkaller.appspotmail.com Closes: https://syzkaller.appspot.com/bug?extid=98f651460e558a21baae Tested-by: syzbot+98f651460e558a21baae@syzkaller.appspotmail.com Signed-off-by: Deepanshu Kartikey --- 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