Hi,
I have tested btrfsck in some error case, such as item missing
case or CRC error case or so. In this test session I found some
bugs in btrfsck.c and disk-io.c in btrfs-progs.
1. The last pair of ptrs and items are not checked at 2 place
of check_node/leaf.
2. Csum printed CRC error message is not correct.
3. In CRC error, btrfsck fails in segmentation fault.
I''ll post three tiny patches to fix these bugs.
--
taruisi
--
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
TARUISI Hiroaki
2009-Oct-02 05:39 UTC
[PATCH] btrfsck: Fix Check limit in check_node/leaf.
In btrfsck.c( check_node/leaf ), it seems that last pair of
ptrs and items are not checked at 2 place.
Signed-off-by: TARUISI Hiroaki <taruishi.hiroak@jp.fujitsu.com>
---
btrfsck.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/btrfsck.c b/btrfsck.c
index 73f1836..ecc7b19 100644
--- a/btrfsck.c
+++ b/btrfsck.c
@@ -1720,7 +1720,7 @@ static int check_node(struct btrfs_root *root,
if (memcmp(parent_key, &key, sizeof(key)))
return 1;
}
- for (i = 0; nritems > 1 && i < nritems - 2; i++) {
+ for (i = 0; nritems > 1 && i < nritems - 1; i++) {
btrfs_node_key(buf, &key, i);
btrfs_node_key_to_cpu(buf, &cpukey, i + 1);
if (btrfs_comp_keys(&key, &cpukey) >= 0)
@@ -1759,7 +1759,7 @@ static int check_leaf(struct btrfs_root *root,
(unsigned long long)btrfs_header_bytenr(buf));
return 1;
}
- for (i = 0; nritems > 1 && i < nritems - 2; i++) {
+ for (i = 0; nritems > 1 && i < nritems - 1; i++) {
btrfs_item_key(buf, &key, i);
btrfs_item_key_to_cpu(buf, &cpukey, i + 1);
if (btrfs_comp_keys(&key, &cpukey) >= 0) {
--
1.6.2.1
--
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
TARUISI Hiroaki
2009-Oct-02 05:52 UTC
[PATCH] btrfs-progs: CRC on disk printed correctly in CRC error message
I''ve tested btrfsck in CRC error case. However CRC on disk printed
in error message is not correct (extent_buffer itsself is printed).
Signed-off-by: TARUISI Hiroaki <taruishi.hiroak@jp.fujitsu.com>
---
disk-io.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/disk-io.c b/disk-io.c
index addebe1..190301f 100644
--- a/disk-io.c
+++ b/disk-io.c
@@ -86,7 +86,7 @@ int csum_tree_block_size(struct extent_buffer *buf, u16
csum_size,
if (memcmp_extent_buffer(buf, result, 0, csum_size)) {
printk("checksum verify failed on %llu wanted %X "
"found %X\n", (unsigned long long)buf->start,
- *((int *)result), *((int *)buf));
+ *((int *)result), *((int *)buf->data));
free(result);
return 1;
}
--
1.6.2.1
--
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
In tree block CRC error case, btrfsck fails in segmentation fault.
In CRC error case, read_tree_block returns NULL as extent_buffer
like other(ex. ENOMEM) case, and btrfsck refers it without check.
If CRC error tree block is tree root, btrfsck aborts in previous
stage.
Like this, as for CRC error handling, BUG() is often used such as
in find_and_setup_root(disk-io.c), but CRC error is not a bug, so,
btrfsck should not abort but report, I think.
Because read_tree_block puts error message in CRC error case,
patch logic is simply return to caller with error.
Signed-off-by: TARUISI Hiroaki <taruishi.hiroak@jp.fujitsu.com>
---
btrfsck.c | 7 +++++++
1 files changed, 7 insertions(+), 0 deletions(-)
diff --git a/btrfsck.c b/btrfsck.c
index ecc7b19..cc00e28 100644
--- a/btrfsck.c
+++ b/btrfsck.c
@@ -2504,6 +2504,9 @@ static int run_next_block(struct btrfs_root *root,
/* fixme, get the real parent transid */
buf = read_tree_block(root, bytenr, size, 0);
+ if (buf == NULL)
+ return -1;
+
nritems = btrfs_header_nritems(buf);
ret = btrfs_lookup_extent_info(NULL, root, bytenr, size, NULL, &flags);
@@ -2785,6 +2788,8 @@ static int check_extents(struct btrfs_root *root)
btrfs_root_bytenr(&ri),
btrfs_level_size(root,
btrfs_root_level(&ri)), 0);
+ if (buf == NULL)
+ return 1;
add_root_to_pending(buf, bits, bits_nr, &extent_cache,
&pending, &seen, &reada, &nodes,
&found_key);
@@ -2796,6 +2801,8 @@ static int check_extents(struct btrfs_root *root)
while(1) {
ret = run_next_block(root, bits, bits_nr, &last, &pending,
&seen, &reada, &nodes, &extent_cache);
+ if (ret < 0)
+ return 1;
if (ret != 0)
break;
}
--
1.6.2.1
--
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
On Fri, Oct 02, 2009 at 02:35:20PM +0900, TARUISI Hiroaki wrote:> Hi, > > I have tested btrfsck in some error case, such as item missing > case or CRC error case or so. In this test session I found some > bugs in btrfsck.c and disk-io.c in btrfs-progs. > 1. The last pair of ptrs and items are not checked at 2 place > of check_node/leaf. > 2. Csum printed CRC error message is not correct. > 3. In CRC error, btrfsck fails in segmentation fault. > > I''ll post three tiny patches to fix these bugs.Thank you for spending time in the btrfsck code. I''ll test these out and get them pushed into the btrfs-progs repository. -chris -- 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