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

Xiang Mei <[email protected]>
Newsgroups org.kernel.vger.linux-xfs,org.kernel.vger.linux-kernel
Message-ID <CAPpSM+RtwKTJTwm-pRLqs5+E6DqaPgJobGdxo649NNLmsYFZ4Q@mail.gmail.com>
On Mon, Jul 13, 2026 at 3:57 PM Darrick J. Wong <[email protected]> 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 <[email protected]>
> > Assisted-by: Claude:claude-opus-4-8
> > Signed-off-by: Xiang Mei <[email protected]>
> > ---
> >  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
> >
> >
lmpx.com only provides a reader for public news (NNTP) servers. It is not affiliated with the servers or forums shown here and is not responsible for the content of articles, which is written by their respective authors.