All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/9] xfs-4.18: strengthen corruption reporting
@ 2018-04-18  2:44 Darrick J. Wong
  2018-04-18  2:44 ` [PATCH 1/9] xfs: strengthen btree pointer checks before use Darrick J. Wong
                   ` (8 more replies)
  0 siblings, 9 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-04-18  2:44 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

Hi all,

This series is the result of me combing through the fuzz xfstests
looking for places where either we fail to report corruption to
userspace, or use fatal ASSERTs for on-disk metadata checks.
In other words, here are nine patches that convert ASSERTs into
-EFSCORRUPTED returns.  On systems with non-fatal asserts (or debugging
disabled) this prevents a number of memory corruptions and crashes
resulting from bad metadata.

If you're going to start using this mess, you probably ought to just
pull from my git trees.  The kernel patches[1] should apply against
4.17-rc1.  xfsprogs[2] and xfstests[3] can be found in their usual
places.  The git trees contain all four series' worth of changes.

Comments and questions are, as always, welcome.

--D

[1] https://git.kernel.org/cgit/linux/kernel/git/djwong/xfs-linux.git/log/?h=djwong-devel
[2] https://git.kernel.org/cgit/linux/kernel/git/djwong/xfsprogs-dev.git/log/?h=djwong-devel
[3] https://git.kernel.org/cgit/linux/kernel/git/djwong/xfstests-dev.git/log/?h=djwong-devel

^ permalink raw reply	[flat|nested] 10+ messages in thread

* [PATCH 1/9] xfs: strengthen btree pointer checks before use
  2018-04-18  2:44 [PATCH 0/9] xfs-4.18: strengthen corruption reporting Darrick J. Wong
@ 2018-04-18  2:44 ` Darrick J. Wong
  2018-04-18  2:44 ` [PATCH 2/9] xfs: don't assert when on-disk btree pointers are garbage Darrick J. Wong
                   ` (7 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-04-18  2:44 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Instead of ASSERTing on null btree pointers in xfs_btree_ptr_to_daddr,
use the new block number verifiers to ensure that the btree pointer
doesn't point to any sensitive areas (AG headers, past-EOFS) and return
-EFSCORRUPTED if this is the case.  Remove the ASSERT because on-disk
corruptions shouldn't trigger ASSERTs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_btree.c |   60 +++++++++++++++++++++++++++++++++++----------
 1 file changed, 47 insertions(+), 13 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index c825c81..2ddf47f 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -988,22 +988,44 @@ xfs_btree_readahead(
 	return xfs_btree_readahead_sblock(cur, lr, block);
 }
 
-STATIC xfs_daddr_t
+STATIC int
 xfs_btree_ptr_to_daddr(
 	struct xfs_btree_cur	*cur,
-	union xfs_btree_ptr	*ptr)
+	union xfs_btree_ptr	*ptr,
+	xfs_daddr_t		*daddr)
 {
+	xfs_fsblock_t		fsbno;
+	xfs_agblock_t		agbno;
+
 	if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
-		ASSERT(ptr->l != cpu_to_be64(NULLFSBLOCK));
+		fsbno = be64_to_cpu(ptr->l);
+
+		if (!xfs_btree_check_lptr(cur, fsbno, 1)) {
+			xfs_err(cur->bc_mp,
+"Inode %llu fork %d: Corrupt btree %d pointer to fsblock %llu",
+					cur->bc_private.b.ip->i_ino,
+					cur->bc_private.b.whichfork,
+					cur->bc_btnum, fsbno);
+			return -EFSCORRUPTED;
+		}
 
-		return XFS_FSB_TO_DADDR(cur->bc_mp, be64_to_cpu(ptr->l));
+		*daddr = XFS_FSB_TO_DADDR(cur->bc_mp, fsbno);
 	} else {
-		ASSERT(cur->bc_private.a.agno != NULLAGNUMBER);
-		ASSERT(ptr->s != cpu_to_be32(NULLAGBLOCK));
+		agbno = be32_to_cpu(ptr->s);
+
+		if (!xfs_btree_check_sptr(cur, agbno, 1)) {
+			xfs_err(cur->bc_mp,
+"AG %u: Corrupt btree %d pointer to agbno %u",
+					cur->bc_private.a.agno, cur->bc_btnum,
+					agbno);
+			return -EFSCORRUPTED;
+		}
 
-		return XFS_AGB_TO_DADDR(cur->bc_mp, cur->bc_private.a.agno,
-					be32_to_cpu(ptr->s));
+		*daddr = XFS_AGB_TO_DADDR(cur->bc_mp, cur->bc_private.a.agno,
+					agbno);
 	}
+
+	return 0;
 }
 
 /*
@@ -1018,8 +1040,11 @@ xfs_btree_readahead_ptr(
 	union xfs_btree_ptr	*ptr,
 	xfs_extlen_t		count)
 {
-	xfs_buf_readahead(cur->bc_mp->m_ddev_targp,
-			  xfs_btree_ptr_to_daddr(cur, ptr),
+	xfs_daddr_t		daddr;
+
+	if (xfs_btree_ptr_to_daddr(cur, ptr, &daddr))
+		return;
+	xfs_buf_readahead(cur->bc_mp->m_ddev_targp, daddr,
 			  cur->bc_mp->m_bsize * count, cur->bc_ops->buf_ops);
 }
 
@@ -1282,11 +1307,14 @@ xfs_btree_get_buf_block(
 {
 	struct xfs_mount	*mp = cur->bc_mp;
 	xfs_daddr_t		d;
+	int			error;
 
 	/* need to sort out how callers deal with failures first */
 	ASSERT(!(flags & XBF_TRYLOCK));
 
-	d = xfs_btree_ptr_to_daddr(cur, ptr);
+	error = xfs_btree_ptr_to_daddr(cur, ptr, &d);
+	if (error)
+		return error;
 	*bpp = xfs_trans_get_buf(cur->bc_tp, mp->m_ddev_targp, d,
 				 mp->m_bsize, flags);
 
@@ -1317,7 +1345,9 @@ xfs_btree_read_buf_block(
 	/* need to sort out how callers deal with failures first */
 	ASSERT(!(flags & XBF_TRYLOCK));
 
-	d = xfs_btree_ptr_to_daddr(cur, ptr);
+	error = xfs_btree_ptr_to_daddr(cur, ptr, &d);
+	if (error)
+		return error;
 	error = xfs_trans_read_buf(mp, cur->bc_tp, mp->m_ddev_targp, d,
 				   mp->m_bsize, flags, bpp,
 				   cur->bc_ops->buf_ops);
@@ -1764,6 +1794,7 @@ xfs_btree_lookup_get_block(
 	struct xfs_btree_block	**blkp) /* return btree block */
 {
 	struct xfs_buf		*bp;	/* buffer pointer for btree block */
+	xfs_daddr_t		daddr;
 	int			error = 0;
 
 	/* special case the root block if in an inode */
@@ -1780,7 +1811,10 @@ xfs_btree_lookup_get_block(
 	 * Otherwise throw it away and get a new one.
 	 */
 	bp = cur->bc_bufs[level];
-	if (bp && XFS_BUF_ADDR(bp) == xfs_btree_ptr_to_daddr(cur, pp)) {
+	error = xfs_btree_ptr_to_daddr(cur, pp, &daddr);
+	if (error)
+		return error;
+	if (bp && XFS_BUF_ADDR(bp) == daddr) {
 		*blkp = XFS_BUF_TO_BLOCK(bp);
 		return 0;
 	}


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 2/9] xfs: don't assert when on-disk btree pointers are garbage
  2018-04-18  2:44 [PATCH 0/9] xfs-4.18: strengthen corruption reporting Darrick J. Wong
  2018-04-18  2:44 ` [PATCH 1/9] xfs: strengthen btree pointer checks before use Darrick J. Wong
@ 2018-04-18  2:44 ` Darrick J. Wong
  2018-04-18  2:44 ` [PATCH 3/9] xfs: check directory bestfree information in the verifier Darrick J. Wong
                   ` (6 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-04-18  2:44 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Don't ASSERT when we encounter bad on-disk btree pointers in the debug
check functions.  Log the error to leave breadcrumbs and let the upper
layers deal with it.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_btree.c |   23 ++++++++++++++++-------
 1 file changed, 16 insertions(+), 7 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 2ddf47f..6101ad6 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -247,16 +247,25 @@ xfs_btree_check_ptr(
 	int			level)
 {
 	if (cur->bc_flags & XFS_BTREE_LONG_PTRS) {
-		XFS_WANT_CORRUPTED_RETURN(cur->bc_mp,
-				xfs_btree_check_lptr(cur,
-					be64_to_cpu((&ptr->l)[index]), level));
+		if (xfs_btree_check_lptr(cur, be64_to_cpu((&ptr->l)[index]),
+				level))
+			return 0;
+		xfs_err(cur->bc_mp,
+"Inode %llu fork %d: Corrupt btree %d pointer at level %d index %d.",
+				cur->bc_private.b.ip->i_ino,
+				cur->bc_private.b.whichfork, cur->bc_btnum,
+				level, index);
 	} else {
-		XFS_WANT_CORRUPTED_RETURN(cur->bc_mp,
-				xfs_btree_check_sptr(cur,
-					be32_to_cpu((&ptr->s)[index]), level));
+		if (xfs_btree_check_sptr(cur, be32_to_cpu((&ptr->s)[index]),
+				level))
+			return 0;
+		xfs_err(cur->bc_mp,
+"AG %u: Corrupt btree %d pointer at level %d index %d.",
+				cur->bc_private.a.agno, cur->bc_btnum,
+				level, index);
 	}
 
-	return 0;
+	return -EFSCORRUPTED;
 }
 #endif
 


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 3/9] xfs: check directory bestfree information in the verifier
  2018-04-18  2:44 [PATCH 0/9] xfs-4.18: strengthen corruption reporting Darrick J. Wong
  2018-04-18  2:44 ` [PATCH 1/9] xfs: strengthen btree pointer checks before use Darrick J. Wong
  2018-04-18  2:44 ` [PATCH 2/9] xfs: don't assert when on-disk btree pointers are garbage Darrick J. Wong
@ 2018-04-18  2:44 ` Darrick J. Wong
  2018-04-18  2:44 ` [PATCH 4/9] xfs: don't assert when reporting on-disk corruption while loading btree Darrick J. Wong
                   ` (5 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-04-18  2:44 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Create a variant of xfs_dir2_data_freefind that is suitable for use in a
verifier.  Because _freefind is called by the verifier, we simply
duplicate the _freefind function, convert the ASSERTs to return
__this_address, and modify the verifier to call our new function.  Once
we've made it impossible for directory blocks with bad bestfree data to
make it into the filesystem we can remove the DEBUG code from the
regular _freefind function.

Underlying argument: corruption of on-disk metadata should return
-EFSCORRUPTED instead of blowing ASSERTs.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_dir2_data.c |  121 +++++++++++++++++++++++++++++------------
 1 file changed, 86 insertions(+), 35 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_dir2_data.c b/fs/xfs/libxfs/xfs_dir2_data.c
index cb67ec7..bc5c0ba 100644
--- a/fs/xfs/libxfs/xfs_dir2_data.c
+++ b/fs/xfs/libxfs/xfs_dir2_data.c
@@ -33,6 +33,11 @@
 #include "xfs_cksum.h"
 #include "xfs_log.h"
 
+static xfs_failaddr_t xfs_dir2_data_freefind_verify(
+		struct xfs_dir2_data_hdr *hdr, struct xfs_dir2_data_free *bf,
+		struct xfs_dir2_data_unused *dup,
+		struct xfs_dir2_data_free **bf_ent);
+
 /*
  * Check the consistency of the data block.
  * The input can also be a block-format directory.
@@ -52,6 +57,7 @@ __xfs_dir3_data_check(
 	xfs_dir2_data_free_t	*dfp;		/* bestfree entry */
 	xfs_dir2_data_unused_t	*dup;		/* unused entry */
 	char			*endp;		/* end of useful data */
+	xfs_failaddr_t		fa;
 	int			freeseen;	/* mask of bestfrees seen */
 	xfs_dahash_t		hash;		/* hash of current name */
 	int			i;		/* leaf index */
@@ -154,7 +160,9 @@ __xfs_dir3_data_check(
 			if (be16_to_cpu(*xfs_dir2_data_unused_tag_p(dup)) !=
 			    (char *)dup - (char *)hdr)
 				return __this_address;
-			dfp = xfs_dir2_data_freefind(hdr, bf, dup);
+			fa = xfs_dir2_data_freefind_verify(hdr, bf, dup, &dfp);
+			if (fa)
+				return fa;
 			if (dfp) {
 				i = (int)(dfp - bf);
 				if ((freeseen & (1 << i)) != 0)
@@ -381,55 +389,98 @@ xfs_dir3_data_readahead(
 }
 
 /*
- * Given a data block and an unused entry from that block,
- * return the bestfree entry if any that corresponds to it.
+ * Find the bestfree entry that exactly coincides with unused directory space
+ * or a verifier error because the bestfree data are bad.
  */
-xfs_dir2_data_free_t *
-xfs_dir2_data_freefind(
-	struct xfs_dir2_data_hdr *hdr,		/* data block header */
-	struct xfs_dir2_data_free *bf,		/* bestfree table pointer */
-	struct xfs_dir2_data_unused *dup)	/* unused space */
+static xfs_failaddr_t
+xfs_dir2_data_freefind_verify(
+	struct xfs_dir2_data_hdr	*hdr,
+	struct xfs_dir2_data_free	*bf,
+	struct xfs_dir2_data_unused	*dup,
+	struct xfs_dir2_data_free	**bf_ent)
 {
-	xfs_dir2_data_free_t	*dfp;		/* bestfree entry */
-	xfs_dir2_data_aoff_t	off;		/* offset value needed */
-#ifdef DEBUG
-	int			matched;	/* matched the value */
-	int			seenzero;	/* saw a 0 bestfree entry */
-#endif
+	struct xfs_dir2_data_free	*dfp;
+	xfs_dir2_data_aoff_t		off;
+	bool				matched = false;
+	bool				seenzero = false;
 
+	*bf_ent = NULL;
 	off = (xfs_dir2_data_aoff_t)((char *)dup - (char *)hdr);
 
-#ifdef DEBUG
 	/*
 	 * Validate some consistency in the bestfree table.
 	 * Check order, non-overlapping entries, and if we find the
 	 * one we're looking for it has to be exact.
 	 */
-	ASSERT(hdr->magic == cpu_to_be32(XFS_DIR2_DATA_MAGIC) ||
-	       hdr->magic == cpu_to_be32(XFS_DIR3_DATA_MAGIC) ||
-	       hdr->magic == cpu_to_be32(XFS_DIR2_BLOCK_MAGIC) ||
-	       hdr->magic == cpu_to_be32(XFS_DIR3_BLOCK_MAGIC));
-	for (dfp = &bf[0], seenzero = matched = 0;
-	     dfp < &bf[XFS_DIR2_DATA_FD_COUNT];
-	     dfp++) {
+	for (dfp = &bf[0]; dfp < &bf[XFS_DIR2_DATA_FD_COUNT]; dfp++) {
 		if (!dfp->offset) {
-			ASSERT(!dfp->length);
-			seenzero = 1;
+			if (dfp->length)
+				return __this_address;
+			seenzero = true;
 			continue;
 		}
-		ASSERT(seenzero == 0);
+		if (seenzero)
+			return __this_address;
 		if (be16_to_cpu(dfp->offset) == off) {
-			matched = 1;
-			ASSERT(dfp->length == dup->length);
-		} else if (off < be16_to_cpu(dfp->offset))
-			ASSERT(off + be16_to_cpu(dup->length) <= be16_to_cpu(dfp->offset));
-		else
-			ASSERT(be16_to_cpu(dfp->offset) + be16_to_cpu(dfp->length) <= off);
-		ASSERT(matched || be16_to_cpu(dfp->length) >= be16_to_cpu(dup->length));
-		if (dfp > &bf[0])
-			ASSERT(be16_to_cpu(dfp[-1].length) >= be16_to_cpu(dfp[0].length));
+			matched = true;
+			if (dfp->length != dup->length)
+				return __this_address;
+		} else if (be16_to_cpu(dfp->offset) > off) {
+			if (off + be16_to_cpu(dup->length) >
+			    be16_to_cpu(dfp->offset))
+				return __this_address;
+		} else {
+			if (be16_to_cpu(dfp->offset) +
+			    be16_to_cpu(dfp->length) > off)
+				return __this_address;
+		}
+		if (!matched &&
+		    be16_to_cpu(dfp->length) < be16_to_cpu(dup->length))
+			return __this_address;
+		if (dfp > &bf[0] &&
+		    be16_to_cpu(dfp[-1].length) < be16_to_cpu(dfp[0].length))
+			return __this_address;
 	}
-#endif
+
+	/*
+	 * If this is smaller than the smallest bestfree entry,
+	 * it can't be there since they're sorted.
+	 */
+	if (be16_to_cpu(dup->length) <
+	    be16_to_cpu(bf[XFS_DIR2_DATA_FD_COUNT - 1].length))
+		return NULL;
+	/*
+	 * Look at the three bestfree entries for our guy.
+	 */
+	for (dfp = &bf[0]; dfp < &bf[XFS_DIR2_DATA_FD_COUNT]; dfp++) {
+		if (!dfp->offset)
+			return NULL;
+		if (be16_to_cpu(dfp->offset) == off) {
+			*bf_ent = dfp;
+			return NULL;
+		}
+	}
+	/*
+	 * Didn't find it.  This only happens if there are duplicate lengths.
+	 */
+	return NULL;
+}
+
+/*
+ * Given a data block and an unused entry from that block,
+ * return the bestfree entry if any that corresponds to it.
+ */
+xfs_dir2_data_free_t *
+xfs_dir2_data_freefind(
+	struct xfs_dir2_data_hdr *hdr,		/* data block header */
+	struct xfs_dir2_data_free *bf,		/* bestfree table pointer */
+	struct xfs_dir2_data_unused *dup)	/* unused space */
+{
+	xfs_dir2_data_free_t	*dfp;		/* bestfree entry */
+	xfs_dir2_data_aoff_t	off;		/* offset value needed */
+
+	off = (xfs_dir2_data_aoff_t)((char *)dup - (char *)hdr);
+
 	/*
 	 * If this is smaller than the smallest bestfree entry,
 	 * it can't be there since they're sorted.


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 4/9] xfs: don't assert when reporting on-disk corruption while loading btree
  2018-04-18  2:44 [PATCH 0/9] xfs-4.18: strengthen corruption reporting Darrick J. Wong
                   ` (2 preceding siblings ...)
  2018-04-18  2:44 ` [PATCH 3/9] xfs: check directory bestfree information in the verifier Darrick J. Wong
@ 2018-04-18  2:44 ` Darrick J. Wong
  2018-04-18  2:44 ` [PATCH 5/9] xfs: remove redundant ASSERT on insufficient bestfree length in _leaf_addname Darrick J. Wong
                   ` (4 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-04-18  2:44 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Don't bother ASSERTing when we're already going to log and return the
corruption status.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_bmap.c |    1 -
 1 file changed, 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c
index 7f079c4..436d13f 100644
--- a/fs/xfs/libxfs/xfs_bmap.c
+++ b/fs/xfs/libxfs/xfs_bmap.c
@@ -1240,7 +1240,6 @@ xfs_iread_extents(
 
 		num_recs = xfs_btree_get_numrecs(block);
 		if (unlikely(i + num_recs > nextents)) {
-			ASSERT(i + num_recs <= nextents);
 			xfs_warn(ip->i_mount,
 				"corrupt dinode %Lu, (btree extents).",
 				(unsigned long long) ip->i_ino);


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 5/9] xfs: remove redundant ASSERT on insufficient bestfree length in _leaf_addname
  2018-04-18  2:44 [PATCH 0/9] xfs-4.18: strengthen corruption reporting Darrick J. Wong
                   ` (3 preceding siblings ...)
  2018-04-18  2:44 ` [PATCH 4/9] xfs: don't assert when reporting on-disk corruption while loading btree Darrick J. Wong
@ 2018-04-18  2:44 ` Darrick J. Wong
  2018-04-18  2:44 ` [PATCH 6/9] xfs: xfs_alloc_get_rec should return EFSCORRUPTED for obvious bnobt corruption Darrick J. Wong
                   ` (3 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-04-18  2:44 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

In xfs_dir2_leaf_addname we ASSERT if the length of the unused space
described by bestfree[0] is less the amount of space we wish to consume.
Immediately after it is a call to xfs_dir2_data_use_free where the
offset parameter is offset of the unused space and the length parameter
is the amount of space we wish to consume.  Both values (and the unused
space pointer) are passed into xfs_dir2_data_check_free, which also
validates that the region of unused space is big enough to cover the
space we wish to consume.  This is effectively the same check that the
ASSERT covers, and since a check failure results in a corruption message
being logged we can remove the ASSERT.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_dir2_leaf.c |    1 -
 1 file changed, 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_dir2_leaf.c b/fs/xfs/libxfs/xfs_dir2_leaf.c
index 50fc9c0..9367f2a 100644
--- a/fs/xfs/libxfs/xfs_dir2_leaf.c
+++ b/fs/xfs/libxfs/xfs_dir2_leaf.c
@@ -872,7 +872,6 @@ xfs_dir2_leaf_addname(
 	 */
 	dup = (xfs_dir2_data_unused_t *)
 	      ((char *)hdr + be16_to_cpu(bf[0].offset));
-	ASSERT(be16_to_cpu(dup->length) >= length);
 	needscan = needlog = 0;
 	/*
 	 * Mark the initial part of our freespace in use for the new entry.


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 6/9] xfs: xfs_alloc_get_rec should return EFSCORRUPTED for obvious bnobt corruption
  2018-04-18  2:44 [PATCH 0/9] xfs-4.18: strengthen corruption reporting Darrick J. Wong
                   ` (4 preceding siblings ...)
  2018-04-18  2:44 ` [PATCH 5/9] xfs: remove redundant ASSERT on insufficient bestfree length in _leaf_addname Darrick J. Wong
@ 2018-04-18  2:44 ` Darrick J. Wong
  2018-04-18  2:45 ` [PATCH 7/9] xfs: btree lookup shouldn't ASSERT on empty btree nodes Darrick J. Wong
                   ` (2 subsequent siblings)
  8 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-04-18  2:44 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Return -EFSCORRUPTED when the bnobt/cntbt return obviously corrupt
values, rather than letting them bounce around in the internal code.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_alloc.c |   12 ++++++++----
 1 file changed, 8 insertions(+), 4 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
index 6dd0984..98b6112 100644
--- a/fs/xfs/libxfs/xfs_alloc.c
+++ b/fs/xfs/libxfs/xfs_alloc.c
@@ -228,10 +228,14 @@ xfs_alloc_get_rec(
 	int			error;
 
 	error = xfs_btree_get_rec(cur, &rec, stat);
-	if (!error && *stat == 1) {
-		*bno = be32_to_cpu(rec->alloc.ar_startblock);
-		*len = be32_to_cpu(rec->alloc.ar_blockcount);
-	}
+	if (error || !(*stat))
+		return error;
+	if (rec->alloc.ar_blockcount == 0)
+		return -EFSCORRUPTED;
+
+	*bno = be32_to_cpu(rec->alloc.ar_startblock);
+	*len = be32_to_cpu(rec->alloc.ar_blockcount);
+
 	return error;
 }
 


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 7/9] xfs: btree lookup shouldn't ASSERT on empty btree nodes
  2018-04-18  2:44 [PATCH 0/9] xfs-4.18: strengthen corruption reporting Darrick J. Wong
                   ` (5 preceding siblings ...)
  2018-04-18  2:44 ` [PATCH 6/9] xfs: xfs_alloc_get_rec should return EFSCORRUPTED for obvious bnobt corruption Darrick J. Wong
@ 2018-04-18  2:45 ` Darrick J. Wong
  2018-04-18  2:45 ` [PATCH 8/9] xfs: don't ASSERT on short form btree root pointer of zero Darrick J. Wong
  2018-04-18  2:45 ` [PATCH 9/9] xfs: don't return garbage buffers in xfs_da3_node_read Darrick J. Wong
  8 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-04-18  2:45 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

If a btree lookup encounters an empty btree node or an empty btree leaf
on a multi-level btree, that's evidence of a corrupt on-disk btree.
Therefore, we should return -EFSCORRUPTED to the upper levels, not an
ASSERT failure.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_btree.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index 6101ad6..4e79baa 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -1939,7 +1939,8 @@ xfs_btree_lookup(
 			high = xfs_btree_get_numrecs(block);
 			if (!high) {
 				/* Block is empty, must be an empty leaf. */
-				ASSERT(level == 0 && cur->bc_nlevels == 1);
+				if (level != 0 || cur->bc_nlevels != 1)
+					return -EFSCORRUPTED;
 
 				cur->bc_ptrs[0] = dir != XFS_LOOKUP_LE;
 				*stat = 0;


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 8/9] xfs: don't ASSERT on short form btree root pointer of zero
  2018-04-18  2:44 [PATCH 0/9] xfs-4.18: strengthen corruption reporting Darrick J. Wong
                   ` (6 preceding siblings ...)
  2018-04-18  2:45 ` [PATCH 7/9] xfs: btree lookup shouldn't ASSERT on empty btree nodes Darrick J. Wong
@ 2018-04-18  2:45 ` Darrick J. Wong
  2018-04-18  2:45 ` [PATCH 9/9] xfs: don't return garbage buffers in xfs_da3_node_read Darrick J. Wong
  8 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-04-18  2:45 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

Don't ASSERT if the short form btree root pointer is zero.  Now that we
use xfs_verify_agbno to check all short form btree pointers, we'll let
that log the error and pass it to the upper layers.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_alloc_btree.c    |    1 -
 fs/xfs/libxfs/xfs_refcount_btree.c |    1 -
 fs/xfs/libxfs/xfs_rmap_btree.c     |    1 -
 3 files changed, 3 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_alloc_btree.c b/fs/xfs/libxfs/xfs_alloc_btree.c
index 18aec7a..a4a001f 100644
--- a/fs/xfs/libxfs/xfs_alloc_btree.c
+++ b/fs/xfs/libxfs/xfs_alloc_btree.c
@@ -242,7 +242,6 @@ xfs_allocbt_init_ptr_from_cur(
 	struct xfs_agf		*agf = XFS_BUF_TO_AGF(cur->bc_private.a.agbp);
 
 	ASSERT(cur->bc_private.a.agno == be32_to_cpu(agf->agf_seqno));
-	ASSERT(agf->agf_roots[cur->bc_btnum] != 0);
 
 	ptr->s = agf->agf_roots[cur->bc_btnum];
 }
diff --git a/fs/xfs/libxfs/xfs_refcount_btree.c b/fs/xfs/libxfs/xfs_refcount_btree.c
index 375abfe..f652b48 100644
--- a/fs/xfs/libxfs/xfs_refcount_btree.c
+++ b/fs/xfs/libxfs/xfs_refcount_btree.c
@@ -192,7 +192,6 @@ xfs_refcountbt_init_ptr_from_cur(
 	struct xfs_agf		*agf = XFS_BUF_TO_AGF(cur->bc_private.a.agbp);
 
 	ASSERT(cur->bc_private.a.agno == be32_to_cpu(agf->agf_seqno));
-	ASSERT(agf->agf_refcount_root != 0);
 
 	ptr->s = agf->agf_refcount_root;
 }
diff --git a/fs/xfs/libxfs/xfs_rmap_btree.c b/fs/xfs/libxfs/xfs_rmap_btree.c
index d756e0b..78ab00b 100644
--- a/fs/xfs/libxfs/xfs_rmap_btree.c
+++ b/fs/xfs/libxfs/xfs_rmap_btree.c
@@ -234,7 +234,6 @@ xfs_rmapbt_init_ptr_from_cur(
 	struct xfs_agf		*agf = XFS_BUF_TO_AGF(cur->bc_private.a.agbp);
 
 	ASSERT(cur->bc_private.a.agno == be32_to_cpu(agf->agf_seqno));
-	ASSERT(agf->agf_roots[cur->bc_btnum] != 0);
 
 	ptr->s = agf->agf_roots[cur->bc_btnum];
 }


^ permalink raw reply related	[flat|nested] 10+ messages in thread

* [PATCH 9/9] xfs: don't return garbage buffers in xfs_da3_node_read
  2018-04-18  2:44 [PATCH 0/9] xfs-4.18: strengthen corruption reporting Darrick J. Wong
                   ` (7 preceding siblings ...)
  2018-04-18  2:45 ` [PATCH 8/9] xfs: don't ASSERT on short form btree root pointer of zero Darrick J. Wong
@ 2018-04-18  2:45 ` Darrick J. Wong
  8 siblings, 0 replies; 10+ messages in thread
From: Darrick J. Wong @ 2018-04-18  2:45 UTC (permalink / raw)
  To: darrick.wong; +Cc: linux-xfs

From: Darrick J. Wong <darrick.wong@oracle.com>

If we're reading a node in a dir/attr btree and the buffer comes off the
disk with a magic number we don't recognize, don't ASSERT and don't set
a garbage buffer type (0 also triggers ASSERTs).  Instead, report the
corruption, release the buffer, and return -EFSCORRUPTED because that's
what the dabtree is -- corrupt.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_da_btree.c |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index ea187b4..39c1013 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -305,9 +305,11 @@ xfs_da3_node_read(
 			type = XFS_BLFT_DIR_LEAFN_BUF;
 			break;
 		default:
-			type = 0;
-			ASSERT(0);
-			break;
+			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW,
+					tp->t_mountp, info);
+			xfs_trans_brelse(tp, *bpp);
+			*bpp = NULL;
+			return -EFSCORRUPTED;
 		}
 		xfs_trans_buf_set_type(tp, *bpp, type);
 	}


^ permalink raw reply related	[flat|nested] 10+ messages in thread

end of thread, other threads:[~2018-04-18  2:45 UTC | newest]

Thread overview: 10+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-04-18  2:44 [PATCH 0/9] xfs-4.18: strengthen corruption reporting Darrick J. Wong
2018-04-18  2:44 ` [PATCH 1/9] xfs: strengthen btree pointer checks before use Darrick J. Wong
2018-04-18  2:44 ` [PATCH 2/9] xfs: don't assert when on-disk btree pointers are garbage Darrick J. Wong
2018-04-18  2:44 ` [PATCH 3/9] xfs: check directory bestfree information in the verifier Darrick J. Wong
2018-04-18  2:44 ` [PATCH 4/9] xfs: don't assert when reporting on-disk corruption while loading btree Darrick J. Wong
2018-04-18  2:44 ` [PATCH 5/9] xfs: remove redundant ASSERT on insufficient bestfree length in _leaf_addname Darrick J. Wong
2018-04-18  2:44 ` [PATCH 6/9] xfs: xfs_alloc_get_rec should return EFSCORRUPTED for obvious bnobt corruption Darrick J. Wong
2018-04-18  2:45 ` [PATCH 7/9] xfs: btree lookup shouldn't ASSERT on empty btree nodes Darrick J. Wong
2018-04-18  2:45 ` [PATCH 8/9] xfs: don't ASSERT on short form btree root pointer of zero Darrick J. Wong
2018-04-18  2:45 ` [PATCH 9/9] xfs: don't return garbage buffers in xfs_da3_node_read Darrick J. Wong

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.