Re: [PATCH] xfs: reject a continuation op with no buffered region in log

[Dave Chinner <dgc@kernel.org>]

On Tue, Nov 10, 2026 at 08:00:00PM +0800, Guikun Yang wrote:
> xlog_recover_add_to_cont_trans() takes the tail item of the transaction
> and indexes ri_buf[ri_cnt - 1] without checking that the item has any
> region buffered:
> 
> 	item = list_entry(trans->r_itemq.prev, struct xlog_recover_item,
> 			  ri_list);
> 	old_ptr = item->ri_buf[item->ri_cnt-1].iov_base;
> 
> An item created by xlog_recover_add_item() has ri_cnt == 0 and
> ri_buf == NULL - the transaction header itself is copied into
> trans->r_theader, not into the item - so this evaluates to NULL[-1].
> xlog_recovery_process_trans() dispatches to this function for any op
> whose header carries XLOG_WAS_CONT_TRANS, and nothing on the way
> checks that flag sequence against the item state; the only related
> assertion in the recovery code is XFS_DEBUG-only.
> 
> A crafted dirty log with a valid transaction header followed by an op
> header with XLOG_WAS_CONT_TRANS therefore faults at mount time on a
> stock kernel, without any sanitizer enabled:
> 
>   BUG: unable to handle page fault for address: fffffffffffffff0
>   #PF: supervisor read access in kernel mode
>   Oops: 0000 [#1] SMP
>   RIP: xlog_recover_add_to_cont_trans+0xf9/0x460
>    xlog_recovery_process_trans
>    xlog_recover_process_ophdr
>    xlog_recover_process_data
>    xlog_recover_process
>    xlog_do_recovery_pass
> 
> Note that reaching it does not require an actual split region: a single
> continuation op right after the transaction header is enough, because
> the dispatcher never checks that the previous region had a buffer
> attached.
> 
> Treat it as log corruption and fail recovery, matching the other
> -EFSCORRUPTED paths in this file.
> 
> Reported-by: Guikun Yang <yangguikun19@qq.com>
> Signed-off-by: Guikun Yang <yangguikun19@qq.com>

NACK, again, for the same reasons.

-- 
Dave Chinner
dgc@kernel.org