[PATCH 0/5] Enhance tree block validation checker

[PATCH 0/5] Enhance tree block validation checker

[Qu Wenruo]

The patchset can be fetched from github:
https://github.com/adam900710/linux/tree/checker_enhance

It's based on David's misc-next branch, with following commit as base:
a5e50b4b444c ("btrfs: Add checker for EXTENT_CSUM")

According to David's suggestion, enhance the output format of tree block
validation checker.

And move them into one separate file: tree-checker.c.

Some example output using btrfsck fsck-test images looks like:

For unagliend file extent member:
---
BTRFS critical (device loop0): corrupt leaf root=1 tree_block=29360128 slot=7 ino=257 file_offset=0: invalid file extent disk_bytenr, have 755944791, should be aligned to 4096
BTRFS error (device loop0): open_ctree failed
---

For bad leaf holes:
---
BTRFS critical (device loop0): corrupt leaf root=1 block=29360128 slot=28: uncontinuous item end, have 9387 expect 15018
BTRFS error (device loop0): open_ctree failed
---


Qu Wenruo (5):
  btrfs-progs: Move leaf and node validation checker to tree-checker.c
  btrfs: tree-checker: Enhance btrfs_check_node output
  btrfs: tree-checker: Enhance output for btrfs_check_leaf
  btrfs: tree-checker: Enhance output for check_csum_item
  btrfs: tree-checker: Enhance output for check_extent_data_item

 fs/btrfs/Makefile       |   2 +-
 fs/btrfs/ctree.h        |   4 +
 fs/btrfs/disk-io.c      | 284 +--------------------------------
 fs/btrfs/tree-checker.c | 417 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 425 insertions(+), 282 deletions(-)
 create mode 100644 fs/btrfs/tree-checker.c

-- 
2.14.1

[PATCH 1/5] btrfs: Move leaf and node validation checker to tree-checker.c

[Qu Wenruo]

It's no doubt the comprehensive tree block checker will become larger
and larger, so move them into their own file is quite reasonable.

Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
 fs/btrfs/Makefile       |   2 +-
 fs/btrfs/ctree.h        |   4 +
 fs/btrfs/disk-io.c      | 284 +-------------------------------------------
 fs/btrfs/tree-checker.c | 309 ++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 317 insertions(+), 282 deletions(-)
 create mode 100644 fs/btrfs/tree-checker.c

diff --git a/fs/btrfs/Makefile b/fs/btrfs/Makefile
index 962a95aefb81..88255e133ade 100644
--- a/fs/btrfs/Makefile
+++ b/fs/btrfs/Makefile
@@ -9,7 +9,7 @@ btrfs-y += super.o ctree.o extent-tree.o print-tree.o root-tree.o dir-item.o \
 	   export.o tree-log.o free-space-cache.o zlib.o lzo.o zstd.o \
 	   compression.o delayed-ref.o relocation.o delayed-inode.o scrub.o \
 	   reada.o backref.o ulist.o qgroup.o send.o dev-replace.o raid56.o \
-	   uuid-tree.o props.o hash.o free-space-tree.o
+	   uuid-tree.o props.o hash.o free-space-tree.o tree-checker.o
 
 btrfs-$(CONFIG_BTRFS_FS_POSIX_ACL) += acl.o
 btrfs-$(CONFIG_BTRFS_FS_CHECK_INTEGRITY) += check-integrity.o
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index ea9c5648ff70..6b7c6fcbc5d5 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3732,4 +3732,8 @@ static inline int btrfs_is_testing(struct btrfs_fs_info *fs_info)
 #endif
 	return 0;
 }
+
+/* Tree block validation checker */
+int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf);
+int btrfs_check_node(struct btrfs_root *root, struct extent_buffer *node);
 #endif
diff --git a/fs/btrfs/disk-io.c b/fs/btrfs/disk-io.c
index c8633f2abdf1..57a9055655d3 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -543,284 +543,6 @@ static int check_tree_block_fsid(struct btrfs_fs_info *fs_info,
 	return ret;
 }
 
-#define CORRUPT(reason, eb, root, slot)					\
-	btrfs_crit(root->fs_info,					\
-		   "corrupt %s, %s: block=%llu, root=%llu, slot=%d",	\
-		   btrfs_header_level(eb) == 0 ? "leaf" : "node",	\
-		   reason, btrfs_header_bytenr(eb), root->objectid, slot)
-
-static int check_extent_data_item(struct btrfs_root *root,
-				  struct extent_buffer *leaf,
-				  struct btrfs_key *key, int slot)
-{
-	struct btrfs_file_extent_item *fi;
-	u32 sectorsize = root->fs_info->sectorsize;
-	u32 item_size = btrfs_item_size_nr(leaf, slot);
-
-	if (!IS_ALIGNED(key->offset, sectorsize)) {
-		CORRUPT("unaligned key offset for file extent",
-			leaf, root, slot);
-		return -EUCLEAN;
-	}
-
-	fi = btrfs_item_ptr(leaf, slot, struct btrfs_file_extent_item);
-
-	if (btrfs_file_extent_type(leaf, fi) > BTRFS_FILE_EXTENT_TYPES) {
-		CORRUPT("invalid file extent type", leaf, root, slot);
-		return -EUCLEAN;
-	}
-
-	/*
-	 * Support for new compression/encrption must introduce incompat flag,
-	 * and must be caught in open_ctree().
-	 */
-	if (btrfs_file_extent_compression(leaf, fi) > BTRFS_COMPRESS_TYPES) {
-		CORRUPT("invalid file extent compression", leaf, root, slot);
-		return -EUCLEAN;
-	}
-	if (btrfs_file_extent_encryption(leaf, fi)) {
-		CORRUPT("invalid file extent encryption", leaf, root, slot);
-		return -EUCLEAN;
-	}
-	if (btrfs_file_extent_type(leaf, fi) == BTRFS_FILE_EXTENT_INLINE) {
-		/* Inline extent must have 0 as key offset */
-		if (key->offset) {
-			CORRUPT("inline extent has non-zero key offset",
-				leaf, root, slot);
-			return -EUCLEAN;
-		}
-
-		/* Compressed inline extent has no on-disk size, skip it */
-		if (btrfs_file_extent_compression(leaf, fi) !=
-		    BTRFS_COMPRESS_NONE)
-			return 0;
-
-		/* Uncompressed inline extent size must match item size */
-		if (item_size != BTRFS_FILE_EXTENT_INLINE_DATA_START +
-		    btrfs_file_extent_ram_bytes(leaf, fi)) {
-			CORRUPT("plaintext inline extent has invalid size",
-				leaf, root, slot);
-			return -EUCLEAN;
-		}
-		return 0;
-	}
-
-	/* Regular or preallocated extent has fixed item size */
-	if (item_size != sizeof(*fi)) {
-		CORRUPT(
-		"regluar or preallocated extent data item size is invalid",
-			leaf, root, slot);
-		return -EUCLEAN;
-	}
-	if (!IS_ALIGNED(btrfs_file_extent_ram_bytes(leaf, fi), sectorsize) ||
-	    !IS_ALIGNED(btrfs_file_extent_disk_bytenr(leaf, fi), sectorsize) ||
-	    !IS_ALIGNED(btrfs_file_extent_disk_num_bytes(leaf, fi), sectorsize) ||
-	    !IS_ALIGNED(btrfs_file_extent_offset(leaf, fi), sectorsize) ||
-	    !IS_ALIGNED(btrfs_file_extent_num_bytes(leaf, fi), sectorsize)) {
-		CORRUPT(
-		"regular or preallocated extent data item has unaligned value",
-			leaf, root, slot);
-		return -EUCLEAN;
-	}
-
-	return 0;
-}
-
-static int check_csum_item(struct btrfs_root *root, struct extent_buffer *leaf,
-			   struct btrfs_key *key, int slot)
-{
-	u32 sectorsize = root->fs_info->sectorsize;
-	u32 csumsize = btrfs_super_csum_size(root->fs_info->super_copy);
-
-	if (key->objectid != BTRFS_EXTENT_CSUM_OBJECTID) {
-		CORRUPT("invalid objectid for csum item", leaf, root, slot);
-		return -EUCLEAN;
-	}
-	if (!IS_ALIGNED(key->offset, sectorsize)) {
-		CORRUPT("unaligned key offset for csum item", leaf, root, slot);
-		return -EUCLEAN;
-	}
-	if (!IS_ALIGNED(btrfs_item_size_nr(leaf, slot), csumsize)) {
-		CORRUPT("unaligned csum item size", leaf, root, slot);
-		return -EUCLEAN;
-	}
-	return 0;
-}
-
-/*
- * Common point to switch the item-specific validation.
- */
-static int check_leaf_item(struct btrfs_root *root,
-			   struct extent_buffer *leaf,
-			   struct btrfs_key *key, int slot)
-{
-	int ret = 0;
-
-	switch (key->type) {
-	case BTRFS_EXTENT_DATA_KEY:
-		ret = check_extent_data_item(root, leaf, key, slot);
-		break;
-	case BTRFS_EXTENT_CSUM_KEY:
-		ret = check_csum_item(root, leaf, key, slot);
-		break;
-	}
-	return ret;
-}
-
-static noinline int check_leaf(struct btrfs_root *root,
-			       struct extent_buffer *leaf)
-{
-	struct btrfs_fs_info *fs_info = root->fs_info;
-	/* No valid key type is 0, so all key should be larger than this key */
-	struct btrfs_key prev_key = {0, 0, 0};
-	struct btrfs_key key;
-	u32 nritems = btrfs_header_nritems(leaf);
-	int slot;
-
-	/*
-	 * Extent buffers from a relocation tree have a owner field that
-	 * corresponds to the subvolume tree they are based on. So just from an
-	 * extent buffer alone we can not find out what is the id of the
-	 * corresponding subvolume tree, so we can not figure out if the extent
-	 * buffer corresponds to the root of the relocation tree or not. So skip
-	 * this check for relocation trees.
-	 */
-	if (nritems == 0 && !btrfs_header_flag(leaf, BTRFS_HEADER_FLAG_RELOC)) {
-		struct btrfs_root *check_root;
-
-		key.objectid = btrfs_header_owner(leaf);
-		key.type = BTRFS_ROOT_ITEM_KEY;
-		key.offset = (u64)-1;
-
-		check_root = btrfs_get_fs_root(fs_info, &key, false);
-		/*
-		 * The only reason we also check NULL here is that during
-		 * open_ctree() some roots has not yet been set up.
-		 */
-		if (!IS_ERR_OR_NULL(check_root)) {
-			struct extent_buffer *eb;
-
-			eb = btrfs_root_node(check_root);
-			/* if leaf is the root, then it's fine */
-			if (leaf != eb) {
-				CORRUPT("non-root leaf's nritems is 0",
-					leaf, check_root, 0);
-				free_extent_buffer(eb);
-				return -EUCLEAN;
-			}
-			free_extent_buffer(eb);
-		}
-		return 0;
-	}
-
-	if (nritems == 0)
-		return 0;
-
-	/*
-	 * Check the following things to make sure this is a good leaf, and
-	 * leaf users won't need to bother with similar sanity checks:
-	 *
-	 * 1) key order
-	 * 2) item offset and size
-	 *    No overlap, no hole, all inside the leaf.
-	 * 3) item content
-	 *    If possible, do comprehensive sanity check.
-	 *    NOTE: All checks must only rely on the item data itself.
-	 */
-	for (slot = 0; slot < nritems; slot++) {
-		u32 item_end_expected;
-		int ret;
-
-		btrfs_item_key_to_cpu(leaf, &key, slot);
-
-		/* Make sure the keys are in the right order */
-		if (btrfs_comp_cpu_keys(&prev_key, &key) >= 0) {
-			CORRUPT("bad key order", leaf, root, slot);
-			return -EUCLEAN;
-		}
-
-		/*
-		 * Make sure the offset and ends are right, remember that the
-		 * item data starts at the end of the leaf and grows towards the
-		 * front.
-		 */
-		if (slot == 0)
-			item_end_expected = BTRFS_LEAF_DATA_SIZE(fs_info);
-		else
-			item_end_expected = btrfs_item_offset_nr(leaf,
-								 slot - 1);
-		if (btrfs_item_end_nr(leaf, slot) != item_end_expected) {
-			CORRUPT("slot offset bad", leaf, root, slot);
-			return -EUCLEAN;
-		}
-
-		/*
-		 * Check to make sure that we don't point outside of the leaf,
-		 * just in case all the items are consistent to each other, but
-		 * all point outside of the leaf.
-		 */
-		if (btrfs_item_end_nr(leaf, slot) >
-		    BTRFS_LEAF_DATA_SIZE(fs_info)) {
-			CORRUPT("slot end outside of leaf", leaf, root, slot);
-			return -EUCLEAN;
-		}
-
-		/* Also check if the item pointer overlaps with btrfs item. */
-		if (btrfs_item_nr_offset(slot) + sizeof(struct btrfs_item) >
-		    btrfs_item_ptr_offset(leaf, slot)) {
-			CORRUPT("slot overlap with its data", leaf, root, slot);
-			return -EUCLEAN;
-		}
-
-		/* Check if the item size and content meet other criteria */
-		ret = check_leaf_item(root, leaf, &key, slot);
-		if (ret < 0)
-			return ret;
-
-		prev_key.objectid = key.objectid;
-		prev_key.type = key.type;
-		prev_key.offset = key.offset;
-	}
-
-	return 0;
-}
-
-static int check_node(struct btrfs_root *root, struct extent_buffer *node)
-{
-	unsigned long nr = btrfs_header_nritems(node);
-	struct btrfs_key key, next_key;
-	int slot;
-	u64 bytenr;
-	int ret = 0;
-
-	if (nr == 0 || nr > BTRFS_NODEPTRS_PER_BLOCK(root->fs_info)) {
-		btrfs_crit(root->fs_info,
-			   "corrupt node: block %llu root %llu nritems %lu",
-			   node->start, root->objectid, nr);
-		return -EIO;
-	}
-
-	for (slot = 0; slot < nr - 1; slot++) {
-		bytenr = btrfs_node_blockptr(node, slot);
-		btrfs_node_key_to_cpu(node, &key, slot);
-		btrfs_node_key_to_cpu(node, &next_key, slot + 1);
-
-		if (!bytenr) {
-			CORRUPT("invalid item slot", node, root, slot);
-			ret = -EIO;
-			goto out;
-		}
-
-		if (btrfs_comp_cpu_keys(&key, &next_key) >= 0) {
-			CORRUPT("bad key order", node, root, slot);
-			ret = -EIO;
-			goto out;
-		}
-	}
-out:
-	return ret;
-}
-
 static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
 				      u64 phy_offset, struct page *page,
 				      u64 start, u64 end, int mirror)
@@ -886,12 +608,12 @@ static int btree_readpage_end_io_hook(struct btrfs_io_bio *io_bio,
 	 * that we don't try and read the other copies of this block, just
 	 * return -EIO.
 	 */
-	if (found_level == 0 && check_leaf(root, eb)) {
+	if (found_level == 0 && btrfs_check_leaf(root, eb)) {
 		set_bit(EXTENT_BUFFER_CORRUPT, &eb->bflags);
 		ret = -EIO;
 	}
 
-	if (found_level > 0 && check_node(root, eb))
+	if (found_level > 0 && btrfs_check_node(root, eb))
 		ret = -EIO;
 
 	if (!ret)
@@ -4146,7 +3868,7 @@ void btrfs_mark_buffer_dirty(struct extent_buffer *buf)
 					 buf->len,
 					 fs_info->dirty_metadata_batch);
 #ifdef CONFIG_BTRFS_FS_CHECK_INTEGRITY
-	if (btrfs_header_level(buf) == 0 && check_leaf(root, buf)) {
+	if (btrfs_header_level(buf) == 0 && btrfs_check_leaf(root, buf)) {
 		btrfs_print_leaf(buf);
 		ASSERT(0);
 	}
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
new file mode 100644
index 000000000000..301243a69dea
--- /dev/null
+++ b/fs/btrfs/tree-checker.c
@@ -0,0 +1,309 @@
+/*
+ * Copyright (C) Qu Wenruo 2017.  All rights reserved.
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public
+ * License along with this program.
+ */
+
+/*
+ * The module is used to catch unexpected/corrupted tree block data.
+ * Such unexpected/corrupted behavior can either be caused by fuzzed image
+ * or undefined behavior change.
+ *
+ * The objective is to do leaf/node validation check when tree block is read
+ * from disk, and check *EVERY* possible member, so other kernel part won't
+ * ever need to bother checking them again elsewhere.
+ *
+ * Due to the importance, every checker should be fully reviewed or it can
+ * easily cause a valid image unable to mount.
+ */
+
+#include "ctree.h"
+#include "disk-io.h"
+#include "compression.h"
+
+#define CORRUPT(reason, eb, root, slot)					\
+	btrfs_crit(root->fs_info,					\
+		   "corrupt %s, %s: block=%llu, root=%llu, slot=%d",	\
+		   btrfs_header_level(eb) == 0 ? "leaf" : "node",	\
+		   reason, btrfs_header_bytenr(eb), root->objectid, slot)
+
+static int check_extent_data_item(struct btrfs_root *root,
+				  struct extent_buffer *leaf,
+				  struct btrfs_key *key, int slot)
+{
+	struct btrfs_file_extent_item *fi;
+	u32 sectorsize = root->fs_info->sectorsize;
+	u32 item_size = btrfs_item_size_nr(leaf, slot);
+
+	if (!IS_ALIGNED(key->offset, sectorsize)) {
+		CORRUPT("unaligned key offset for file extent",
+			leaf, root, slot);
+		return -EUCLEAN;
+	}
+
+	fi = btrfs_item_ptr(leaf, slot, struct btrfs_file_extent_item);
+
+	if (btrfs_file_extent_type(leaf, fi) > BTRFS_FILE_EXTENT_TYPES) {
+		CORRUPT("invalid file extent type", leaf, root, slot);
+		return -EUCLEAN;
+	}
+
+	/*
+	 * Support for new compression/encrption must introduce incompat flag,
+	 * and must be caught in open_ctree().
+	 */
+	if (btrfs_file_extent_compression(leaf, fi) > BTRFS_COMPRESS_TYPES) {
+		CORRUPT("invalid file extent compression", leaf, root, slot);
+		return -EUCLEAN;
+	}
+	if (btrfs_file_extent_encryption(leaf, fi)) {
+		CORRUPT("invalid file extent encryption", leaf, root, slot);
+		return -EUCLEAN;
+	}
+	if (btrfs_file_extent_type(leaf, fi) == BTRFS_FILE_EXTENT_INLINE) {
+		/* Inline extent must have 0 as key offset */
+		if (key->offset) {
+			CORRUPT("inline extent has non-zero key offset",
+				leaf, root, slot);
+			return -EUCLEAN;
+		}
+
+		/* Compressed inline extent has no on-disk size, skip it */
+		if (btrfs_file_extent_compression(leaf, fi) !=
+		    BTRFS_COMPRESS_NONE)
+			return 0;
+
+		/* Uncompressed inline extent size must match item size */
+		if (item_size != BTRFS_FILE_EXTENT_INLINE_DATA_START +
+		    btrfs_file_extent_ram_bytes(leaf, fi)) {
+			CORRUPT("plaintext inline extent has invalid size",
+				leaf, root, slot);
+			return -EUCLEAN;
+		}
+		return 0;
+	}
+
+	/* Regular or preallocated extent has fixed item size */
+	if (item_size != sizeof(*fi)) {
+		CORRUPT(
+		"regluar or preallocated extent data item size is invalid",
+			leaf, root, slot);
+		return -EUCLEAN;
+	}
+	if (!IS_ALIGNED(btrfs_file_extent_ram_bytes(leaf, fi), sectorsize) ||
+	    !IS_ALIGNED(btrfs_file_extent_disk_bytenr(leaf, fi), sectorsize) ||
+	    !IS_ALIGNED(btrfs_file_extent_disk_num_bytes(leaf, fi), sectorsize) ||
+	    !IS_ALIGNED(btrfs_file_extent_offset(leaf, fi), sectorsize) ||
+	    !IS_ALIGNED(btrfs_file_extent_num_bytes(leaf, fi), sectorsize)) {
+		CORRUPT(
+		"regular or preallocated extent data item has unaligned value",
+			leaf, root, slot);
+		return -EUCLEAN;
+	}
+
+	return 0;
+}
+
+static int check_csum_item(struct btrfs_root *root, struct extent_buffer *leaf,
+			   struct btrfs_key *key, int slot)
+{
+	u32 sectorsize = root->fs_info->sectorsize;
+	u32 csumsize = btrfs_super_csum_size(root->fs_info->super_copy);
+
+	if (key->objectid != BTRFS_EXTENT_CSUM_OBJECTID) {
+		CORRUPT("invalid objectid for csum item", leaf, root, slot);
+		return -EUCLEAN;
+	}
+	if (!IS_ALIGNED(key->offset, sectorsize)) {
+		CORRUPT("unaligned key offset for csum item", leaf, root, slot);
+		return -EUCLEAN;
+	}
+	if (!IS_ALIGNED(btrfs_item_size_nr(leaf, slot), csumsize)) {
+		CORRUPT("unaligned csum item size", leaf, root, slot);
+		return -EUCLEAN;
+	}
+	return 0;
+}
+
+/*
+ * Common point to switch the item-specific validation.
+ */
+static int check_leaf_item(struct btrfs_root *root,
+			   struct extent_buffer *leaf,
+			   struct btrfs_key *key, int slot)
+{
+	int ret = 0;
+
+	switch (key->type) {
+	case BTRFS_EXTENT_DATA_KEY:
+		ret = check_extent_data_item(root, leaf, key, slot);
+		break;
+	case BTRFS_EXTENT_CSUM_KEY:
+		ret = check_csum_item(root, leaf, key, slot);
+		break;
+	}
+	return ret;
+}
+
+int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
+{
+	struct btrfs_fs_info *fs_info = root->fs_info;
+	/* No valid key type is 0, so all key should be larger than this key */
+	struct btrfs_key prev_key = {0, 0, 0};
+	struct btrfs_key key;
+	u32 nritems = btrfs_header_nritems(leaf);
+	int slot;
+
+	/*
+	 * Extent buffers from a relocation tree have a owner field that
+	 * corresponds to the subvolume tree they are based on. So just from an
+	 * extent buffer alone we can not find out what is the id of the
+	 * corresponding subvolume tree, so we can not figure out if the extent
+	 * buffer corresponds to the root of the relocation tree or not. So skip
+	 * this check for relocation trees.
+	 */
+	if (nritems == 0 && !btrfs_header_flag(leaf, BTRFS_HEADER_FLAG_RELOC)) {
+		struct btrfs_root *check_root;
+
+		key.objectid = btrfs_header_owner(leaf);
+		key.type = BTRFS_ROOT_ITEM_KEY;
+		key.offset = (u64)-1;
+
+		check_root = btrfs_get_fs_root(fs_info, &key, false);
+		/*
+		 * The only reason we also check NULL here is that during
+		 * open_ctree() some roots has not yet been set up.
+		 */
+		if (!IS_ERR_OR_NULL(check_root)) {
+			struct extent_buffer *eb;
+
+			eb = btrfs_root_node(check_root);
+			/* if leaf is the root, then it's fine */
+			if (leaf != eb) {
+				CORRUPT("non-root leaf's nritems is 0",
+					leaf, check_root, 0);
+				free_extent_buffer(eb);
+				return -EUCLEAN;
+			}
+			free_extent_buffer(eb);
+		}
+		return 0;
+	}
+
+	if (nritems == 0)
+		return 0;
+
+	/*
+	 * Check the following things to make sure this is a good leaf, and
+	 * leaf users won't need to bother with similar sanity checks:
+	 *
+	 * 1) key order
+	 * 2) item offset and size
+	 *    No overlap, no hole, all inside the leaf.
+	 * 3) item content
+	 *    If possible, do comprehensive sanity check.
+	 *    NOTE: All checks must only rely on the item data itself.
+	 */
+	for (slot = 0; slot < nritems; slot++) {
+		u32 item_end_expected;
+		int ret;
+
+		btrfs_item_key_to_cpu(leaf, &key, slot);
+
+		/* Make sure the keys are in the right order */
+		if (btrfs_comp_cpu_keys(&prev_key, &key) >= 0) {
+			CORRUPT("bad key order", leaf, root, slot);
+			return -EUCLEAN;
+		}
+
+		/*
+		 * Make sure the offset and ends are right, remember that the
+		 * item data starts at the end of the leaf and grows towards the
+		 * front.
+		 */
+		if (slot == 0)
+			item_end_expected = BTRFS_LEAF_DATA_SIZE(fs_info);
+		else
+			item_end_expected = btrfs_item_offset_nr(leaf,
+								 slot - 1);
+		if (btrfs_item_end_nr(leaf, slot) != item_end_expected) {
+			CORRUPT("slot offset bad", leaf, root, slot);
+			return -EUCLEAN;
+		}
+
+		/*
+		 * Check to make sure that we don't point outside of the leaf,
+		 * just in case all the items are consistent to each other, but
+		 * all point outside of the leaf.
+		 */
+		if (btrfs_item_end_nr(leaf, slot) >
+		    BTRFS_LEAF_DATA_SIZE(fs_info)) {
+			CORRUPT("slot end outside of leaf", leaf, root, slot);
+			return -EUCLEAN;
+		}
+
+		/* Also check if the item pointer overlaps with btrfs item. */
+		if (btrfs_item_nr_offset(slot) + sizeof(struct btrfs_item) >
+		    btrfs_item_ptr_offset(leaf, slot)) {
+			CORRUPT("slot overlap with its data", leaf, root, slot);
+			return -EUCLEAN;
+		}
+
+		/* Check if the item size and content meet other criteria */
+		ret = check_leaf_item(root, leaf, &key, slot);
+		if (ret < 0)
+			return ret;
+
+		prev_key.objectid = key.objectid;
+		prev_key.type = key.type;
+		prev_key.offset = key.offset;
+	}
+
+	return 0;
+}
+
+int btrfs_check_node(struct btrfs_root *root, struct extent_buffer *node)
+{
+	unsigned long nr = btrfs_header_nritems(node);
+	struct btrfs_key key, next_key;
+	int slot;
+	u64 bytenr;
+	int ret = 0;
+
+	if (nr == 0 || nr > BTRFS_NODEPTRS_PER_BLOCK(root->fs_info)) {
+		btrfs_crit(root->fs_info,
+			   "corrupt node: block %llu root %llu nritems %lu",
+			   node->start, root->objectid, nr);
+		return -EIO;
+	}
+
+	for (slot = 0; slot < nr - 1; slot++) {
+		bytenr = btrfs_node_blockptr(node, slot);
+		btrfs_node_key_to_cpu(node, &key, slot);
+		btrfs_node_key_to_cpu(node, &next_key, slot + 1);
+
+		if (!bytenr) {
+			CORRUPT("invalid item slot", node, root, slot);
+			ret = -EIO;
+			goto out;
+		}
+
+		if (btrfs_comp_cpu_keys(&key, &next_key) >= 0) {
+			CORRUPT("bad key order", node, root, slot);
+			ret = -EIO;
+			goto out;
+		}
+	}
+out:
+	return ret;
+}
-- 
2.14.1

[PATCH 2/5] btrfs: tree-checker: Enhance btrfs_check_node output

[Qu Wenruo]

Use inline function to replace macro since we don't need
stringification.
(Macro still exist until all caller get updated)

And add more info about the error.

For nr_items error, report if it's too large or too small, and output
valid value range.

For blk pointer, added a new alignment checker.

For key order, also output the next key to make the problem more
obvious.

Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
 fs/btrfs/tree-checker.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
 1 file changed, 44 insertions(+), 4 deletions(-)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 301243a69dea..4b615beb0ca9 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -37,6 +37,32 @@
 		   btrfs_header_level(eb) == 0 ? "leaf" : "node",	\
 		   reason, btrfs_header_bytenr(eb), root->objectid, slot)
 
+/*
+ * Append the generic "corrupt leaf/node root=%llu block=%llu slot=%d: " to
+ * @fmt.
+ * Allowing user to customize their output.
+ */
+__printf(4, 5)
+static void generic_err(const struct btrfs_root *root,
+			const struct extent_buffer *eb,
+			int slot, const char *fmt, ...)
+{
+	struct va_format vaf;
+	va_list args;
+
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	btrfs_crit(root->fs_info,
+		"corrupt %s root=%llu block=%llu slot=%d: %pV",
+		btrfs_header_level(eb) == 0 ? "leaf" : "node",
+		root->objectid, btrfs_header_bytenr(eb), slot,
+		&vaf);
+	va_end(args);
+}
+
 static int check_extent_data_item(struct btrfs_root *root,
 				  struct extent_buffer *leaf,
 				  struct btrfs_key *key, int slot)
@@ -282,8 +308,10 @@ int btrfs_check_node(struct btrfs_root *root, struct extent_buffer *node)
 
 	if (nr == 0 || nr > BTRFS_NODEPTRS_PER_BLOCK(root->fs_info)) {
 		btrfs_crit(root->fs_info,
-			   "corrupt node: block %llu root %llu nritems %lu",
-			   node->start, root->objectid, nr);
+			"corrupt node: root=%llu block=%llu nritems too %s, have %lu, range [1,%u]",
+			   root->objectid, node->start,
+			   nr == 0 ? "small" : "large", nr,
+			   BTRFS_NODEPTRS_PER_BLOCK(root->fs_info));
 		return -EIO;
 	}
 
@@ -293,13 +321,25 @@ int btrfs_check_node(struct btrfs_root *root, struct extent_buffer *node)
 		btrfs_node_key_to_cpu(node, &next_key, slot + 1);
 
 		if (!bytenr) {
-			CORRUPT("invalid item slot", node, root, slot);
+			generic_err(root, node, slot,
+			"invalid node pointer, shouldn't point to zero");
 			ret = -EIO;
 			goto out;
 		}
+		if (!IS_ALIGNED(bytenr, root->fs_info->sectorsize)) {
+			generic_err(root, node, slot,
+			"unaligned pointer, have %llu, should be aligned to %u",
+				     bytenr, root->fs_info->sectorsize);
+			ret = -EUCLEAN;
+			goto out;
+		}
 
 		if (btrfs_comp_cpu_keys(&key, &next_key) >= 0) {
-			CORRUPT("bad key order", node, root, slot);
+			generic_err(root, node, slot,
+			"bad key order, current key (%llu %u %llu) next key (%llu %u %llu)",
+				     key.objectid, key.type, key.offset,
+				     next_key.objectid, next_key.type,
+				     next_key.offset);
 			ret = -EIO;
 			goto out;
 		}
-- 
2.14.1

[PATCH 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf

[Qu Wenruo]

Enhance the output to print:
1) Reason
2) Bad value
   If reason can't explain enough
3) Good value (range)

Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
 fs/btrfs/tree-checker.c | 26 ++++++++++++++++++++------
 1 file changed, 20 insertions(+), 6 deletions(-)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 4b615beb0ca9..4f847955fdc6 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -216,8 +216,8 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
 			eb = btrfs_root_node(check_root);
 			/* if leaf is the root, then it's fine */
 			if (leaf != eb) {
-				CORRUPT("non-root leaf's nritems is 0",
-					leaf, check_root, 0);
+				generic_err(check_root, leaf, 0,
+					    "non-root leaf's nritems is 0");
 				free_extent_buffer(eb);
 				return -EUCLEAN;
 			}
@@ -248,7 +248,11 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
 
 		/* Make sure the keys are in the right order */
 		if (btrfs_comp_cpu_keys(&prev_key, &key) >= 0) {
-			CORRUPT("bad key order", leaf, root, slot);
+			generic_err(root, leaf, slot,
+				"bad key order, prev key (%llu %u %llu) current key (%llu %u %llu)",
+				prev_key.objectid, prev_key.type,
+				prev_key.offset, key.objectid, key.type,
+				key.offset);
 			return -EUCLEAN;
 		}
 
@@ -263,7 +267,10 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
 			item_end_expected = btrfs_item_offset_nr(leaf,
 								 slot - 1);
 		if (btrfs_item_end_nr(leaf, slot) != item_end_expected) {
-			CORRUPT("slot offset bad", leaf, root, slot);
+			generic_err(root, leaf, slot,
+				"uncontinuous item end, have %u expect %u",
+				btrfs_item_end_nr(leaf, slot),
+				item_end_expected);
 			return -EUCLEAN;
 		}
 
@@ -274,14 +281,21 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
 		 */
 		if (btrfs_item_end_nr(leaf, slot) >
 		    BTRFS_LEAF_DATA_SIZE(fs_info)) {
-			CORRUPT("slot end outside of leaf", leaf, root, slot);
+			generic_err(root, leaf, slot,
+				"slot end outside of leaf, have %u expect range [0, %u]",
+				btrfs_item_end_nr(leaf, slot),
+				BTRFS_LEAF_DATA_SIZE(fs_info));
 			return -EUCLEAN;
 		}
 
 		/* Also check if the item pointer overlaps with btrfs item. */
 		if (btrfs_item_nr_offset(slot) + sizeof(struct btrfs_item) >
 		    btrfs_item_ptr_offset(leaf, slot)) {
-			CORRUPT("slot overlap with its data", leaf, root, slot);
+			generic_err(root, leaf, slot,
+				"slot overlap with its data, item end %lu data start %lu",
+				btrfs_item_nr_offset(slot) +
+				sizeof(struct btrfs_item),
+				btrfs_item_ptr_offset(leaf, slot));
 			return -EUCLEAN;
 		}
 
-- 
2.14.1

[PATCH 4/5] btrfs: tree-checker: Enhance output for check_csum_item

[Qu Wenruo]

Output the bad value and expected good value (or its alignment).

Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
 fs/btrfs/tree-checker.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 4f847955fdc6..52e9ab8c2a79 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -147,15 +147,21 @@ static int check_csum_item(struct btrfs_root *root, struct extent_buffer *leaf,
 	u32 csumsize = btrfs_super_csum_size(root->fs_info->super_copy);
 
 	if (key->objectid != BTRFS_EXTENT_CSUM_OBJECTID) {
-		CORRUPT("invalid objectid for csum item", leaf, root, slot);
+		generic_err(root, leaf, slot,
+			"invalid objectid for CSUM_ITEM, have %llu expect %llu",
+			key->objectid, BTRFS_EXTENT_CSUM_OBJECTID);
 		return -EUCLEAN;
 	}
 	if (!IS_ALIGNED(key->offset, sectorsize)) {
-		CORRUPT("unaligned key offset for csum item", leaf, root, slot);
+		generic_err(root, leaf, slot,
+			"unaligned key offset for CSUM_ITEM, have %llu should be aligned to %u",
+			key->offset, sectorsize);
 		return -EUCLEAN;
 	}
 	if (!IS_ALIGNED(btrfs_item_size_nr(leaf, slot), csumsize)) {
-		CORRUPT("unaligned csum item size", leaf, root, slot);
+		generic_err(root, leaf, slot,
+			"unaligned item size for CSUM_ITEM, have %u should be aligned to %u",
+			btrfs_item_size_nr(leaf, slot), csumsize);
 		return -EUCLEAN;
 	}
 	return 0;
-- 
2.14.1

[PATCH 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item

[Qu Wenruo]

Output the invalid member name and its bad value, along with its
expected value range or alignment.

Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
 fs/btrfs/tree-checker.c | 92 +++++++++++++++++++++++++++++++++++++------------
 1 file changed, 70 insertions(+), 22 deletions(-)

diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 52e9ab8c2a79..1324fcae93c0 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -63,6 +63,47 @@ static void generic_err(const struct btrfs_root *root,
 	va_end(args);
 }
 
+/*
+ * Customized reporter for extent data item, since its key objectid and
+ * offset has its own meaning.
+ */
+__printf(4, 5)
+static void file_extent_err(const struct btrfs_root *root,
+			    const struct extent_buffer *eb,
+			    int slot, const char *fmt, ...)
+{
+	struct btrfs_key key;
+	struct va_format vaf;
+	va_list args;
+
+	btrfs_item_key_to_cpu(eb, &key, slot);
+	va_start(args, fmt);
+
+	vaf.fmt = fmt;
+	vaf.va = &args;
+
+	btrfs_crit(root->fs_info,
+		"corrupt %s root=%llu tree_block=%llu slot=%d ino=%llu file_offset=%llu: %pV",
+		btrfs_header_level(eb) == 0 ? "leaf" : "node",
+		root->objectid, btrfs_header_bytenr(eb), slot,
+		key.objectid, key.offset, &vaf);
+	va_end(args);
+}
+
+/*
+ * Return 0 if the btrfs_file_extent_##name is aligned to @align
+ * Else return 1
+ */
+#define CHECK_FI_ALIGN(root, leaf, slot, fi, name, align)		\
+({									\
+	if (!IS_ALIGNED(btrfs_file_extent_##name(leaf, fi), align))	\
+		file_extent_err(root, leaf, slot,			\
+			"invalid file extent %s, have %llu, should be aligned to %u",\
+			#name, btrfs_file_extent_##name(leaf, fi),	\
+			align);						\
+	(!IS_ALIGNED(btrfs_file_extent_##name(leaf, fi), align));	\
+})
+
 static int check_extent_data_item(struct btrfs_root *root,
 				  struct extent_buffer *leaf,
 				  struct btrfs_key *key, int slot)
@@ -72,15 +113,19 @@ static int check_extent_data_item(struct btrfs_root *root,
 	u32 item_size = btrfs_item_size_nr(leaf, slot);
 
 	if (!IS_ALIGNED(key->offset, sectorsize)) {
-		CORRUPT("unaligned key offset for file extent",
-			leaf, root, slot);
+		file_extent_err(root, leaf, slot,
+			"unaligned key offset, have %llu should be aligned to %u",
+			key->offset, sectorsize);
 		return -EUCLEAN;
 	}
 
 	fi = btrfs_item_ptr(leaf, slot, struct btrfs_file_extent_item);
 
 	if (btrfs_file_extent_type(leaf, fi) > BTRFS_FILE_EXTENT_TYPES) {
-		CORRUPT("invalid file extent type", leaf, root, slot);
+		file_extent_err(root, leaf, slot,
+			"invalid file extent type, have %u expect range [0, %u]",
+			btrfs_file_extent_type(leaf, fi),
+			BTRFS_FILE_EXTENT_TYPES);
 		return -EUCLEAN;
 	}
 
@@ -89,18 +134,24 @@ static int check_extent_data_item(struct btrfs_root *root,
 	 * and must be caught in open_ctree().
 	 */
 	if (btrfs_file_extent_compression(leaf, fi) > BTRFS_COMPRESS_TYPES) {
-		CORRUPT("invalid file extent compression", leaf, root, slot);
+		file_extent_err(root, leaf, slot,
+			"invalid file extent compression, have %u expect range [0, %u]",
+			btrfs_file_extent_compression(leaf, fi),
+			BTRFS_COMPRESS_TYPES);
 		return -EUCLEAN;
 	}
 	if (btrfs_file_extent_encryption(leaf, fi)) {
-		CORRUPT("invalid file extent encryption", leaf, root, slot);
+		file_extent_err(root, leaf, slot,
+			"invalid file extent encryption, have %u expect 0",
+			btrfs_file_extent_encryption(leaf, fi));
 		return -EUCLEAN;
 	}
 	if (btrfs_file_extent_type(leaf, fi) == BTRFS_FILE_EXTENT_INLINE) {
 		/* Inline extent must have 0 as key offset */
 		if (key->offset) {
-			CORRUPT("inline extent has non-zero key offset",
-				leaf, root, slot);
+			file_extent_err(root, leaf, slot,
+				"invalid offset for inline extent, have %llu expect 0",
+				key->offset);
 			return -EUCLEAN;
 		}
 
@@ -112,8 +163,10 @@ static int check_extent_data_item(struct btrfs_root *root,
 		/* Uncompressed inline extent size must match item size */
 		if (item_size != BTRFS_FILE_EXTENT_INLINE_DATA_START +
 		    btrfs_file_extent_ram_bytes(leaf, fi)) {
-			CORRUPT("plaintext inline extent has invalid size",
-				leaf, root, slot);
+			file_extent_err(root, leaf, slot,
+				"invalid uncompressed inline extent size, have %u expect %llu",
+				item_size, BTRFS_FILE_EXTENT_INLINE_DATA_START +
+				btrfs_file_extent_ram_bytes(leaf, fi));
 			return -EUCLEAN;
 		}
 		return 0;
@@ -121,22 +174,17 @@ static int check_extent_data_item(struct btrfs_root *root,
 
 	/* Regular or preallocated extent has fixed item size */
 	if (item_size != sizeof(*fi)) {
-		CORRUPT(
-		"regluar or preallocated extent data item size is invalid",
-			leaf, root, slot);
+		file_extent_err(root, leaf, slot,
+			"invalid extent data item size for reg/prealloc, have %u expect %lu",
+			item_size, sizeof(*fi));
 		return -EUCLEAN;
 	}
-	if (!IS_ALIGNED(btrfs_file_extent_ram_bytes(leaf, fi), sectorsize) ||
-	    !IS_ALIGNED(btrfs_file_extent_disk_bytenr(leaf, fi), sectorsize) ||
-	    !IS_ALIGNED(btrfs_file_extent_disk_num_bytes(leaf, fi), sectorsize) ||
-	    !IS_ALIGNED(btrfs_file_extent_offset(leaf, fi), sectorsize) ||
-	    !IS_ALIGNED(btrfs_file_extent_num_bytes(leaf, fi), sectorsize)) {
-		CORRUPT(
-		"regular or preallocated extent data item has unaligned value",
-			leaf, root, slot);
+	if (CHECK_FI_ALIGN(root, leaf, slot, fi, ram_bytes, sectorsize) ||
+	    CHECK_FI_ALIGN(root, leaf, slot, fi, disk_bytenr, sectorsize) ||
+	    CHECK_FI_ALIGN(root, leaf, slot, fi, disk_num_bytes, sectorsize) ||
+	    CHECK_FI_ALIGN(root, leaf, slot, fi, offset, sectorsize) ||
+	    CHECK_FI_ALIGN(root, leaf, slot, fi, num_bytes, sectorsize))
 		return -EUCLEAN;
-	}
-
 	return 0;
 }
 
-- 
2.14.1

Re: [PATCH 2/5] btrfs: tree-checker: Enhance btrfs_check_node output

[Nikolay Borisov]

On 28.09.2017 06:36, Qu Wenruo wrote:
> Use inline function to replace macro since we don't need
> stringification.
> (Macro still exist until all caller get updated)
> 
> And add more info about the error.
> 
> For nr_items error, report if it's too large or too small, and output
> valid value range.
> 
> For blk pointer, added a new alignment checker.
> 
> For key order, also output the next key to make the problem more
> obvious.
> 
> Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
> ---
>  fs/btrfs/tree-checker.c | 48 ++++++++++++++++++++++++++++++++++++++++++++----
>  1 file changed, 44 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 301243a69dea..4b615beb0ca9 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -37,6 +37,32 @@
>  		   btrfs_header_level(eb) == 0 ? "leaf" : "node",	\
>  		   reason, btrfs_header_bytenr(eb), root->objectid, slot)
>  
> +/*
> + * Append the generic "corrupt leaf/node root=%llu block=%llu slot=%d: " to
> + * @fmt.
> + * Allowing user to customize their output.
> + */
> +__printf(4, 5)
> +static void generic_err(const struct btrfs_root *root,
> +			const struct extent_buffer *eb,
> +			int slot, const char *fmt, ...)
> +{
> +	struct va_format vaf;
> +	va_list args;
> +
> +	va_start(args, fmt);
> +
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +
> +	btrfs_crit(root->fs_info,
> +		"corrupt %s root=%llu block=%llu slot=%d: %pV",
> +		btrfs_header_level(eb) == 0 ? "leaf" : "node",
> +		root->objectid, btrfs_header_bytenr(eb), slot,
> +		&vaf);
> +	va_end(args);
> +}
> +
>  static int check_extent_data_item(struct btrfs_root *root,
>  				  struct extent_buffer *leaf,
>  				  struct btrfs_key *key, int slot)
> @@ -282,8 +308,10 @@ int btrfs_check_node(struct btrfs_root *root, struct extent_buffer *node)
>  
>  	if (nr == 0 || nr > BTRFS_NODEPTRS_PER_BLOCK(root->fs_info)) {
>  		btrfs_crit(root->fs_info,
> -			   "corrupt node: block %llu root %llu nritems %lu",
> -			   node->start, root->objectid, nr);
> +			"corrupt node: root=%llu block=%llu nritems too %s, have %lu, range [1,%u]",

It's a nit but I'd rather have consistency between formatting of error
messages. Here you begin the line with "corrupt node:" whereas when
using generic_err you won't have the ':'. I'd prefer if everything is
consistent so it's easy for people to grep.

> +			   root->objectid, node->start,
> +			   nr == 0 ? "small" : "large", nr,
> +			   BTRFS_NODEPTRS_PER_BLOCK(root->fs_info));
>  		return -EIO;
>  	}
>  
> @@ -293,13 +321,25 @@ int btrfs_check_node(struct btrfs_root *root, struct extent_buffer *node)
>  		btrfs_node_key_to_cpu(node, &next_key, slot + 1);
>  
>  		if (!bytenr) {
> -			CORRUPT("invalid item slot", node, root, slot);
> +			generic_err(root, node, slot,
> +			"invalid node pointer, shouldn't point to zero");
>  			ret = -EIO;
>  			goto out;
>  		}
> +		if (!IS_ALIGNED(bytenr, root->fs_info->sectorsize)) {
> +			generic_err(root, node, slot,
> +			"unaligned pointer, have %llu, should be aligned to %u",
> +				     bytenr, root->fs_info->sectorsize);
> +			ret = -EUCLEAN;
> +			goto out;
> +		}
>  
>  		if (btrfs_comp_cpu_keys(&key, &next_key) >= 0) {
> -			CORRUPT("bad key order", node, root, slot);
> +			generic_err(root, node, slot,
> +			"bad key order, current key (%llu %u %llu) next key (%llu %u %llu)",
> +				     key.objectid, key.type, key.offset,
> +				     next_key.objectid, next_key.type,
> +				     next_key.offset);
>  			ret = -EIO;
>  			goto out;
>  		}
> 

Re: [PATCH 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf

[Nikolay Borisov]

On 28.09.2017 06:36, Qu Wenruo wrote:
> Enhance the output to print:
> 1) Reason
> 2) Bad value
>    If reason can't explain enough
> 3) Good value (range)
> 
> Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
> ---
>  fs/btrfs/tree-checker.c | 26 ++++++++++++++++++++------
>  1 file changed, 20 insertions(+), 6 deletions(-)
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 4b615beb0ca9..4f847955fdc6 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -216,8 +216,8 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
>  			eb = btrfs_root_node(check_root);
>  			/* if leaf is the root, then it's fine */
>  			if (leaf != eb) {
> -				CORRUPT("non-root leaf's nritems is 0",
> -					leaf, check_root, 0);
> +				generic_err(check_root, leaf, 0,
> +					    "non-root leaf's nritems is 0");
>  				free_extent_buffer(eb);
>  				return -EUCLEAN;
>  			}
> @@ -248,7 +248,11 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
>  
>  		/* Make sure the keys are in the right order */
>  		if (btrfs_comp_cpu_keys(&prev_key, &key) >= 0) {
> -			CORRUPT("bad key order", leaf, root, slot);
> +			generic_err(root, leaf, slot,
> +				"bad key order, prev key (%llu %u %llu) current key (%llu %u %llu)",
> +				prev_key.objectid, prev_key.type,
> +				prev_key.offset, key.objectid, key.type,
> +				key.offset);
>  			return -EUCLEAN;
>  		}
>  
> @@ -263,7 +267,10 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
>  			item_end_expected = btrfs_item_offset_nr(leaf,
>  								 slot - 1);
>  		if (btrfs_item_end_nr(leaf, slot) != item_end_expected) {
> -			CORRUPT("slot offset bad", leaf, root, slot);
> +			generic_err(root, leaf, slot,
> +				"uncontinuous item end, have %u expect %u",
nit: s/uncontinious/discontinious
> +				btrfs_item_end_nr(leaf, slot),
> +				item_end_expected);
>  			return -EUCLEAN;
>  		}
>  
> @@ -274,14 +281,21 @@ int btrfs_check_leaf(struct btrfs_root *root, struct extent_buffer *leaf)
>  		 */
>  		if (btrfs_item_end_nr(leaf, slot) >
>  		    BTRFS_LEAF_DATA_SIZE(fs_info)) {
> -			CORRUPT("slot end outside of leaf", leaf, root, slot);
> +			generic_err(root, leaf, slot,
> +				"slot end outside of leaf, have %u expect range [0, %u]",
> +				btrfs_item_end_nr(leaf, slot),
> +				BTRFS_LEAF_DATA_SIZE(fs_info));
>  			return -EUCLEAN;
>  		}
>  
>  		/* Also check if the item pointer overlaps with btrfs item. */
>  		if (btrfs_item_nr_offset(slot) + sizeof(struct btrfs_item) >
>  		    btrfs_item_ptr_offset(leaf, slot)) {
> -			CORRUPT("slot overlap with its data", leaf, root, slot);
> +			generic_err(root, leaf, slot,
> +				"slot overlap with its data, item end %lu data start %lu",
> +				btrfs_item_nr_offset(slot) +
> +				sizeof(struct btrfs_item),
> +				btrfs_item_ptr_offset(leaf, slot));
>  			return -EUCLEAN;
>  		}
>  
> 

Re: [PATCH 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item

[Nikolay Borisov]

On 28.09.2017 06:36, Qu Wenruo wrote:
> Output the invalid member name and its bad value, along with its
> expected value range or alignment.
> 
> Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
> ---
>  fs/btrfs/tree-checker.c | 92 +++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 70 insertions(+), 22 deletions(-)
> 
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index 52e9ab8c2a79..1324fcae93c0 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -63,6 +63,47 @@ static void generic_err(const struct btrfs_root *root,
>  	va_end(args);
>  }
>  
> +/*
> + * Customized reporter for extent data item, since its key objectid and
> + * offset has its own meaning.
> + */
> +__printf(4, 5)
> +static void file_extent_err(const struct btrfs_root *root,
> +			    const struct extent_buffer *eb,
> +			    int slot, const char *fmt, ...)
> +{
> +	struct btrfs_key key;
> +	struct va_format vaf;
> +	va_list args;
> +
> +	btrfs_item_key_to_cpu(eb, &key, slot);
> +	va_start(args, fmt);
> +
> +	vaf.fmt = fmt;
> +	vaf.va = &args;
> +
> +	btrfs_crit(root->fs_info,
> +		"corrupt %s root=%llu tree_block=%llu slot=%d ino=%llu file_offset=%llu: %pV",

nit: Again, consider whether we should have : after the first %s so that
the string is consistent among different verifiers.

> +		btrfs_header_level(eb) == 0 ? "leaf" : "node",
> +		root->objectid, btrfs_header_bytenr(eb), slot,
> +		key.objectid, key.offset, &vaf);
> +	va_end(args);
> +}
> +
> +/*
> + * Return 0 if the btrfs_file_extent_##name is aligned to @align
> + * Else return 1
> + */
> +#define CHECK_FI_ALIGN(root, leaf, slot, fi, name, align)		\
> +({									\
> +	if (!IS_ALIGNED(btrfs_file_extent_##name(leaf, fi), align))	\
> +		file_extent_err(root, leaf, slot,			\
> +			"invalid file extent %s, have %llu, should be aligned to %u",\
> +			#name, btrfs_file_extent_##name(leaf, fi),	\
> +			align);						\
> +	(!IS_ALIGNED(btrfs_file_extent_##name(leaf, fi), align));	\
> +})
> +
>  static int check_extent_data_item(struct btrfs_root *root,
>  				  struct extent_buffer *leaf,
>  				  struct btrfs_key *key, int slot)
> @@ -72,15 +113,19 @@ static int check_extent_data_item(struct btrfs_root *root,
>  	u32 item_size = btrfs_item_size_nr(leaf, slot);
>  
>  	if (!IS_ALIGNED(key->offset, sectorsize)) {
> -		CORRUPT("unaligned key offset for file extent",
> -			leaf, root, slot);
> +		file_extent_err(root, leaf, slot,
> +			"unaligned key offset, have %llu should be aligned to %u",
> +			key->offset, sectorsize);
>  		return -EUCLEAN;
>  	}
>  
>  	fi = btrfs_item_ptr(leaf, slot, struct btrfs_file_extent_item);
>  
>  	if (btrfs_file_extent_type(leaf, fi) > BTRFS_FILE_EXTENT_TYPES) {
> -		CORRUPT("invalid file extent type", leaf, root, slot);
> +		file_extent_err(root, leaf, slot,
> +			"invalid file extent type, have %u expect range [0, %u]",
> +			btrfs_file_extent_type(leaf, fi),
> +			BTRFS_FILE_EXTENT_TYPES);
>  		return -EUCLEAN;
>  	}
>  
> @@ -89,18 +134,24 @@ static int check_extent_data_item(struct btrfs_root *root,
>  	 * and must be caught in open_ctree().
>  	 */
>  	if (btrfs_file_extent_compression(leaf, fi) > BTRFS_COMPRESS_TYPES) {
> -		CORRUPT("invalid file extent compression", leaf, root, slot);
> +		file_extent_err(root, leaf, slot,
> +			"invalid file extent compression, have %u expect range [0, %u]",
> +			btrfs_file_extent_compression(leaf, fi),
> +			BTRFS_COMPRESS_TYPES);
>  		return -EUCLEAN;
>  	}
>  	if (btrfs_file_extent_encryption(leaf, fi)) {
> -		CORRUPT("invalid file extent encryption", leaf, root, slot);
> +		file_extent_err(root, leaf, slot,
> +			"invalid file extent encryption, have %u expect 0",
> +			btrfs_file_extent_encryption(leaf, fi));
>  		return -EUCLEAN;
>  	}
>  	if (btrfs_file_extent_type(leaf, fi) == BTRFS_FILE_EXTENT_INLINE) {
>  		/* Inline extent must have 0 as key offset */
>  		if (key->offset) {
> -			CORRUPT("inline extent has non-zero key offset",
> -				leaf, root, slot);
> +			file_extent_err(root, leaf, slot,
> +				"invalid offset for inline extent, have %llu expect 0",
> +				key->offset);
>  			return -EUCLEAN;
>  		}
>  
> @@ -112,8 +163,10 @@ static int check_extent_data_item(struct btrfs_root *root,
>  		/* Uncompressed inline extent size must match item size */
>  		if (item_size != BTRFS_FILE_EXTENT_INLINE_DATA_START +
>  		    btrfs_file_extent_ram_bytes(leaf, fi)) {
> -			CORRUPT("plaintext inline extent has invalid size",
> -				leaf, root, slot);
> +			file_extent_err(root, leaf, slot,
> +				"invalid uncompressed inline extent size, have %u expect %llu",
> +				item_size, BTRFS_FILE_EXTENT_INLINE_DATA_START +
> +				btrfs_file_extent_ram_bytes(leaf, fi));
>  			return -EUCLEAN;
>  		}
>  		return 0;
> @@ -121,22 +174,17 @@ static int check_extent_data_item(struct btrfs_root *root,
>  
>  	/* Regular or preallocated extent has fixed item size */
>  	if (item_size != sizeof(*fi)) {
> -		CORRUPT(
> -		"regluar or preallocated extent data item size is invalid",
> -			leaf, root, slot);
> +		file_extent_err(root, leaf, slot,
> +			"invalid extent data item size for reg/prealloc, have %u expect %lu",
> +			item_size, sizeof(*fi));
>  		return -EUCLEAN;
>  	}
> -	if (!IS_ALIGNED(btrfs_file_extent_ram_bytes(leaf, fi), sectorsize) ||
> -	    !IS_ALIGNED(btrfs_file_extent_disk_bytenr(leaf, fi), sectorsize) ||
> -	    !IS_ALIGNED(btrfs_file_extent_disk_num_bytes(leaf, fi), sectorsize) ||
> -	    !IS_ALIGNED(btrfs_file_extent_offset(leaf, fi), sectorsize) ||
> -	    !IS_ALIGNED(btrfs_file_extent_num_bytes(leaf, fi), sectorsize)) {
> -		CORRUPT(
> -		"regular or preallocated extent data item has unaligned value",
> -			leaf, root, slot);
> +	if (CHECK_FI_ALIGN(root, leaf, slot, fi, ram_bytes, sectorsize) ||
> +	    CHECK_FI_ALIGN(root, leaf, slot, fi, disk_bytenr, sectorsize) ||
> +	    CHECK_FI_ALIGN(root, leaf, slot, fi, disk_num_bytes, sectorsize) ||
> +	    CHECK_FI_ALIGN(root, leaf, slot, fi, offset, sectorsize) ||
> +	    CHECK_FI_ALIGN(root, leaf, slot, fi, num_bytes, sectorsize))
>  		return -EUCLEAN;
> -	}
> -
>  	return 0;
>  }
>  
> 

Re: [PATCH 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item

[Qu Wenruo]

On 2017年09月28日 14:09, Nikolay Borisov wrote:
> 
> 
> On 28.09.2017 06:36, Qu Wenruo wrote:
>> Output the invalid member name and its bad value, along with its
>> expected value range or alignment.
>>
>> Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
>> ---
>>   fs/btrfs/tree-checker.c | 92 +++++++++++++++++++++++++++++++++++++------------
>>   1 file changed, 70 insertions(+), 22 deletions(-)
>>
>> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
>> index 52e9ab8c2a79..1324fcae93c0 100644
>> --- a/fs/btrfs/tree-checker.c
>> +++ b/fs/btrfs/tree-checker.c
>> @@ -63,6 +63,47 @@ static void generic_err(const struct btrfs_root *root,
>>   	va_end(args);
>>   }
>>   
>> +/*
>> + * Customized reporter for extent data item, since its key objectid and
>> + * offset has its own meaning.
>> + */
>> +__printf(4, 5)
>> +static void file_extent_err(const struct btrfs_root *root,
>> +			    const struct extent_buffer *eb,
>> +			    int slot, const char *fmt, ...)
>> +{
>> +	struct btrfs_key key;
>> +	struct va_format vaf;
>> +	va_list args;
>> +
>> +	btrfs_item_key_to_cpu(eb, &key, slot);
>> +	va_start(args, fmt);
>> +
>> +	vaf.fmt = fmt;
>> +	vaf.va = &args;
>> +
>> +	btrfs_crit(root->fs_info,
>> +		"corrupt %s root=%llu tree_block=%llu slot=%d ino=%llu file_offset=%llu: %pV",
> 
> nit: Again, consider whether we should have : after the first %s so that
> the string is consistent among different verifiers.

Consistence is important indeed.

I'll update the patchset to address it.

Thanks for pointing it out,
Qu

> 
>> +		btrfs_header_level(eb) == 0 ? "leaf" : "node",
>> +		root->objectid, btrfs_header_bytenr(eb), slot,
>> +		key.objectid, key.offset, &vaf);
>> +	va_end(args);
>> +}
>> +
>> +/*
>> + * Return 0 if the btrfs_file_extent_##name is aligned to @align
>> + * Else return 1
>> + */
>> +#define CHECK_FI_ALIGN(root, leaf, slot, fi, name, align)		\
>> +({									\
>> +	if (!IS_ALIGNED(btrfs_file_extent_##name(leaf, fi), align))	\
>> +		file_extent_err(root, leaf, slot,			\
>> +			"invalid file extent %s, have %llu, should be aligned to %u",\
>> +			#name, btrfs_file_extent_##name(leaf, fi),	\
>> +			align);						\
>> +	(!IS_ALIGNED(btrfs_file_extent_##name(leaf, fi), align));	\
>> +})
>> +
>>   static int check_extent_data_item(struct btrfs_root *root,
>>   				  struct extent_buffer *leaf,
>>   				  struct btrfs_key *key, int slot)
>> @@ -72,15 +113,19 @@ static int check_extent_data_item(struct btrfs_root *root,
>>   	u32 item_size = btrfs_item_size_nr(leaf, slot);
>>   
>>   	if (!IS_ALIGNED(key->offset, sectorsize)) {
>> -		CORRUPT("unaligned key offset for file extent",
>> -			leaf, root, slot);
>> +		file_extent_err(root, leaf, slot,
>> +			"unaligned key offset, have %llu should be aligned to %u",
>> +			key->offset, sectorsize);
>>   		return -EUCLEAN;
>>   	}
>>   
>>   	fi = btrfs_item_ptr(leaf, slot, struct btrfs_file_extent_item);
>>   
>>   	if (btrfs_file_extent_type(leaf, fi) > BTRFS_FILE_EXTENT_TYPES) {
>> -		CORRUPT("invalid file extent type", leaf, root, slot);
>> +		file_extent_err(root, leaf, slot,
>> +			"invalid file extent type, have %u expect range [0, %u]",
>> +			btrfs_file_extent_type(leaf, fi),
>> +			BTRFS_FILE_EXTENT_TYPES);
>>   		return -EUCLEAN;
>>   	}
>>   
>> @@ -89,18 +134,24 @@ static int check_extent_data_item(struct btrfs_root *root,
>>   	 * and must be caught in open_ctree().
>>   	 */
>>   	if (btrfs_file_extent_compression(leaf, fi) > BTRFS_COMPRESS_TYPES) {
>> -		CORRUPT("invalid file extent compression", leaf, root, slot);
>> +		file_extent_err(root, leaf, slot,
>> +			"invalid file extent compression, have %u expect range [0, %u]",
>> +			btrfs_file_extent_compression(leaf, fi),
>> +			BTRFS_COMPRESS_TYPES);
>>   		return -EUCLEAN;
>>   	}
>>   	if (btrfs_file_extent_encryption(leaf, fi)) {
>> -		CORRUPT("invalid file extent encryption", leaf, root, slot);
>> +		file_extent_err(root, leaf, slot,
>> +			"invalid file extent encryption, have %u expect 0",
>> +			btrfs_file_extent_encryption(leaf, fi));
>>   		return -EUCLEAN;
>>   	}
>>   	if (btrfs_file_extent_type(leaf, fi) == BTRFS_FILE_EXTENT_INLINE) {
>>   		/* Inline extent must have 0 as key offset */
>>   		if (key->offset) {
>> -			CORRUPT("inline extent has non-zero key offset",
>> -				leaf, root, slot);
>> +			file_extent_err(root, leaf, slot,
>> +				"invalid offset for inline extent, have %llu expect 0",
>> +				key->offset);
>>   			return -EUCLEAN;
>>   		}
>>   
>> @@ -112,8 +163,10 @@ static int check_extent_data_item(struct btrfs_root *root,
>>   		/* Uncompressed inline extent size must match item size */
>>   		if (item_size != BTRFS_FILE_EXTENT_INLINE_DATA_START +
>>   		    btrfs_file_extent_ram_bytes(leaf, fi)) {
>> -			CORRUPT("plaintext inline extent has invalid size",
>> -				leaf, root, slot);
>> +			file_extent_err(root, leaf, slot,
>> +				"invalid uncompressed inline extent size, have %u expect %llu",
>> +				item_size, BTRFS_FILE_EXTENT_INLINE_DATA_START +
>> +				btrfs_file_extent_ram_bytes(leaf, fi));
>>   			return -EUCLEAN;
>>   		}
>>   		return 0;
>> @@ -121,22 +174,17 @@ static int check_extent_data_item(struct btrfs_root *root,
>>   
>>   	/* Regular or preallocated extent has fixed item size */
>>   	if (item_size != sizeof(*fi)) {
>> -		CORRUPT(
>> -		"regluar or preallocated extent data item size is invalid",
>> -			leaf, root, slot);
>> +		file_extent_err(root, leaf, slot,
>> +			"invalid extent data item size for reg/prealloc, have %u expect %lu",
>> +			item_size, sizeof(*fi));
>>   		return -EUCLEAN;
>>   	}
>> -	if (!IS_ALIGNED(btrfs_file_extent_ram_bytes(leaf, fi), sectorsize) ||
>> -	    !IS_ALIGNED(btrfs_file_extent_disk_bytenr(leaf, fi), sectorsize) ||
>> -	    !IS_ALIGNED(btrfs_file_extent_disk_num_bytes(leaf, fi), sectorsize) ||
>> -	    !IS_ALIGNED(btrfs_file_extent_offset(leaf, fi), sectorsize) ||
>> -	    !IS_ALIGNED(btrfs_file_extent_num_bytes(leaf, fi), sectorsize)) {
>> -		CORRUPT(
>> -		"regular or preallocated extent data item has unaligned value",
>> -			leaf, root, slot);
>> +	if (CHECK_FI_ALIGN(root, leaf, slot, fi, ram_bytes, sectorsize) ||
>> +	    CHECK_FI_ALIGN(root, leaf, slot, fi, disk_bytenr, sectorsize) ||
>> +	    CHECK_FI_ALIGN(root, leaf, slot, fi, disk_num_bytes, sectorsize) ||
>> +	    CHECK_FI_ALIGN(root, leaf, slot, fi, offset, sectorsize) ||
>> +	    CHECK_FI_ALIGN(root, leaf, slot, fi, num_bytes, sectorsize))
>>   		return -EUCLEAN;
>> -	}
>> -
>>   	return 0;
>>   }
>>   
>>