[SRU,X/A/B,1/1] xfs: set format back to extents if xfs_bmap_extents_to_btree

Message ID 20180706053329.23440-2-khalid.elmously@canonical.com
State New
Headers show
Series
  • [SRU,X/A/B,1/1] xfs: set format back to extents if xfs_bmap_extents_to_btree
Related show

Commit Message

Khaled Elmously July 6, 2018, 5:33 a.m.
From: Eric Sandeen <sandeen@redhat.com>

CVE-2018-10323

If xfs_bmap_extents_to_btree fails in a mode where we call
xfs_iroot_realloc(-1) to de-allocate the root, set the
format back to extents.

Otherwise we can assume we can dereference ifp->if_broot
based on the XFS_DINODE_FMT_BTREE format, and crash.

Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=199423
Signed-off-by: Eric Sandeen <sandeen@redhat.com>
Reviewed-by: Christoph Hellwig <hch@lst.de>
Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
(cherry-picked from 2c4306f719b083d17df2963bc761777576b8ad1b)
Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>
---
 fs/xfs/libxfs/xfs_bmap.c | 4 ++++
 1 file changed, 4 insertions(+)

Comments

Kleber Souza July 26, 2018, 4:19 p.m. | #1
On 07/06/18 07:33, Khalid Elmously wrote:
> From: Eric Sandeen <sandeen@redhat.com>
> 
> CVE-2018-10323
> 
> If xfs_bmap_extents_to_btree fails in a mode where we call
> xfs_iroot_realloc(-1) to de-allocate the root, set the
> format back to extents.
> 
> Otherwise we can assume we can dereference ifp->if_broot
> based on the XFS_DINODE_FMT_BTREE format, and crash.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=199423
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> (cherry-picked from 2c4306f719b083d17df2963bc761777576b8ad1b)
> Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 1bddbba6b80c..c756db755a5a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -725,12 +725,16 @@ xfs_bmap_extents_to_btree(
>  	*logflagsp = 0;
>  	if ((error = xfs_alloc_vextent(&args))) {
>  		xfs_iroot_realloc(ip, -1, whichfork);
> +		ASSERT(ifp->if_broot == NULL);
> +		XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
>  		xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
>  		return error;
>  	}
>  
>  	if (WARN_ON_ONCE(args.fsbno == NULLFSBLOCK)) {
>  		xfs_iroot_realloc(ip, -1, whichfork);
> +		ASSERT(ifp->if_broot == NULL);
> +		XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
>  		xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
>  		return -ENOSPC;
>  	}
> 

The patch doesn't apply on Xenial. The second block was introduced by
2fcc319d2467a (xfs: try any AG when allocating the first btree block
when reflinking), and it hasn't been backported to the Xenial kernel. So
the same backport for Trusty would potentially work for Xenial as well.

Artful is EOL.

So NAK for Xenial and Artful.


Kleber
Kleber Souza July 26, 2018, 4:20 p.m. | #2
On 07/06/18 07:33, Khalid Elmously wrote:
> From: Eric Sandeen <sandeen@redhat.com>
> 
> CVE-2018-10323
> 
> If xfs_bmap_extents_to_btree fails in a mode where we call
> xfs_iroot_realloc(-1) to de-allocate the root, set the
> format back to extents.
> 
> Otherwise we can assume we can dereference ifp->if_broot
> based on the XFS_DINODE_FMT_BTREE format, and crash.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=199423
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> (cherry-picked from 2c4306f719b083d17df2963bc761777576b8ad1b)
> Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 1bddbba6b80c..c756db755a5a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -725,12 +725,16 @@ xfs_bmap_extents_to_btree(
>  	*logflagsp = 0;
>  	if ((error = xfs_alloc_vextent(&args))) {
>  		xfs_iroot_realloc(ip, -1, whichfork);
> +		ASSERT(ifp->if_broot == NULL);
> +		XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
>  		xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
>  		return error;
>  	}
>  
>  	if (WARN_ON_ONCE(args.fsbno == NULLFSBLOCK)) {
>  		xfs_iroot_realloc(ip, -1, whichfork);
> +		ASSERT(ifp->if_broot == NULL);
> +		XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
>  		xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
>  		return -ENOSPC;
>  	}
> 

For Bionic only:

Acked-by: Kleber Sacilotto de Souza <kleber.souza@canonical.com>
Kleber Souza July 27, 2018, 1:06 p.m. | #3
On 07/06/18 07:33, Khalid Elmously wrote:
> From: Eric Sandeen <sandeen@redhat.com>
> 
> CVE-2018-10323
> 
> If xfs_bmap_extents_to_btree fails in a mode where we call
> xfs_iroot_realloc(-1) to de-allocate the root, set the
> format back to extents.
> 
> Otherwise we can assume we can dereference ifp->if_broot
> based on the XFS_DINODE_FMT_BTREE format, and crash.
> 
> Bugzilla: https://bugzilla.kernel.org/show_bug.cgi?id=199423
> Signed-off-by: Eric Sandeen <sandeen@redhat.com>
> Reviewed-by: Christoph Hellwig <hch@lst.de>
> Reviewed-by: Darrick J. Wong <darrick.wong@oracle.com>
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> (cherry-picked from 2c4306f719b083d17df2963bc761777576b8ad1b)
> Signed-off-by: Khalid Elmously <khalid.elmously@canonical.com>
> ---
>  fs/xfs/libxfs/xfs_bmap.c | 4 ++++
>  1 file changed, 4 insertions(+)
> 
> diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
> index 1bddbba6b80c..c756db755a5a 100644
> --- a/fs/xfs/libxfs/xfs_bmap.c
> +++ b/fs/xfs/libxfs/xfs_bmap.c
> @@ -725,12 +725,16 @@ xfs_bmap_extents_to_btree(
>  	*logflagsp = 0;
>  	if ((error = xfs_alloc_vextent(&args))) {
>  		xfs_iroot_realloc(ip, -1, whichfork);
> +		ASSERT(ifp->if_broot == NULL);
> +		XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
>  		xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
>  		return error;
>  	}
>  
>  	if (WARN_ON_ONCE(args.fsbno == NULLFSBLOCK)) {
>  		xfs_iroot_realloc(ip, -1, whichfork);
> +		ASSERT(ifp->if_broot == NULL);
> +		XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
>  		xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
>  		return -ENOSPC;
>  	}
> 

Applied to bionic/master-next branch.

Thanks,
Kleber

Patch

diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 1bddbba6b80c..c756db755a5a 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -725,12 +725,16 @@  xfs_bmap_extents_to_btree(
 	*logflagsp = 0;
 	if ((error = xfs_alloc_vextent(&args))) {
 		xfs_iroot_realloc(ip, -1, whichfork);
+		ASSERT(ifp->if_broot == NULL);
+		XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
 		xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
 		return error;
 	}
 
 	if (WARN_ON_ONCE(args.fsbno == NULLFSBLOCK)) {
 		xfs_iroot_realloc(ip, -1, whichfork);
+		ASSERT(ifp->if_broot == NULL);
+		XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS);
 		xfs_btree_del_cursor(cur, XFS_BTREE_ERROR);
 		return -ENOSPC;
 	}