Re: [PATCH] xfs: reject log continuation for a header-only item in recovery

[Xiang Mei]

On Mon, Jul 13, 2026 at 3:57 PM Darrick J. Wong <djwong@xxxxxxxxxx> wrote:
>
> On Sun, Jul 12, 2026 at 03:08:54PM -0700, Xiang Mei wrote:
> > On an XLOG_WAS_CONT_TRANS op, xlog_recover_add_to_cont_trans() extends the
> > tail item's last region via item->ri_buf[item->ri_cnt-1] without checking
> > that the item has one. A header-only item, created by
> > xlog_recover_add_to_trans() when a transaction's first op is a bare
> > sizeof(struct xfs_trans_header) op carrying XFS_TRANS_HEADER_MAGIC, sits on
> > r_itemq with ri_cnt == 0 and ri_buf == NULL. A crafted log whose ops are
> > XLOG_START_TRANS, that header op, then XLOG_WAS_CONT_TRANS thus
> > dereferences ((struct kvec *)NULL)[-1] and faults during mount(2) log
> > recovery of an untrusted XFS image.
> >
> > A valid XLOG_WAS_CONT_TRANS only follows a region written with
> > XLOG_CONTINUE_TRANS, so ri_cnt >= 1 always holds for a real log. Reject the
> > ri_cnt == 0 case as corruption with -EFSCORRUPTED, which the caller already
> > propagates to abort recovery.
> >
> >   BUG: unable to handle page fault for address: fffffffffffffff0
> >   #PF: supervisor read access in kernel mode
> >   Oops: 0000 [#1] SMP KASAN NOPTI
> >   RIP: 0010:xlog_recover_add_to_cont_trans (fs/xfs/xfs_log_recover.c:2135)
> >   Call Trace:
> >    xlog_recovery_process_trans (fs/xfs/xfs_log_recover.c:2306)
> >    xlog_recover_process_data (fs/xfs/xfs_log_recover.c:2500)
> >    xlog_do_recovery_pass (fs/xfs/xfs_log_recover.c:3243)
> >    xlog_do_log_recovery (fs/xfs/xfs_log_recover.c:3331)
> >    xlog_do_recover (fs/xfs/xfs_log_recover.c:3368)
> >    xlog_recover (fs/xfs/xfs_log_recover.c:3493)
> >    xfs_log_mount (fs/xfs/xfs_log.c:617)
> >    xfs_mountfs (fs/xfs/xfs_mount.c:1031)
> >    xfs_fs_fill_super (fs/xfs/xfs_super.c:1940)
> >    get_tree_bdev_flags (fs/super.c:1634)
> >    vfs_get_tree (fs/super.c:1694)
> >    fc_mount (fs/namespace.c:1198)
> >    path_mount (fs/namespace.c:4161)
> >    __x64_sys_mount (fs/namespace.c:4367)
> >    do_syscall_64 (arch/x86/entry/syscall_64.c:94)
> >    entry_SYSCALL_64_after_hwframe (arch/x86/entry/entry_64.S:121)
> >   Kernel panic - not syncing: Fatal exception
> >
> > Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> > Reported-by: Weiming Shi <bestswngs@xxxxxxxxx>
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Xiang Mei <xmei5@xxxxxxx>
> > ---
> >  fs/xfs/xfs_log_recover.c | 3 +++
> >  1 file changed, 3 insertions(+)
> >
> > diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> > index 5f984bf5698a..fa8e04bf1aac 100644
> > --- a/fs/xfs/xfs_log_recover.c
> > +++ b/fs/xfs/xfs_log_recover.c
> > @@ -2131,6 +2131,9 @@ xlog_recover_add_to_cont_trans(
> >       item = list_entry(trans->r_itemq.prev, struct xlog_recover_item,
> >                         ri_list);
> >
> > +     if (item->ri_cnt == 0)
> > +             return -EFSCORRUPTED;
>
> Didn't Weiming Shi already post a similar patch?  Is this a duplicate,
> or a complement?
>
> --D
>

Thanks for the reminder. Weiming and I both pulled one crash from the
to-report dataset: The crashes are different, but they could share the
same root cause.

After investigation, I found it's a complement. Same root cause: a
header-only item left on r_itemq with ri_cnt == 0 and ri_buf == NULL.
Proved by crashing the kernel with:

https://lore.kernel.org/linux-xfs/178342425117.389943.13021791343607135682.b4-ty@b4/

Weiming's v3 3/3 guards xlog_recover_reorder_trans(), which only runs
from xlog_recover_commit_trans() on an XLOG_COMMIT_TRANS op. This
patch guards xlog_recover_add_to_cont_trans(), reached from the
separate XLOG_WAS_CONT_TRANS case in xlog_recovery_process_trans().
Please let me know if you want us to merge them into one series.

Xiang
> > +
> >       old_ptr = item->ri_buf[item->ri_cnt-1].iov_base;
> >       old_len = item->ri_buf[item->ri_cnt-1].iov_len;
> >
> > --
> > 2.43.0
> >
> >