Re: [PATCH] xfs: fix inode number overflow in ifree cluster helper

["Darrick J. Wong" <darrick.wong@oracle.com>]

On Thu, Apr 02, 2020 at 06:57:18AM -0400, Brian Foster wrote:
> Qian Cai reports seemingly random buffer read verifier errors during
> filesystem writeback. This was isolated to a recent patch that
> factored out some inode cluster freeing code and happened to cast an
> unsigned inode number type to a signed value. If the inode number
> value overflows, we can skip marking in-core inodes associated with
> the underlying buffer stale at the time the physical inodes are
> freed. If such an inode happens to be dirty, xfsaild will eventually
> attempt to write it back over non-inode blocks. The invalidation of
> the underlying inode buffer causes writeback to read the buffer from
> disk. This fails the read verifier (preventing eventual corruption)
> if the buffer no longer looks like an inode cluster. Analysis by
> Dave Chinner.
> 
> Fix up the helper to use the proper type for inode number values.
> 
> Fixes: 5806165a6663 ("xfs: factor inode lookup from xfs_ifree_cluster")
> Reported-by: Qian Cai <cai@lca.pw>
> Signed-off-by: Brian Foster <bfoster@redhat.com>
> ---
> 
> Fixes the problem described here[1]. I wasn't sure if we planned on
> fixing the original patch in for-next or wanted a separate patch. Feel
> free to commit standalone or fold into the original...
> 
> Brian
> 
> [1] https://lore.kernel.org/linux-xfs/990EDC4E-1A4E-4AC3-84D9-078ACF5EB9CC@lca.pw/

Looks ok, I'll probably just attach it to the end rather than rebase
for-next at this super late point...

Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>

--D

> 
>  fs/xfs/xfs_inode.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/xfs/xfs_inode.c b/fs/xfs/xfs_inode.c
> index 0cac0d37e3ae..ae86c870da92 100644
> --- a/fs/xfs/xfs_inode.c
> +++ b/fs/xfs/xfs_inode.c
> @@ -2511,7 +2511,7 @@ static struct xfs_inode *
>  xfs_ifree_get_one_inode(
>  	struct xfs_perag	*pag,
>  	struct xfs_inode	*free_ip,
> -	int			inum)
> +	xfs_ino_t		inum)
>  {
>  	struct xfs_mount	*mp = pag->pag_mount;
>  	struct xfs_inode	*ip;
> -- 
> 2.21.1
>