Re: [PATCH v2 3/3] xfs: fail recovery on a committed log item with no regions

["Darrick J. Wong" <djwong@kernel.org>]

On Wed, Jul 01, 2026 at 08:38:33AM -0700, Weiming Shi wrote:
> If the first op of a transaction is a bare transaction header
> (len == sizeof(struct xfs_trans_header)), xlog_recover_add_to_trans()
> adds a recovery item but no region, leaving it on r_itemq with
> ri_cnt == 0 and ri_buf == NULL.
> 
> If a commit op follows, xlog_recover_reorder_trans() calls ITEM_TYPE()
> on the item, which reads *(unsigned short *)item->ri_buf[0].iov_base and
> faults on the NULL ri_buf.

How do we escape xlog_recover_add_to_trans with item->ri_cnt==0 when
there's a bare transaction header?  Is this the "!len" case at the top
of xlog_recover_add_to_trans?

Why don't we abort log recovery right then and there?  Are we hoping
that a subsequent "continue" section will supply some items, and that's
why we can only check ri_cnt/ri_buf if the ondisk transaction is
committed?

> The log is CRC-checked, so this op sequence comes from a crafted image,
> not media corruption.  It faults the kernel when such an image is
> mounted:
> 
>  KASAN: null-ptr-deref in range [0x0000000000000000-0x0000000000000007]
>  RIP: 0010:xlog_recover_reorder_trans (fs/xfs/xfs_log_recover.c:1836)
>   xlog_recover_commit_trans (fs/xfs/xfs_log_recover.c:2043)
>   xlog_recover_process_data (fs/xfs/xfs_log_recover.c:2501)
>   xlog_do_recovery_pass (fs/xfs/xfs_log_recover.c:3244)
>   xlog_recover (fs/xfs/xfs_log_recover.c:3493)
>   xfs_log_mount (fs/xfs/xfs_log.c:618)
>   xfs_mountfs (fs/xfs/xfs_mount.c:1034)
>   xfs_fs_fill_super (fs/xfs/xfs_super.c:1938)
>   vfs_get_tree (fs/super.c:1695)
>   path_mount (fs/namespace.c:4161)
>   __x64_sys_mount (fs/namespace.c:4367)
> 
> A committed item always carries its format descriptor in ri_buf[0], so
> one with no regions is invalid.  Reject it with -EFSCORRUPTED, like the
> unrecognized item type below.
> 
> Fixes: 89cebc847729 ("xfs: validate transaction header length on log recovery")
> Reported-by: Xiang Mei <xmei5@asu.edu>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <bestswngs@gmail.com>

Cc: <stable@vger.kernel.org> # v4.3

I agree that a totally empty log item is a sign of corruption, or at
least something going seriously wrong.  Can the runtime log code ever be
tricked into emitting a bare transaction header?

--D

> ---
>  fs/xfs/xfs_log_recover.c | 9 +++++++++
>  1 file changed, 9 insertions(+)
> 
> diff --git a/fs/xfs/xfs_log_recover.c b/fs/xfs/xfs_log_recover.c
> index 5347f6a5ec42..461e847c32a2 100644
> --- a/fs/xfs/xfs_log_recover.c
> +++ b/fs/xfs/xfs_log_recover.c
> @@ -1907,6 +1907,15 @@ xlog_recover_reorder_trans(
>  	list_for_each_entry_safe(item, n, &sort_list, ri_list) {
>  		enum xlog_recover_reorder	fate = XLOG_REORDER_ITEM_LIST;
>  
> +		/* a committed item with no regions has a NULL ri_buf[0] */
> +		if (!item->ri_cnt || !item->ri_buf) {
> +			xfs_warn(log->l_mp,
> +				"%s: committed log item has no regions",
> +				__func__);
> +			error = -EFSCORRUPTED;
> +			break;
> +		}
> +
>  		item->ri_ops = xlog_find_item_ops(item);
>  		if (!item->ri_ops) {
>  			xfs_warn(log->l_mp,
> -- 
> 2.43.0
> 
>