* [PATCH v4 1/5] btrfs: Move leaf and node validation checker to tree-checker.ch
2017-10-09 1:51 [PATCH v4 0/5] Enhance tree block validation checker Qu Wenruo
@ 2017-10-09 1:51 ` Qu Wenruo
2017-10-09 1:51 ` [PATCH v4 2/5] btrfs: tree-checker: Enhance btrfs_check_node output Qu Wenruo
` (4 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2017-10-09 1:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, nborisov
It's no doubt the comprehensive tree block checker will become larger
and larger, so moving them into their own files is quite reasonable.
Signed-off-by: Qu Wenruo <quwenruo.btrfs@gmx.com>
---
fs/btrfs/Makefile | 2 +-
fs/btrfs/disk-io.c | 285 +-------------------------------------------
fs/btrfs/tree-checker.c | 310 ++++++++++++++++++++++++++++++++++++++++++++++++
fs/btrfs/tree-checker.h | 23 ++++
4 files changed, 338 insertions(+), 282 deletions(-)
create mode 100644 fs/btrfs/tree-checker.c
create mode 100644 fs/btrfs/tree-checker.h
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/disk-io.c b/fs/btrfs/disk-io.c
index c8633f2abdf1..09407384e996 100644
--- a/fs/btrfs/disk-io.c
+++ b/fs/btrfs/disk-io.c
@@ -50,6 +50,7 @@
#include "sysfs.h"
#include "qgroup.h"
#include "compression.h"
+#include "tree-checker.h"
#ifdef CONFIG_X86
#include <asm/cpufeature.h>
@@ -543,284 +544,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 +609,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 +3869,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..63307e8426a6
--- /dev/null
+++ b/fs/btrfs/tree-checker.c
@@ -0,0 +1,310 @@
+/*
+ * 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 "tree-checker.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;
+}
diff --git a/fs/btrfs/tree-checker.h b/fs/btrfs/tree-checker.h
new file mode 100644
index 000000000000..7b789063749c
--- /dev/null
+++ b/fs/btrfs/tree-checker.h
@@ -0,0 +1,23 @@
+/*
+ * 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.
+ */
+
+#ifndef __BTRFS_TREE_CHECKER__
+#define __BTRFS_TREE_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
--
2.14.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v4 2/5] btrfs: tree-checker: Enhance btrfs_check_node output
2017-10-09 1:51 [PATCH v4 0/5] Enhance tree block validation checker Qu Wenruo
2017-10-09 1:51 ` [PATCH v4 1/5] btrfs: Move leaf and node validation checker to tree-checker.ch Qu Wenruo
@ 2017-10-09 1:51 ` Qu Wenruo
2017-10-09 1:51 ` [PATCH v4 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf Qu Wenruo
` (3 subsequent siblings)
5 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2017-10-09 1:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, nborisov
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, and replace EIO with EUCLEAN.
For nr_items error, report if it's too large or too small, and output
valid value range.
For node block 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 | 70 ++++++++++++++++++++++++++++++++++++++++++++-----
1 file changed, 63 insertions(+), 7 deletions(-)
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 63307e8426a6..b4ced8d3ce2a 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -38,6 +38,48 @@
btrfs_header_level(eb) == 0 ? "leaf" : "node", \
reason, btrfs_header_bytenr(eb), root->objectid, slot)
+/*
+ * Error message should follow the format below:
+ * corrupt <type>: <identifier>, <reason>[, <bad_value>]
+ *
+ * @type: Either leaf or node
+ * @identifier: The necessary info to locate the leaf/node.
+ * It's recommened to decode key.objecitd/offset if it's
+ * meaningful.
+ * @reason: What's wrong
+ * @bad_value: Optional, it's recommened to output bad value and its
+ * expected value (range).
+ *
+ * Since comma is used to separate the components, only SPACE is allowed
+ * inside each component.
+ */
+
+/*
+ * 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)
@@ -283,9 +325,11 @@ 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);
- return -EIO;
+ "corrupt node: root=%llu block=%llu, nritems too %s, have %lu expect range [1,%u]",
+ root->objectid, node->start,
+ nr == 0 ? "small" : "large", nr,
+ BTRFS_NODEPTRS_PER_BLOCK(root->fs_info));
+ return -EUCLEAN;
}
for (slot = 0; slot < nr - 1; slot++) {
@@ -294,14 +338,26 @@ 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);
- ret = -EIO;
+ generic_err(root, node, slot,
+ "invalid NULL node pointer");
+ ret = -EUCLEAN;
+ 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);
- ret = -EIO;
+ 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 = -EUCLEAN;
goto out;
}
}
--
2.14.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v4 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf
2017-10-09 1:51 [PATCH v4 0/5] Enhance tree block validation checker Qu Wenruo
2017-10-09 1:51 ` [PATCH v4 1/5] btrfs: Move leaf and node validation checker to tree-checker.ch Qu Wenruo
2017-10-09 1:51 ` [PATCH v4 2/5] btrfs: tree-checker: Enhance btrfs_check_node output Qu Wenruo
@ 2017-10-09 1:51 ` Qu Wenruo
2017-10-11 15:41 ` Nikolay Borisov
2017-10-09 1:51 ` [PATCH v4 4/5] btrfs: tree-checker: Enhance output for check_csum_item Qu Wenruo
` (2 subsequent siblings)
5 siblings, 1 reply; 11+ messages in thread
From: Qu Wenruo @ 2017-10-09 1:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, nborisov
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 | 27 +++++++++++++++++++++------
1 file changed, 21 insertions(+), 6 deletions(-)
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index b4ced8d3ce2a..7bba195ecc8b 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -233,8 +233,9 @@ 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,
+ "invalid nritems, have %u shouldn't be 0 for non-root leaf",
+ nritems);
free_extent_buffer(eb);
return -EUCLEAN;
}
@@ -265,7 +266,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;
}
@@ -280,7 +285,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,
+ "discontinious item end, have %u expect %u",
+ btrfs_item_end_nr(leaf, slot),
+ item_end_expected);
return -EUCLEAN;
}
@@ -291,14 +299,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.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v4 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf
2017-10-09 1:51 ` [PATCH v4 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf Qu Wenruo
@ 2017-10-11 15:41 ` Nikolay Borisov
2017-10-11 17:56 ` David Sterba
2017-10-12 0:28 ` Qu Wenruo
0 siblings, 2 replies; 11+ messages in thread
From: Nikolay Borisov @ 2017-10-11 15:41 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: dsterba
On 9.10.2017 04:51, 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 | 27 +++++++++++++++++++++------
> 1 file changed, 21 insertions(+), 6 deletions(-)
>
> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> index b4ced8d3ce2a..7bba195ecc8b 100644
> --- a/fs/btrfs/tree-checker.c
> +++ b/fs/btrfs/tree-checker.c
> @@ -233,8 +233,9 @@ 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,
> + "invalid nritems, have %u shouldn't be 0 for non-root leaf",
> + nritems);
I'm a bit confused by what this error messages wants to convey. Even
reading the previous version with CORRUPT() it still didn't make sense.
So what we want to say here is we shouldn't have empty leaf nodes. So
Something along the line of "Unexpected empty leaf".
Why would the (leaf != eb) check not trigger, given that we call
btrfs_check_leaf when we now that the item is a leaf (level is 0 )?
> free_extent_buffer(eb);
> return -EUCLEAN;
> }
> @@ -265,7 +266,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;
> }
>
> @@ -280,7 +285,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,
> + "discontinious item end, have %u expect %u",
s/discontinious/unexpected ?
> + btrfs_item_end_nr(leaf, slot),
> + item_end_expected);
> return -EUCLEAN;
> }
>
> @@ -291,14 +299,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;
> }
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf
2017-10-11 15:41 ` Nikolay Borisov
@ 2017-10-11 17:56 ` David Sterba
2017-10-12 0:28 ` Qu Wenruo
1 sibling, 0 replies; 11+ messages in thread
From: David Sterba @ 2017-10-11 17:56 UTC (permalink / raw)
To: Nikolay Borisov; +Cc: Qu Wenruo, linux-btrfs, dsterba
On Wed, Oct 11, 2017 at 06:41:32PM +0300, Nikolay Borisov wrote:
>
>
> On 9.10.2017 04:51, 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 | 27 +++++++++++++++++++++------
> > 1 file changed, 21 insertions(+), 6 deletions(-)
> >
> > diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
> > index b4ced8d3ce2a..7bba195ecc8b 100644
> > --- a/fs/btrfs/tree-checker.c
> > +++ b/fs/btrfs/tree-checker.c
> > @@ -233,8 +233,9 @@ 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,
> > + "invalid nritems, have %u shouldn't be 0 for non-root leaf",
> > + nritems);
>
> I'm a bit confused by what this error messages wants to convey. Even
> reading the previous version with CORRUPT() it still didn't make sense.
> So what we want to say here is we shouldn't have empty leaf nodes. So
> Something along the line of "Unexpected empty leaf".
>
> Why would the (leaf != eb) check not trigger, given that we call
> btrfs_check_leaf when we now that the item is a leaf (level is 0 )?
I've merged the patches, with more adjusmtents to the wording, so any
updates please send as separate patches.
>
>
> > free_extent_buffer(eb);
> > return -EUCLEAN;
> > }
> > @@ -265,7 +266,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;
> > }
> >
> > @@ -280,7 +285,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,
> > + "discontinious item end, have %u expect %u",
>
> s/discontinious/unexpected ?
I've changed that to 'unexpected item end, ...'
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf
2017-10-11 15:41 ` Nikolay Borisov
2017-10-11 17:56 ` David Sterba
@ 2017-10-12 0:28 ` Qu Wenruo
2017-10-12 5:57 ` Nikolay Borisov
1 sibling, 1 reply; 11+ messages in thread
From: Qu Wenruo @ 2017-10-12 0:28 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs; +Cc: dsterba
On 2017年10月11日 23:41, Nikolay Borisov wrote:
>
>
> On 9.10.2017 04:51, 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 | 27 +++++++++++++++++++++------
>> 1 file changed, 21 insertions(+), 6 deletions(-)
>>
>> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
>> index b4ced8d3ce2a..7bba195ecc8b 100644
>> --- a/fs/btrfs/tree-checker.c
>> +++ b/fs/btrfs/tree-checker.c
>> @@ -233,8 +233,9 @@ 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,
>> + "invalid nritems, have %u shouldn't be 0 for non-root leaf",
>> + nritems);
>
> I'm a bit confused by what this error messages wants to convey. Even
> reading the previous version with CORRUPT() it still didn't make sense.
> So what we want to say here is we shouldn't have empty leaf nodes. So
> Something along the line of "Unexpected empty leaf".
Yes, the error message is too fixed to follow the output format.
>
> Why would the (leaf != eb) check not trigger, given that we call
> btrfs_check_leaf when we now that the item is a leaf (level is 0 )?
What's the problem here? I didn't really get your point.
Did you mean leaf can't be tree root? Or empty tree root is not possible?
It's completely possible for a leaf to be a tree root.
All tree roots of a newly created (without --rootdir) is leaf.
Because the content of each tree is so few that one leaf can contain
them all.
And it's also very possible to have empty tree, whose root (leaf) is
also empty.
Still for a newly created btrfs, its csum tree is empty.
Its uuid tree is also empty.
But the only valid case for empty leaf is when it's a tree root.
So the code just checks it, and I didn't find anything wrong here.
Thanks,
Qu
>
>
>> free_extent_buffer(eb);
>> return -EUCLEAN;
>> }
>> @@ -265,7 +266,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;
>> }
>>
>> @@ -280,7 +285,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,
>> + "discontinious item end, have %u expect %u",
>
> s/discontinious/unexpected ?
>
>> + btrfs_item_end_nr(leaf, slot),
>> + item_end_expected);
>> return -EUCLEAN;
>> }
>>
>> @@ -291,14 +299,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;
>> }
>>
>>
^ permalink raw reply [flat|nested] 11+ messages in thread* Re: [PATCH v4 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf
2017-10-12 0:28 ` Qu Wenruo
@ 2017-10-12 5:57 ` Nikolay Borisov
0 siblings, 0 replies; 11+ messages in thread
From: Nikolay Borisov @ 2017-10-12 5:57 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs
On 12.10.2017 03:28, Qu Wenruo wrote:
>
>
> On 2017年10月11日 23:41, Nikolay Borisov wrote:
>>
>>
>> On 9.10.2017 04:51, 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 | 27 +++++++++++++++++++++------
>>> 1 file changed, 21 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
>>> index b4ced8d3ce2a..7bba195ecc8b 100644
>>> --- a/fs/btrfs/tree-checker.c
>>> +++ b/fs/btrfs/tree-checker.c
>>> @@ -233,8 +233,9 @@ 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,
>>> + "invalid nritems, have %u shouldn't be 0 for
>>> non-root leaf",
>>> + nritems);
>>
>> I'm a bit confused by what this error messages wants to convey. Even
>> reading the previous version with CORRUPT() it still didn't make sense.
>> So what we want to say here is we shouldn't have empty leaf nodes. So
>> Something along the line of "Unexpected empty leaf".
>
> Yes, the error message is too fixed to follow the output format.
>>
>> Why would the (leaf != eb) check not trigger, given that we call
>> btrfs_check_leaf when we now that the item is a leaf (level is 0 )?
>
> What's the problem here? I didn't really get your point.
>
> Did you mean leaf can't be tree root? Or empty tree root is not possible?
>
>
> It's completely possible for a leaf to be a tree root.
>
> All tree roots of a newly created (without --rootdir) is leaf.
> Because the content of each tree is so few that one leaf can contain
> them all.
>
>
> And it's also very possible to have empty tree, whose root (leaf) is
> also empty.
> Still for a newly created btrfs, its csum tree is empty.
> Its uuid tree is also empty.
>
>
> But the only valid case for empty leaf is when it's a tree root.
> So the code just checks it, and I didn't find anything wrong here.
I was just confused when the invariant wouldn't hold. Now that you've
explained it I agree with the change.
>
> Thanks,
> Qu
>
>>
>>
>>> free_extent_buffer(eb);
>>> return -EUCLEAN;
>>> }
>>> @@ -265,7 +266,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;
>>> }
>>> @@ -280,7 +285,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,
>>> + "discontinious item end, have %u expect %u",
>>
>> s/discontinious/unexpected ?
>>
>>> + btrfs_item_end_nr(leaf, slot),
>>> + item_end_expected);
>>> return -EUCLEAN;
>>> }
>>> @@ -291,14 +299,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;
>>> }
>>>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH v4 4/5] btrfs: tree-checker: Enhance output for check_csum_item
2017-10-09 1:51 [PATCH v4 0/5] Enhance tree block validation checker Qu Wenruo
` (2 preceding siblings ...)
2017-10-09 1:51 ` [PATCH v4 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf Qu Wenruo
@ 2017-10-09 1:51 ` Qu Wenruo
2017-10-09 1:51 ` [PATCH v4 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item Qu Wenruo
2017-10-11 17:57 ` [PATCH v4 0/5] Enhance tree block validation checker David Sterba
5 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2017-10-09 1:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, nborisov
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 7bba195ecc8b..42e002f8a560 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -164,15 +164,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 key 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.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* [PATCH v4 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item
2017-10-09 1:51 [PATCH v4 0/5] Enhance tree block validation checker Qu Wenruo
` (3 preceding siblings ...)
2017-10-09 1:51 ` [PATCH v4 4/5] btrfs: tree-checker: Enhance output for check_csum_item Qu Wenruo
@ 2017-10-09 1:51 ` Qu Wenruo
2017-10-11 17:57 ` [PATCH v4 0/5] Enhance tree block validation checker David Sterba
5 siblings, 0 replies; 11+ messages in thread
From: Qu Wenruo @ 2017-10-09 1:51 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba, nborisov
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 | 98 +++++++++++++++++++++++++++++++++++--------------
1 file changed, 70 insertions(+), 28 deletions(-)
diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c
index 42e002f8a560..c7a09cc2a17c 100644
--- a/fs/btrfs/tree-checker.c
+++ b/fs/btrfs/tree-checker.c
@@ -32,12 +32,6 @@
#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)
-
/*
* Error message should follow the format below:
* corrupt <type>: <identifier>, <reason>[, <bad_value>]
@@ -80,6 +74,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 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 @alignment
+ * Else return 1
+ */
+#define CHECK_FE_ALIGNED(root, leaf, slot, fi, name, alignment) \
+({ \
+ if (!IS_ALIGNED(btrfs_file_extent_##name((leaf), (fi)), (alignment)))\
+ file_extent_err((root), (leaf), (slot), \
+ "invalid %s for file extent, have %llu, should be aligned to %u",\
+ (#name), btrfs_file_extent_##name((leaf), (fi)),\
+ (alignment)); \
+ (!IS_ALIGNED(btrfs_file_extent_##name((leaf), (fi)), (alignment)));\
+})
+
static int check_extent_data_item(struct btrfs_root *root,
struct extent_buffer *leaf,
struct btrfs_key *key, int slot)
@@ -89,15 +124,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 file_offset for file extent, 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 type for file extent, have %u expect range [0, %u]",
+ btrfs_file_extent_type(leaf, fi),
+ BTRFS_FILE_EXTENT_TYPES);
return -EUCLEAN;
}
@@ -106,18 +145,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 compression for file extent, 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 encryption for file extent, 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 file_offset for inline file extent, have %llu expect 0",
+ key->offset);
return -EUCLEAN;
}
@@ -129,8 +174,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 ram_bytes for uncompressed inline extent, have %u expect %llu",
+ item_size, BTRFS_FILE_EXTENT_INLINE_DATA_START +
+ btrfs_file_extent_ram_bytes(leaf, fi));
return -EUCLEAN;
}
return 0;
@@ -138,22 +185,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 item size for reg/prealloc file extent, 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_FE_ALIGNED(root, leaf, slot, fi, ram_bytes, sectorsize) ||
+ CHECK_FE_ALIGNED(root, leaf, slot, fi, disk_bytenr, sectorsize) ||
+ CHECK_FE_ALIGNED(root, leaf, slot, fi, disk_num_bytes, sectorsize) ||
+ CHECK_FE_ALIGNED(root, leaf, slot, fi, offset, sectorsize) ||
+ CHECK_FE_ALIGNED(root, leaf, slot, fi, num_bytes, sectorsize))
return -EUCLEAN;
- }
-
return 0;
}
--
2.14.2
^ permalink raw reply related [flat|nested] 11+ messages in thread* Re: [PATCH v4 0/5] Enhance tree block validation checker
2017-10-09 1:51 [PATCH v4 0/5] Enhance tree block validation checker Qu Wenruo
` (4 preceding siblings ...)
2017-10-09 1:51 ` [PATCH v4 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item Qu Wenruo
@ 2017-10-11 17:57 ` David Sterba
5 siblings, 0 replies; 11+ messages in thread
From: David Sterba @ 2017-10-11 17:57 UTC (permalink / raw)
To: Qu Wenruo; +Cc: linux-btrfs, dsterba, nborisov
On Mon, Oct 09, 2017 at 01:51:01AM +0000, Qu Wenruo wrote:
> 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 separate files: tree-checker.[ch].
>
> Also added a output format rule to try to make all output message
> follow the same format.
>
> Some example output using btrfs-progs fsck-test images looks like:
>
> For unagliend file extent member:
> ---
> BTRFS critical (device loop0): corrupt leaf: root=1 block=29360128 slot=7 ino=257 file_offset=0, invalid disk_bytenr for file extent, have 755944791, should be aligned to 4096
> ---
>
> For bad leaf holes:
> ---
> BTRFS critical (device loop0): corrupt leaf: root=1 block=29360128 slot=28, discontinious item end, have 9387 expect 15018
> ---
>
> Changelog:
> v2:
> Unify the error string format, so it should be easier to grep them
> from dmesg. Thanks Nikolay for pointing this out.
> Remove unused CORRUPT() macro.
> v3:
> Replace EIO with EUCLEAN in 2nd patch. Thanks Nikolay for pointing
> this out.
> Correct "btrfs-progs:" to "btrfs:" for 1st patch.
> v4:
> Code style change suggested by David.
> Use more easy-to-understand error message for NULL node pointer,
> suggested by Nikolay.
> Helper macro enhancement, including naming change and argument
> protection suggested by David.
> Separate tree-checker.h suggested by David.
Patches merged to the devel queue, with updates. The strings that
overflow 80 chars should be un-indented, plase do that in future
patches. Thanks.
^ permalink raw reply [flat|nested] 11+ messages in thread