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 > > > >