ACK: [X/B][SRU][CVE-2018-20976][PATCH] xfs: clear sb->s_fs_info on mount failure

[Kleber Souza]

On 8/29/19 4:33 PM, Connor Kuehl wrote:
> From: Dave Chinner <dchinner at redhat.com>
> 
> CVE-2018-20976
> 
> We recently had an oops reported on a 4.14 kernel in
> xfs_reclaim_inodes_count() where sb->s_fs_info pointed to garbage
> and so the m_perag_tree lookup walked into lala land.
> 
> Essentially, the machine was under memory pressure when the mount
> was being run, xfs_fs_fill_super() failed after allocating the
> xfs_mount and attaching it to sb->s_fs_info. It then cleaned up and
> freed the xfs_mount, but the sb->s_fs_info field still pointed to
> the freed memory. Hence when the superblock shrinker then ran
> it fell off the bad pointer.
> 
> With the superblock shrinker problem fixed at teh VFS level, this
> stale s_fs_info pointer is still a problem - we use it
> unconditionally in ->put_super when the superblock is being torn
> down, and hence we can still trip over it after a ->fill_super
> call failure. Hence we need to clear s_fs_info if
> xfs-fs_fill_super() fails, and we need to check if it's valid in
> the places it can potentially be dereferenced after a ->fill_super
> failure.
> 
> Signed-Off-By: Dave Chinner <dchinner at redhat.com>
> Reviewed-by: Darrick J. Wong <darrick.wong at oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong at oracle.com>
> (cherry picked from commit c9fbd7bbc23dbdd73364be4d045e5d3612cf6e82)
> Signed-off-by: Connor Kuehl <connor.kuehl at canonical.com>

Acked-by: Kleber Sacilotto de Souza <kleber.souza at canonical.com>

> ---
>  fs/xfs/xfs_super.c | 10 ++++++++++
>  1 file changed, 10 insertions(+)
> 
> diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c
> index ef64a1e1a66a..ff3f5812c0fd 100644
> --- a/fs/xfs/xfs_super.c
> +++ b/fs/xfs/xfs_super.c
> @@ -1572,6 +1572,7 @@ xfs_fs_fill_super(
>   out_close_devices:
>  	xfs_close_devices(mp);
>   out_free_fsname:
> +	sb->s_fs_info = NULL;
>  	xfs_free_fsname(mp);
>  	kfree(mp);
>   out:
> @@ -1589,6 +1590,10 @@ xfs_fs_put_super(
>  {
>  	struct xfs_mount	*mp = XFS_M(sb);
>  
> +	/* if ->fill_super failed, we have no mount to tear down */
> +	if (!sb->s_fs_info)
> +		return;
> +
>  	xfs_notice(mp, "Unmounting Filesystem");
>  	xfs_filestream_unmount(mp);
>  	xfs_unmountfs(mp);
> @@ -1598,6 +1603,8 @@ xfs_fs_put_super(
>  	xfs_destroy_percpu_counters(mp);
>  	xfs_destroy_mount_workqueues(mp);
>  	xfs_close_devices(mp);
> +
> +	sb->s_fs_info = NULL;
>  	xfs_free_fsname(mp);
>  	kfree(mp);
>  }
> @@ -1617,6 +1624,9 @@ xfs_fs_nr_cached_objects(
>  	struct super_block	*sb,
>  	struct shrink_control	*sc)
>  {
> +	/* Paranoia: catch incorrect calls during mount setup or teardown */
> +	if (WARN_ON_ONCE(!sb->s_fs_info))
> +		return 0;
>  	return xfs_reclaim_inodes_count(XFS_M(sb));
>  }
>  
>