Re: [PATCH 1/2] xfs: validate inode di_forkoff

[Brian Foster <bfoster@redhat.com>]

On Mon, Sep 10, 2018 at 05:22:08PM -0500, Eric Sandeen wrote:
> Verify the inode di_forkoff, lifted from xfs_repair's
> process_check_inode_forkoff().
> 
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> ---
> 
> diff --git a/fs/xfs/libxfs/xfs_inode_buf.c b/fs/xfs/libxfs/xfs_inode_buf.c
> index 30d1d60f1d46..8d76637a49a7 100644
> --- a/fs/xfs/libxfs/xfs_inode_buf.c
> +++ b/fs/xfs/libxfs/xfs_inode_buf.c
> @@ -415,6 +415,31 @@ xfs_dinode_verify_fork(
>  	return NULL;
>  }
>  
> +static xfs_failaddr_t
> +xfs_dinode_verify_forkoff(
> +	struct xfs_dinode	*dip,
> +	struct xfs_mount	*mp)
> +{
> +	if (dip->di_forkoff == 0)
> +		return NULL;

I think it would be good to use XFS_DFORK_Q() here, just to be
consistent with the other, similar checks. Otherwise looks good:

Reviewed-by: Brian Foster <bfoster@redhat.com>

> +
> +	switch (dip->di_format)  {
> +	case XFS_DINODE_FMT_DEV:
> +		if (dip->di_forkoff != (roundup(sizeof(xfs_dev_t), 8) >> 3))
> +			return __this_address;
> +		break;
> +	case XFS_DINODE_FMT_LOCAL:	/* fall through ... */
> +	case XFS_DINODE_FMT_EXTENTS:    /* fall through ... */
> +	case XFS_DINODE_FMT_BTREE:
> +		if (dip->di_forkoff >= (XFS_LITINO(mp, dip->di_version) >> 3))
> +			return __this_address;
> +		break;
> +	default:
> +		return __this_address;
> +	}
> +	return NULL;
> +}
> +
>  xfs_failaddr_t
>  xfs_dinode_verify(
>  	struct xfs_mount	*mp,
> @@ -470,6 +498,11 @@ xfs_dinode_verify(
>  	if (mode && (flags & XFS_DIFLAG_REALTIME) && !mp->m_rtdev_targp)
>  		return __this_address;
>  
> +	/* check for illegal values of di_forkoff */
> +	fa = xfs_dinode_verify_forkoff(dip, mp);
> +	if (fa)
> +		return fa;
> +
>  	/* Do we have appropriate data fork formats for the mode? */
>  	switch (mode & S_IFMT) {
>  	case S_IFIFO:
>