Re: [PATCH] xfs: bound inode fork length against the fork size during log recovery

[Darrick J. Wong]

On Tue, Jul 07, 2026 at 10:58:43AM -0300, Aldo Ariel Panzardo wrote:
> xlog_recover_inode_commit_pass2() copies an inode log item's data/attr
> fork region into the inode buffer using the on-log region length without
> bounding it against the fork capacity, e.g.:
> 
> 	len = item->ri_buf[2].iov_len;
> 	memcpy(XFS_DFORK_DPTR(dip), src, len);
> 
> The only guard is an ASSERT, which is a no-op on production kernels
> (CONFIG_XFS_DEBUG off), and xfs_dinode_verify() runs only after the copy.
> A crafted image with a dirty log can therefore drive a heap out-of-bounds
> write at mount time. The XFS_ILOG_DBROOT sibling already passes
> XFS_DFORK_DSIZE as a bound; the DDATA/DEXT and ADATA/AEXT memcpy paths
> did not.
> 
> Reject the log item with -EFSCORRUPTED when the region length exceeds the
> destination fork size.
> 
> Fixes: 658fa68b6f34 ("xfs: refactor log recovery inode item dispatch for pass2 commit functions")
> Signed-off-by: Aldo Ariel Panzardo <qwe.aldo@xxxxxxxxx>

This is a bug fix, it should engage the LTS backporting process:
Cc: <stable@xxxxxxxxxxxxxxx> # v5.8

> ---
>  fs/xfs/xfs_inode_item_recover.c | 9 ++++++++-
>  1 file changed, 8 insertions(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_inode_item_recover.c b/fs/xfs/xfs_inode_item_recover.c
> index 169a8fe3bf0a..388d3a8dbd31 100644
> --- a/fs/xfs/xfs_inode_item_recover.c
> +++ b/fs/xfs/xfs_inode_item_recover.c
> @@ -510,6 +510,10 @@ xlog_recover_inode_commit_pass2(
>  	switch (fields & XFS_ILOG_DFORK) {
>  	case XFS_ILOG_DDATA:
>  	case XFS_ILOG_DEXT:
> +		if (len > XFS_DFORK_DSIZE(dip, mp)) {
> +			error = -EFSCORRUPTED;
> +			goto out_release;
> +		}

But in any case this is new validation code, which means that it should
go at the top the function before we memcpy anything into the ondisk
inode.

--D

>  		memcpy(XFS_DFORK_DPTR(dip), src, len);
>  		break;
>  
> @@ -545,8 +549,11 @@ xlog_recover_inode_commit_pass2(
>  		switch (in_f->ilf_fields & XFS_ILOG_AFORK) {
>  		case XFS_ILOG_ADATA:
>  		case XFS_ILOG_AEXT:
> +			if (len > XFS_DFORK_ASIZE(dip, mp)) {
> +				error = -EFSCORRUPTED;
> +				goto out_release;
> +			}
>  			dest = XFS_DFORK_APTR(dip);
> -			ASSERT(len <= XFS_DFORK_ASIZE(dip, mp));
>  			memcpy(dest, src, len);
>  			break;
>  
> -- 
> 2.43.0
> 
>