* [PATCH 1/5] btrfs: Move leaf and node validation checker to tree-checker.c
2017-09-28 3:36 [PATCH 0/5] Enhance tree block validation checker Qu Wenruo
@ 2017-09-28 3:36 ` Qu Wenruo
2017-09-28 3:36 ` [PATCH 2/5] btrfs: tree-checker: Enhance btrfs_check_node output Qu Wenruo
` (3 subsequent siblings)
4 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2017-09-28 3:36 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 2/5] btrfs: tree-checker: Enhance btrfs_check_node output
2017-09-28 3:36 [PATCH 0/5] Enhance tree block validation checker Qu Wenruo
2017-09-28 3:36 ` [PATCH 1/5] btrfs: Move leaf and node validation checker to tree-checker.c Qu Wenruo
@ 2017-09-28 3:36 ` Qu Wenruo
2017-09-28 5:57 ` Nikolay Borisov
2017-09-28 3:36 ` [PATCH 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf Qu Wenruo
` (2 subsequent siblings)
4 siblings, 1 reply; 10+ messages in thread
From: Qu Wenruo @ 2017-09-28 3:36 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 2/5] btrfs: tree-checker: Enhance btrfs_check_node output
2017-09-28 3:36 ` [PATCH 2/5] btrfs: tree-checker: Enhance btrfs_check_node output Qu Wenruo
@ 2017-09-28 5:57 ` Nikolay Borisov
0 siblings, 0 replies; 10+ messages in thread
From: Nikolay Borisov @ 2017-09-28 5:57 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: dsterba
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;
> }
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf
2017-09-28 3:36 [PATCH 0/5] Enhance tree block validation checker Qu Wenruo
2017-09-28 3:36 ` [PATCH 1/5] btrfs: Move leaf and node validation checker to tree-checker.c Qu Wenruo
2017-09-28 3:36 ` [PATCH 2/5] btrfs: tree-checker: Enhance btrfs_check_node output Qu Wenruo
@ 2017-09-28 3:36 ` Qu Wenruo
2017-09-28 6:01 ` Nikolay Borisov
2017-09-28 3:36 ` [PATCH 4/5] btrfs: tree-checker: Enhance output for check_csum_item Qu Wenruo
2017-09-28 3:36 ` [PATCH 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item Qu Wenruo
4 siblings, 1 reply; 10+ messages in thread
From: Qu Wenruo @ 2017-09-28 3:36 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf
2017-09-28 3:36 ` [PATCH 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf Qu Wenruo
@ 2017-09-28 6:01 ` Nikolay Borisov
0 siblings, 0 replies; 10+ messages in thread
From: Nikolay Borisov @ 2017-09-28 6:01 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: dsterba
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;
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 4/5] btrfs: tree-checker: Enhance output for check_csum_item
2017-09-28 3:36 [PATCH 0/5] Enhance tree block validation checker Qu Wenruo
` (2 preceding siblings ...)
2017-09-28 3:36 ` [PATCH 3/5] btrfs: tree-checker: Enhance output for btrfs_check_leaf Qu Wenruo
@ 2017-09-28 3:36 ` Qu Wenruo
2017-09-28 3:36 ` [PATCH 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item Qu Wenruo
4 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2017-09-28 3:36 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* [PATCH 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item
2017-09-28 3:36 [PATCH 0/5] Enhance tree block validation checker Qu Wenruo
` (3 preceding siblings ...)
2017-09-28 3:36 ` [PATCH 4/5] btrfs: tree-checker: Enhance output for check_csum_item Qu Wenruo
@ 2017-09-28 3:36 ` Qu Wenruo
2017-09-28 6:09 ` Nikolay Borisov
4 siblings, 1 reply; 10+ messages in thread
From: Qu Wenruo @ 2017-09-28 3:36 UTC (permalink / raw)
To: linux-btrfs; +Cc: dsterba
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
^ permalink raw reply related [flat|nested] 10+ messages in thread* Re: [PATCH 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item
2017-09-28 3:36 ` [PATCH 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item Qu Wenruo
@ 2017-09-28 6:09 ` Nikolay Borisov
2017-09-28 8:27 ` Qu Wenruo
0 siblings, 1 reply; 10+ messages in thread
From: Nikolay Borisov @ 2017-09-28 6:09 UTC (permalink / raw)
To: Qu Wenruo, linux-btrfs; +Cc: dsterba
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;
> }
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread* Re: [PATCH 5/5] btrfs: tree-checker: Enhance output for check_extent_data_item
2017-09-28 6:09 ` Nikolay Borisov
@ 2017-09-28 8:27 ` Qu Wenruo
0 siblings, 0 replies; 10+ messages in thread
From: Qu Wenruo @ 2017-09-28 8:27 UTC (permalink / raw)
To: Nikolay Borisov, linux-btrfs; +Cc: dsterba
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;
>> }
>>
>>
^ permalink raw reply [flat|nested] 10+ messages in thread