From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from aserp2120.oracle.com ([141.146.126.78]:38816 "EHLO aserp2120.oracle.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1753002AbeDRCY2 (ORCPT ); Tue, 17 Apr 2018 22:24:28 -0400 Received: from pps.filterd (aserp2120.oracle.com [127.0.0.1]) by aserp2120.oracle.com (8.16.0.22/8.16.0.22) with SMTP id w3I2LU0Z088994 for ; Wed, 18 Apr 2018 02:24:28 GMT Received: from aserv0021.oracle.com (aserv0021.oracle.com [141.146.126.233]) by aserp2120.oracle.com with ESMTP id 2hdrxn8hy0-1 (version=TLSv1.2 cipher=ECDHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 18 Apr 2018 02:24:27 +0000 Received: from aserv0122.oracle.com (aserv0122.oracle.com [141.146.126.236]) by aserv0021.oracle.com (8.14.4/8.14.4) with ESMTP id w3I2ORrB001524 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-GCM-SHA384 bits=256 verify=OK) for ; Wed, 18 Apr 2018 02:24:27 GMT Received: from abhmp0016.oracle.com (abhmp0016.oracle.com [141.146.116.22]) by aserv0122.oracle.com (8.14.4/8.14.4) with ESMTP id w3I2ORwI022505 for ; Wed, 18 Apr 2018 02:24:27 GMT Date: Tue, 17 Apr 2018 19:24:26 -0700 From: "Darrick J. Wong" Subject: Re: [PATCH 1/1] xfs: fix a null pointer dereference in xfs_bmap_extents_to_btree Message-ID: <20180418022426.GL24738@magnolia> References: <1524017385-6671-1-git-send-email-shan.hai@oracle.com> <9fbccc81-103f-5b21-af3c-e32b82eba24b@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <9fbccc81-103f-5b21-af3c-e32b82eba24b@oracle.com> Sender: linux-xfs-owner@vger.kernel.org List-ID: List-Id: xfs To: Shan Hai Cc: linux-xfs@vger.kernel.org On Wed, Apr 18, 2018 at 10:13:22AM +0800, Shan Hai wrote: > > > On 2018年04月18日 10:09, Shan Hai wrote: > >Fuzzing tool reports a write to null pointer error in the > >xfs_bmap_extents_to_btree, fix it by bailing out on encountering > >a null pointer. > > > >Signed-off-by: Shan Hai > > This one supposed to be applied on top of below: > > https://www.spinics.net/lists/linux-xfs/msg17254.html > [PATCH] xfs: set format back to extents if xfs_bmap_extents_to_btree fails > > Thanks > Shan Hai > >--- > > fs/xfs/libxfs/xfs_bmap.c | 24 ++++++++++++++++-------- > > 1 file changed, 16 insertions(+), 8 deletions(-) > > > >diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c > >index 040eeda..90b743d 100644 > >--- a/fs/xfs/libxfs/xfs_bmap.c > >+++ b/fs/xfs/libxfs/xfs_bmap.c > >@@ -724,19 +724,14 @@ xfs_bmap_extents_to_btree( > > args.wasdel = wasdel; > > *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; > >+ goto err1; > > } > > 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; > >+ error = -ENOSPC; > >+ goto err1; > > } > > /* > > * Allocation can't fail, the space was reserved. > >@@ -748,6 +743,10 @@ xfs_bmap_extents_to_btree( > > ip->i_d.di_nblocks++; > > xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT, 1L); > > abp = xfs_btree_get_bufl(mp, tp, args.fsbno, 0); > >+ if (!abp) { When does this happen? We got args.fsbno from the allocator, so we're not out of space. Or are you saying that something fuzzed the free space btree, therefore the allocator gave back a nonsense block number (say pointing past the end of the fs), and therefore the _get_bufl call returned NULL? If so, maybe we need to change that WARN_ON_ONCE thing above to: if (WARN_ON_ONCE(...) || !xfs_verify_fsbno(..., args.fsbno)) { /* undo state and return */ } --D > >+ error = -ENOSPC; > >+ goto err2; > >+ } > > /* > > * Fill in the child block. > > */ > >@@ -787,6 +786,15 @@ xfs_bmap_extents_to_btree( > > *curp = cur; > > *logflagsp = XFS_ILOG_CORE | xfs_ilog_fbroot(whichfork); > > return 0; > >+ > >+err2: > >+ xfs_trans_mod_dquot_byino(tp, ip, XFS_TRANS_DQ_BCOUNT, -1L); > >+err1: > >+ xfs_iroot_realloc(ip, -1, whichfork); > >+ XFS_IFORK_FMT_SET(ip, whichfork, XFS_DINODE_FMT_EXTENTS); > >+ xfs_btree_del_cursor(cur, XFS_BTREE_ERROR); > >+ > >+ return error; > > } > > /* > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html