[PATCH 0/3] xfs: various 4.15 fixes

[PATCH 0/3] xfs: various 4.15 fixes

[Darrick J. Wong]

Hi all,

Here's a rollup of the last few patches I have for 4.15.

--Darrick

[PATCH 1/3] xfs: refactor extended attribute list operation

[Darrick J. Wong]

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

When we're iterating the attribute list and we can't find our previous
location based off the attribute cursor, we'll instead walk down the
attribute btree from the root trying to find where we left off.  Move
this code into a separate function for later cleanups.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/xfs_attr_list.c |  130 +++++++++++++++++++++++++++++-------------------
 1 file changed, 78 insertions(+), 52 deletions(-)


diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 5816786..021ec5a 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -204,19 +204,83 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
 	return 0;
 }
 
+/*
+ * We didn't find the block & hash mentioned in the cursor state, so
+ * walk down the attr btree looking for the hash.
+ */
 STATIC int
-xfs_attr_node_list(xfs_attr_list_context_t *context)
+xfs_attr_node_list_lookup(
+	struct xfs_attr_list_context	*context,
+	struct attrlist_cursor_kern	*cursor,
+	struct xfs_buf			**pbp)
 {
-	attrlist_cursor_kern_t *cursor;
-	xfs_attr_leafblock_t *leaf;
-	xfs_da_intnode_t *node;
-	struct xfs_attr3_icleaf_hdr leafhdr;
-	struct xfs_da3_icnode_hdr nodehdr;
-	struct xfs_da_node_entry *btree;
-	int error, i;
-	struct xfs_buf *bp;
-	struct xfs_inode	*dp = context->dp;
-	struct xfs_mount	*mp = dp->i_mount;
+	struct xfs_da3_icnode_hdr	nodehdr;
+	struct xfs_da_intnode		*node;
+	struct xfs_da_node_entry	*btree;
+	struct xfs_inode		*dp = context->dp;
+	struct xfs_mount		*mp = dp->i_mount;
+	struct xfs_trans		*tp = context->tp;
+	struct xfs_buf			*bp;
+	int				i;
+	int				error = 0;
+	uint16_t			magic;
+
+	ASSERT(*pbp == NULL);
+	cursor->blkno = 0;
+	for (;;) {
+		error = xfs_da3_node_read(tp, dp, cursor->blkno, -1, &bp,
+				XFS_ATTR_FORK);
+		if (error)
+			return error;
+		node = bp->b_addr;
+		magic = be16_to_cpu(node->hdr.info.magic);
+		if (magic == XFS_ATTR_LEAF_MAGIC ||
+		    magic == XFS_ATTR3_LEAF_MAGIC)
+			break;
+		if (magic != XFS_DA_NODE_MAGIC &&
+		    magic != XFS_DA3_NODE_MAGIC) {
+			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
+					node);
+			goto out_corruptbuf;
+		}
+
+		dp->d_ops->node_hdr_from_disk(&nodehdr, node);
+
+		btree = dp->d_ops->node_tree_p(node);
+		for (i = 0; i < nodehdr.count; btree++, i++) {
+			if (cursor->hashval <= be32_to_cpu(btree->hashval)) {
+				cursor->blkno = be32_to_cpu(btree->before);
+				trace_xfs_attr_list_node_descend(context,
+						btree);
+				break;
+			}
+		}
+		xfs_trans_brelse(tp, bp);
+
+		if (i == nodehdr.count)
+			return 0;
+	}
+
+	*pbp = bp;
+	return 0;
+
+out_corruptbuf:
+	xfs_trans_brelse(tp, bp);
+	return -EFSCORRUPTED;
+}
+
+STATIC int
+xfs_attr_node_list(
+	struct xfs_attr_list_context	*context)
+{
+	struct xfs_attr3_icleaf_hdr	leafhdr;
+	struct attrlist_cursor_kern	*cursor;
+	struct xfs_attr_leafblock	*leaf;
+	struct xfs_da_intnode		*node;
+	struct xfs_buf			*bp;
+	struct xfs_inode		*dp = context->dp;
+	struct xfs_mount		*mp = dp->i_mount;
+	int				error;
 
 	trace_xfs_attr_node_list(context);
 
@@ -277,47 +341,9 @@ xfs_attr_node_list(xfs_attr_list_context_t *context)
 	 * Note that start of node block is same as start of leaf block.
 	 */
 	if (bp == NULL) {
-		cursor->blkno = 0;
-		for (;;) {
-			uint16_t magic;
-
-			error = xfs_da3_node_read(context->tp, dp,
-						      cursor->blkno, -1, &bp,
-						      XFS_ATTR_FORK);
-			if (error)
-				return error;
-			node = bp->b_addr;
-			magic = be16_to_cpu(node->hdr.info.magic);
-			if (magic == XFS_ATTR_LEAF_MAGIC ||
-			    magic == XFS_ATTR3_LEAF_MAGIC)
-				break;
-			if (magic != XFS_DA_NODE_MAGIC &&
-			    magic != XFS_DA3_NODE_MAGIC) {
-				XFS_CORRUPTION_ERROR("xfs_attr_node_list(3)",
-						     XFS_ERRLEVEL_LOW,
-						     context->dp->i_mount,
-						     node);
-				xfs_trans_brelse(context->tp, bp);
-				return -EFSCORRUPTED;
-			}
-
-			dp->d_ops->node_hdr_from_disk(&nodehdr, node);
-			btree = dp->d_ops->node_tree_p(node);
-			for (i = 0; i < nodehdr.count; btree++, i++) {
-				if (cursor->hashval
-						<= be32_to_cpu(btree->hashval)) {
-					cursor->blkno = be32_to_cpu(btree->before);
-					trace_xfs_attr_list_node_descend(context,
-									 btree);
-					break;
-				}
-			}
-			if (i == nodehdr.count) {
-				xfs_trans_brelse(context->tp, bp);
-				return 0;
-			}
-			xfs_trans_brelse(context->tp, bp);
-		}
+		error = xfs_attr_node_list_lookup(context, cursor, &bp);
+		if (error || !bp)
+			return error;
 	}
 	ASSERT(bp != NULL);
 

[PATCH 2/3] xfs: abort dir/attr btree operation if btree is obviously weird

[Darrick J. Wong]

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

Abort an dir/attr btree operation if the attr btree has obvious problems
like loops back to the root or pointers don't point down the tree.
Found by fuzzing btree[0].before to zero in xfs/402, which livelocks on
the cycle in the attr btree.

Apply the same checks to xfs_da3_node_lookup_int.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_da_btree.c |   22 +++++++++++++++++++++-
 fs/xfs/xfs_attr_list.c       |   20 ++++++++++++++++++++
 2 files changed, 41 insertions(+), 1 deletion(-)


diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
index 6d43358..6516115 100644
--- a/fs/xfs/libxfs/xfs_da_btree.c
+++ b/fs/xfs/libxfs/xfs_da_btree.c
@@ -1466,6 +1466,7 @@ xfs_da3_node_lookup_int(
 	int			max;
 	int			error;
 	int			retval;
+	unsigned int		expected_level = 0;
 	struct xfs_inode	*dp = state->args->dp;
 
 	args = state->args;
@@ -1474,7 +1475,7 @@ xfs_da3_node_lookup_int(
 	 * Descend thru the B-tree searching each level for the right
 	 * node to use, until the right hashval is found.
 	 */
-	blkno = (args->whichfork == XFS_DATA_FORK)? args->geo->leafblk : 0;
+	blkno = args->geo->leafblk;
 	for (blk = &state->path.blk[0], state->path.active = 1;
 			 state->path.active <= XFS_DA_NODE_MAXDEPTH;
 			 blk++, state->path.active++) {
@@ -1517,6 +1518,18 @@ xfs_da3_node_lookup_int(
 		dp->d_ops->node_hdr_from_disk(&nodehdr, node);
 		btree = dp->d_ops->node_tree_p(node);
 
+		/* Tree taller than we can handle; bail out! */
+		if (nodehdr.level >= XFS_DA_NODE_MAXDEPTH)
+			return -EFSCORRUPTED;
+
+		/* Check the level from the root. */
+		if (blkno == args->geo->leafblk)
+			expected_level = nodehdr.level - 1;
+		else if (expected_level != nodehdr.level)
+			return -EFSCORRUPTED;
+		else
+			expected_level--;
+
 		max = nodehdr.count;
 		blk->hashval = be32_to_cpu(btree[max - 1].hashval);
 
@@ -1562,8 +1575,15 @@ xfs_da3_node_lookup_int(
 			blk->index = probe;
 			blkno = be32_to_cpu(btree[probe].before);
 		}
+
+		/* We can't point back to the root. */
+		if (blkno == args->geo->leafblk)
+			return -EFSCORRUPTED;
 	}
 
+	if (expected_level != 0)
+		return -EFSCORRUPTED;
+
 	/*
 	 * A leaf block that ends in the hashval that we are interested in
 	 * (final hashval == search hashval) means that the next block may
diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
index 021ec5a..a360310 100644
--- a/fs/xfs/xfs_attr_list.c
+++ b/fs/xfs/xfs_attr_list.c
@@ -223,6 +223,7 @@ xfs_attr_node_list_lookup(
 	struct xfs_buf			*bp;
 	int				i;
 	int				error = 0;
+	unsigned int			expected_level = 0;
 	uint16_t			magic;
 
 	ASSERT(*pbp == NULL);
@@ -246,6 +247,18 @@ xfs_attr_node_list_lookup(
 
 		dp->d_ops->node_hdr_from_disk(&nodehdr, node);
 
+		/* Tree taller than we can handle; bail out! */
+		if (nodehdr.level >= XFS_DA_NODE_MAXDEPTH)
+			goto out_corruptbuf;
+
+		/* Check the level from the root node. */
+		if (cursor->blkno == 0)
+			expected_level = nodehdr.level - 1;
+		else if (expected_level != nodehdr.level)
+			goto out_corruptbuf;
+		else
+			expected_level--;
+
 		btree = dp->d_ops->node_tree_p(node);
 		for (i = 0; i < nodehdr.count; btree++, i++) {
 			if (cursor->hashval <= be32_to_cpu(btree->hashval)) {
@@ -259,8 +272,15 @@ xfs_attr_node_list_lookup(
 
 		if (i == nodehdr.count)
 			return 0;
+
+		/* We can't point back to the root. */
+		if (cursor->blkno == 0)
+			return -EFSCORRUPTED;
 	}
 
+	if (expected_level != 0)
+		goto out_corruptbuf;
+
 	*pbp = bp;
 	return 0;
 

[PATCH 3/3] xfs: compare btree block keys to parent block's keys during scrub

[Darrick J. Wong]

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

When we're done checking all the records/keys in a btree block, compute
the low and high key of the block and compare them to the associated key
in the parent btree block.

Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
---
 fs/xfs/libxfs/xfs_btree.c |    4 ++--
 fs/xfs/libxfs/xfs_btree.h |    4 ++++
 fs/xfs/scrub/btree.c      |   46 +++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 52 insertions(+), 2 deletions(-)


diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
index b3cd82a..848f371 100644
--- a/fs/xfs/libxfs/xfs_btree.c
+++ b/fs/xfs/libxfs/xfs_btree.c
@@ -2027,7 +2027,7 @@ xfs_btree_lookup(
 }
 
 /* Find the high key storage area from a regular key. */
-STATIC union xfs_btree_key *
+union xfs_btree_key *
 xfs_btree_high_key_from_key(
 	struct xfs_btree_cur	*cur,
 	union xfs_btree_key	*key)
@@ -2101,7 +2101,7 @@ xfs_btree_get_node_keys(
 }
 
 /* Derive the keys for any btree block. */
-STATIC void
+void
 xfs_btree_get_keys(
 	struct xfs_btree_cur	*cur,
 	struct xfs_btree_block	*block,
diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
index be82f41..b57501c 100644
--- a/fs/xfs/libxfs/xfs_btree.h
+++ b/fs/xfs/libxfs/xfs_btree.h
@@ -541,5 +541,9 @@ int64_t xfs_btree_diff_two_ptrs(struct xfs_btree_cur *cur,
 void xfs_btree_get_sibling(struct xfs_btree_cur *cur,
 			   struct xfs_btree_block *block,
 			   union xfs_btree_ptr *ptr, int lr);
+void xfs_btree_get_keys(struct xfs_btree_cur *cur,
+		struct xfs_btree_block *block, union xfs_btree_key *key);
+union xfs_btree_key *xfs_btree_high_key_from_key(struct xfs_btree_cur *cur,
+		union xfs_btree_key *key);
 
 #endif	/* __XFS_BTREE_H__ */
diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
index 9ccf763..9e8b67a 100644
--- a/fs/xfs/scrub/btree.c
+++ b/fs/xfs/scrub/btree.c
@@ -358,6 +358,50 @@ xfs_scrub_btree_get_block(
 }
 
 /*
+ * Check that the low and high keys of this block match the keys stored
+ * in the parent block.
+ */
+STATIC void
+xfs_scrub_btree_block_keys(
+	struct xfs_scrub_btree		*bs,
+	int				level,
+	struct xfs_btree_block		*block)
+{
+	union xfs_btree_key		block_keys;
+	struct xfs_btree_cur		*cur = bs->cur;
+	union xfs_btree_key		*high_bk;
+	union xfs_btree_key		*parent_keys;
+	union xfs_btree_key		*high_pk;
+	struct xfs_btree_block		*parent_block;
+	struct xfs_buf			*bp;
+
+	if (level >= cur->bc_nlevels - 1)
+		return;
+
+	/* Calculate the keys for this block. */
+	xfs_btree_get_keys(cur, block, &block_keys);
+
+	/* Obtain the parent's copy of the keys for this block. */
+	parent_block = xfs_btree_get_block(cur, level + 1, &bp);
+	parent_keys = xfs_btree_key_addr(cur, cur->bc_ptrs[level + 1],
+			parent_block);
+
+	if (cur->bc_ops->diff_two_keys(cur, &block_keys, parent_keys) != 0)
+		xfs_scrub_btree_set_corrupt(bs->sc, cur, 1);
+
+	if (!(cur->bc_flags & XFS_BTREE_OVERLAPPING))
+		return;
+
+	/* Get high keys */
+	high_bk = xfs_btree_high_key_from_key(cur, &block_keys);
+	high_pk = xfs_btree_high_key_addr(cur, cur->bc_ptrs[level + 1],
+			parent_block);
+
+	if (cur->bc_ops->diff_two_keys(cur, high_bk, high_pk) != 0)
+		xfs_scrub_btree_set_corrupt(bs->sc, cur, 1);
+}
+
+/*
  * Visit all nodes and leaves of a btree.  Check that all pointers and
  * records are in order, that the keys reflect the records, and use a callback
  * so that the caller can verify individual records.
@@ -418,6 +462,7 @@ xfs_scrub_btree(
 			/* End of leaf, pop back towards the root. */
 			if (cur->bc_ptrs[level] >
 			    be16_to_cpu(block->bb_numrecs)) {
+				xfs_scrub_btree_block_keys(&bs, level, block);
 				if (level < cur->bc_nlevels - 1)
 					cur->bc_ptrs[level + 1]++;
 				level++;
@@ -442,6 +487,7 @@ xfs_scrub_btree(
 
 		/* End of node, pop back towards the root. */
 		if (cur->bc_ptrs[level] > be16_to_cpu(block->bb_numrecs)) {
+			xfs_scrub_btree_block_keys(&bs, level, block);
 			if (level < cur->bc_nlevels - 1)
 				cur->bc_ptrs[level + 1]++;
 			level++;

Re: [PATCH 1/3] xfs: refactor extended attribute list operation

[Brian Foster]

On Thu, Oct 26, 2017 at 03:49:30PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When we're iterating the attribute list and we can't find our previous
> location based off the attribute cursor, we'll instead walk down the
> attribute btree from the root trying to find where we left off.  Move
> this code into a separate function for later cleanups.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/xfs_attr_list.c |  130 +++++++++++++++++++++++++++++-------------------
>  1 file changed, 78 insertions(+), 52 deletions(-)
> 
> 
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 5816786..021ec5a 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -204,19 +204,83 @@ xfs_attr_shortform_list(xfs_attr_list_context_t *context)
>  	return 0;
>  }
>  
> +/*
> + * We didn't find the block & hash mentioned in the cursor state, so
> + * walk down the attr btree looking for the hash.
> + */
>  STATIC int
> -xfs_attr_node_list(xfs_attr_list_context_t *context)
> +xfs_attr_node_list_lookup(
> +	struct xfs_attr_list_context	*context,
> +	struct attrlist_cursor_kern	*cursor,
> +	struct xfs_buf			**pbp)
>  {
> -	attrlist_cursor_kern_t *cursor;
> -	xfs_attr_leafblock_t *leaf;
> -	xfs_da_intnode_t *node;
> -	struct xfs_attr3_icleaf_hdr leafhdr;
> -	struct xfs_da3_icnode_hdr nodehdr;
> -	struct xfs_da_node_entry *btree;
> -	int error, i;
> -	struct xfs_buf *bp;
> -	struct xfs_inode	*dp = context->dp;
> -	struct xfs_mount	*mp = dp->i_mount;
> +	struct xfs_da3_icnode_hdr	nodehdr;
> +	struct xfs_da_intnode		*node;
> +	struct xfs_da_node_entry	*btree;
> +	struct xfs_inode		*dp = context->dp;
> +	struct xfs_mount		*mp = dp->i_mount;
> +	struct xfs_trans		*tp = context->tp;
> +	struct xfs_buf			*bp;
> +	int				i;
> +	int				error = 0;
> +	uint16_t			magic;
> +
> +	ASSERT(*pbp == NULL);
> +	cursor->blkno = 0;
> +	for (;;) {
> +		error = xfs_da3_node_read(tp, dp, cursor->blkno, -1, &bp,
> +				XFS_ATTR_FORK);
> +		if (error)
> +			return error;
> +		node = bp->b_addr;
> +		magic = be16_to_cpu(node->hdr.info.magic);
> +		if (magic == XFS_ATTR_LEAF_MAGIC ||
> +		    magic == XFS_ATTR3_LEAF_MAGIC)
> +			break;
> +		if (magic != XFS_DA_NODE_MAGIC &&
> +		    magic != XFS_DA3_NODE_MAGIC) {
> +			XFS_CORRUPTION_ERROR(__func__, XFS_ERRLEVEL_LOW, mp,
> +					node);
> +			goto out_corruptbuf;
> +		}
> +
> +		dp->d_ops->node_hdr_from_disk(&nodehdr, node);
> +
> +		btree = dp->d_ops->node_tree_p(node);
> +		for (i = 0; i < nodehdr.count; btree++, i++) {
> +			if (cursor->hashval <= be32_to_cpu(btree->hashval)) {
> +				cursor->blkno = be32_to_cpu(btree->before);
> +				trace_xfs_attr_list_node_descend(context,
> +						btree);
> +				break;
> +			}
> +		}
> +		xfs_trans_brelse(tp, bp);
> +
> +		if (i == nodehdr.count)
> +			return 0;
> +	}
> +
> +	*pbp = bp;
> +	return 0;
> +
> +out_corruptbuf:
> +	xfs_trans_brelse(tp, bp);
> +	return -EFSCORRUPTED;
> +}
> +
> +STATIC int
> +xfs_attr_node_list(
> +	struct xfs_attr_list_context	*context)
> +{
> +	struct xfs_attr3_icleaf_hdr	leafhdr;
> +	struct attrlist_cursor_kern	*cursor;
> +	struct xfs_attr_leafblock	*leaf;
> +	struct xfs_da_intnode		*node;
> +	struct xfs_buf			*bp;
> +	struct xfs_inode		*dp = context->dp;
> +	struct xfs_mount		*mp = dp->i_mount;
> +	int				error;
>  
>  	trace_xfs_attr_node_list(context);
>  
> @@ -277,47 +341,9 @@ xfs_attr_node_list(xfs_attr_list_context_t *context)
>  	 * Note that start of node block is same as start of leaf block.
>  	 */
>  	if (bp == NULL) {
> -		cursor->blkno = 0;
> -		for (;;) {
> -			uint16_t magic;
> -
> -			error = xfs_da3_node_read(context->tp, dp,
> -						      cursor->blkno, -1, &bp,
> -						      XFS_ATTR_FORK);
> -			if (error)
> -				return error;
> -			node = bp->b_addr;
> -			magic = be16_to_cpu(node->hdr.info.magic);
> -			if (magic == XFS_ATTR_LEAF_MAGIC ||
> -			    magic == XFS_ATTR3_LEAF_MAGIC)
> -				break;
> -			if (magic != XFS_DA_NODE_MAGIC &&
> -			    magic != XFS_DA3_NODE_MAGIC) {
> -				XFS_CORRUPTION_ERROR("xfs_attr_node_list(3)",
> -						     XFS_ERRLEVEL_LOW,
> -						     context->dp->i_mount,
> -						     node);
> -				xfs_trans_brelse(context->tp, bp);
> -				return -EFSCORRUPTED;
> -			}
> -
> -			dp->d_ops->node_hdr_from_disk(&nodehdr, node);
> -			btree = dp->d_ops->node_tree_p(node);
> -			for (i = 0; i < nodehdr.count; btree++, i++) {
> -				if (cursor->hashval
> -						<= be32_to_cpu(btree->hashval)) {
> -					cursor->blkno = be32_to_cpu(btree->before);
> -					trace_xfs_attr_list_node_descend(context,
> -									 btree);
> -					break;
> -				}
> -			}
> -			if (i == nodehdr.count) {
> -				xfs_trans_brelse(context->tp, bp);
> -				return 0;
> -			}
> -			xfs_trans_brelse(context->tp, bp);
> -		}
> +		error = xfs_attr_node_list_lookup(context, cursor, &bp);
> +		if (error || !bp)
> +			return error;
>  	}
>  	ASSERT(bp != NULL);
>  
> 
> --
> 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

Re: [PATCH 2/3] xfs: abort dir/attr btree operation if btree is obviously weird

[Brian Foster]

On Thu, Oct 26, 2017 at 03:49:36PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> Abort an dir/attr btree operation if the attr btree has obvious problems
> like loops back to the root or pointers don't point down the tree.
> Found by fuzzing btree[0].before to zero in xfs/402, which livelocks on
> the cycle in the attr btree.
> 
> Apply the same checks to xfs_da3_node_lookup_int.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_da_btree.c |   22 +++++++++++++++++++++-
>  fs/xfs/xfs_attr_list.c       |   20 ++++++++++++++++++++
>  2 files changed, 41 insertions(+), 1 deletion(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_da_btree.c b/fs/xfs/libxfs/xfs_da_btree.c
> index 6d43358..6516115 100644
> --- a/fs/xfs/libxfs/xfs_da_btree.c
> +++ b/fs/xfs/libxfs/xfs_da_btree.c
> @@ -1466,6 +1466,7 @@ xfs_da3_node_lookup_int(
>  	int			max;
>  	int			error;
>  	int			retval;
> +	unsigned int		expected_level = 0;
>  	struct xfs_inode	*dp = state->args->dp;
>  
>  	args = state->args;
> @@ -1474,7 +1475,7 @@ xfs_da3_node_lookup_int(
>  	 * Descend thru the B-tree searching each level for the right
>  	 * node to use, until the right hashval is found.
>  	 */
> -	blkno = (args->whichfork == XFS_DATA_FORK)? args->geo->leafblk : 0;
> +	blkno = args->geo->leafblk;
>  	for (blk = &state->path.blk[0], state->path.active = 1;
>  			 state->path.active <= XFS_DA_NODE_MAXDEPTH;
>  			 blk++, state->path.active++) {
> @@ -1517,6 +1518,18 @@ xfs_da3_node_lookup_int(
>  		dp->d_ops->node_hdr_from_disk(&nodehdr, node);
>  		btree = dp->d_ops->node_tree_p(node);
>  
> +		/* Tree taller than we can handle; bail out! */
> +		if (nodehdr.level >= XFS_DA_NODE_MAXDEPTH)
> +			return -EFSCORRUPTED;
> +
> +		/* Check the level from the root. */
> +		if (blkno == args->geo->leafblk)
> +			expected_level = nodehdr.level - 1;
> +		else if (expected_level != nodehdr.level)
> +			return -EFSCORRUPTED;
> +		else
> +			expected_level--;
> +
>  		max = nodehdr.count;
>  		blk->hashval = be32_to_cpu(btree[max - 1].hashval);
>  
> @@ -1562,8 +1575,15 @@ xfs_da3_node_lookup_int(
>  			blk->index = probe;
>  			blkno = be32_to_cpu(btree[probe].before);
>  		}
> +
> +		/* We can't point back to the root. */
> +		if (blkno == args->geo->leafblk)
> +			return -EFSCORRUPTED;
>  	}
>  
> +	if (expected_level != 0)
> +		return -EFSCORRUPTED;
> +
>  	/*
>  	 * A leaf block that ends in the hashval that we are interested in
>  	 * (final hashval == search hashval) means that the next block may
> diff --git a/fs/xfs/xfs_attr_list.c b/fs/xfs/xfs_attr_list.c
> index 021ec5a..a360310 100644
> --- a/fs/xfs/xfs_attr_list.c
> +++ b/fs/xfs/xfs_attr_list.c
> @@ -223,6 +223,7 @@ xfs_attr_node_list_lookup(
>  	struct xfs_buf			*bp;
>  	int				i;
>  	int				error = 0;
> +	unsigned int			expected_level = 0;
>  	uint16_t			magic;
>  
>  	ASSERT(*pbp == NULL);
> @@ -246,6 +247,18 @@ xfs_attr_node_list_lookup(
>  
>  		dp->d_ops->node_hdr_from_disk(&nodehdr, node);
>  
> +		/* Tree taller than we can handle; bail out! */
> +		if (nodehdr.level >= XFS_DA_NODE_MAXDEPTH)
> +			goto out_corruptbuf;
> +
> +		/* Check the level from the root node. */
> +		if (cursor->blkno == 0)
> +			expected_level = nodehdr.level - 1;
> +		else if (expected_level != nodehdr.level)
> +			goto out_corruptbuf;
> +		else
> +			expected_level--;
> +
>  		btree = dp->d_ops->node_tree_p(node);
>  		for (i = 0; i < nodehdr.count; btree++, i++) {
>  			if (cursor->hashval <= be32_to_cpu(btree->hashval)) {
> @@ -259,8 +272,15 @@ xfs_attr_node_list_lookup(
>  
>  		if (i == nodehdr.count)
>  			return 0;
> +
> +		/* We can't point back to the root. */
> +		if (cursor->blkno == 0)
> +			return -EFSCORRUPTED;
>  	}
>  
> +	if (expected_level != 0)
> +		goto out_corruptbuf;
> +
>  	*pbp = bp;
>  	return 0;
>  
> 
> --
> 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

Re: [PATCH 3/3] xfs: compare btree block keys to parent block's keys during scrub

[Brian Foster]

On Thu, Oct 26, 2017 at 03:49:42PM -0700, Darrick J. Wong wrote:
> From: Darrick J. Wong <darrick.wong@oracle.com>
> 
> When we're done checking all the records/keys in a btree block, compute
> the low and high key of the block and compare them to the associated key
> in the parent btree block.
> 
> Signed-off-by: Darrick J. Wong <darrick.wong@oracle.com>
> ---

I haven't gone through all of the latest scrub bits yet, but this looks
sane to me:

Reviewed-by: Brian Foster <bfoster@redhat.com>

>  fs/xfs/libxfs/xfs_btree.c |    4 ++--
>  fs/xfs/libxfs/xfs_btree.h |    4 ++++
>  fs/xfs/scrub/btree.c      |   46 +++++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 52 insertions(+), 2 deletions(-)
> 
> 
> diff --git a/fs/xfs/libxfs/xfs_btree.c b/fs/xfs/libxfs/xfs_btree.c
> index b3cd82a..848f371 100644
> --- a/fs/xfs/libxfs/xfs_btree.c
> +++ b/fs/xfs/libxfs/xfs_btree.c
> @@ -2027,7 +2027,7 @@ xfs_btree_lookup(
>  }
>  
>  /* Find the high key storage area from a regular key. */
> -STATIC union xfs_btree_key *
> +union xfs_btree_key *
>  xfs_btree_high_key_from_key(
>  	struct xfs_btree_cur	*cur,
>  	union xfs_btree_key	*key)
> @@ -2101,7 +2101,7 @@ xfs_btree_get_node_keys(
>  }
>  
>  /* Derive the keys for any btree block. */
> -STATIC void
> +void
>  xfs_btree_get_keys(
>  	struct xfs_btree_cur	*cur,
>  	struct xfs_btree_block	*block,
> diff --git a/fs/xfs/libxfs/xfs_btree.h b/fs/xfs/libxfs/xfs_btree.h
> index be82f41..b57501c 100644
> --- a/fs/xfs/libxfs/xfs_btree.h
> +++ b/fs/xfs/libxfs/xfs_btree.h
> @@ -541,5 +541,9 @@ int64_t xfs_btree_diff_two_ptrs(struct xfs_btree_cur *cur,
>  void xfs_btree_get_sibling(struct xfs_btree_cur *cur,
>  			   struct xfs_btree_block *block,
>  			   union xfs_btree_ptr *ptr, int lr);
> +void xfs_btree_get_keys(struct xfs_btree_cur *cur,
> +		struct xfs_btree_block *block, union xfs_btree_key *key);
> +union xfs_btree_key *xfs_btree_high_key_from_key(struct xfs_btree_cur *cur,
> +		union xfs_btree_key *key);
>  
>  #endif	/* __XFS_BTREE_H__ */
> diff --git a/fs/xfs/scrub/btree.c b/fs/xfs/scrub/btree.c
> index 9ccf763..9e8b67a 100644
> --- a/fs/xfs/scrub/btree.c
> +++ b/fs/xfs/scrub/btree.c
> @@ -358,6 +358,50 @@ xfs_scrub_btree_get_block(
>  }
>  
>  /*
> + * Check that the low and high keys of this block match the keys stored
> + * in the parent block.
> + */
> +STATIC void
> +xfs_scrub_btree_block_keys(
> +	struct xfs_scrub_btree		*bs,
> +	int				level,
> +	struct xfs_btree_block		*block)
> +{
> +	union xfs_btree_key		block_keys;
> +	struct xfs_btree_cur		*cur = bs->cur;
> +	union xfs_btree_key		*high_bk;
> +	union xfs_btree_key		*parent_keys;
> +	union xfs_btree_key		*high_pk;
> +	struct xfs_btree_block		*parent_block;
> +	struct xfs_buf			*bp;
> +
> +	if (level >= cur->bc_nlevels - 1)
> +		return;
> +
> +	/* Calculate the keys for this block. */
> +	xfs_btree_get_keys(cur, block, &block_keys);
> +
> +	/* Obtain the parent's copy of the keys for this block. */
> +	parent_block = xfs_btree_get_block(cur, level + 1, &bp);
> +	parent_keys = xfs_btree_key_addr(cur, cur->bc_ptrs[level + 1],
> +			parent_block);
> +
> +	if (cur->bc_ops->diff_two_keys(cur, &block_keys, parent_keys) != 0)
> +		xfs_scrub_btree_set_corrupt(bs->sc, cur, 1);
> +
> +	if (!(cur->bc_flags & XFS_BTREE_OVERLAPPING))
> +		return;
> +
> +	/* Get high keys */
> +	high_bk = xfs_btree_high_key_from_key(cur, &block_keys);
> +	high_pk = xfs_btree_high_key_addr(cur, cur->bc_ptrs[level + 1],
> +			parent_block);
> +
> +	if (cur->bc_ops->diff_two_keys(cur, high_bk, high_pk) != 0)
> +		xfs_scrub_btree_set_corrupt(bs->sc, cur, 1);
> +}
> +
> +/*
>   * Visit all nodes and leaves of a btree.  Check that all pointers and
>   * records are in order, that the keys reflect the records, and use a callback
>   * so that the caller can verify individual records.
> @@ -418,6 +462,7 @@ xfs_scrub_btree(
>  			/* End of leaf, pop back towards the root. */
>  			if (cur->bc_ptrs[level] >
>  			    be16_to_cpu(block->bb_numrecs)) {
> +				xfs_scrub_btree_block_keys(&bs, level, block);
>  				if (level < cur->bc_nlevels - 1)
>  					cur->bc_ptrs[level + 1]++;
>  				level++;
> @@ -442,6 +487,7 @@ xfs_scrub_btree(
>  
>  		/* End of node, pop back towards the root. */
>  		if (cur->bc_ptrs[level] > be16_to_cpu(block->bb_numrecs)) {
> +			xfs_scrub_btree_block_keys(&bs, level, block);
>  			if (level < cur->bc_nlevels - 1)
>  				cur->bc_ptrs[level + 1]++;
>  			level++;
> 
> --
> 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