[PATCH] btrfs-progs: dump-tree: add extra chunk type checks

[PATCH] btrfs-progs: dump-tree: add extra chunk type checks

[Qu Wenruo]

There is a report in the mailing list where a seemingly confused end
user is passing a physical bytenr into "btrfs inspect dump-tree".

The best case is that bytenr has no corresponding chunk and will be
rejected early.

But if there is a chunk cover that bytenr, and that chunk is a data
chunk, "btrfs ins dump-tree" will mostly result a checksum mismatch,
e.g.:

 $ btrfs ins dump-tree -b 13631488 ~/test.img
 btrfs-progs v6.19
 checksum verify failed on 13631488 wanted 0xcdcdcdcd found 0xf9d338a0
 ERROR: failed to read tree block 13631488

Note that 13631488 is the logical bytenr of the first data chunk.

This can be confusing for end users, as they won't know if it's really
some corrupted tree blocks, or they are just passing wrong numbers.

Enhance this by introducing a new check_metadata_logical() helper, to
make sure a chunk exists and the type is also correct.

Now the error would be more obvious to end users:

 $ ./btrfs ins dump-tree -b 13631488 ~/test.img
 btrfs-progs v6.19
 ERROR: logical 13631488 is not in a metadata chunk, found chunk 13631488 len 8388608 flags 0x1

Link: https://lore.kernel.org/linux-btrfs/77535c20-da3e-4dfb-b3d7-9426c25b5da3@free.fr/
Signed-off-by: Qu Wenruo <wqu@suse.com>
---
 cmds/inspect-dump-tree.c | 24 ++++++++++++++++++++++++
 1 file changed, 24 insertions(+)

diff --git a/cmds/inspect-dump-tree.c b/cmds/inspect-dump-tree.c
index 2bc3f68179a5..11eb950dbdcb 100644
--- a/cmds/inspect-dump-tree.c
+++ b/cmds/inspect-dump-tree.c
@@ -33,6 +33,7 @@
 #include "kernel-shared/print-tree.h"
 #include "kernel-shared/extent_io.h"
 #include "kernel-shared/tree-checker.h"
+#include "kernel-shared/volumes.h"
 #include "common/defs.h"
 #include "common/extent-cache.h"
 #include "common/messages.h"
@@ -194,6 +195,26 @@ static int dump_add_tree_block(struct cache_tree *tree, u64 bytenr)
 	return ret;
 }
 
+static int check_metadata_logical(struct btrfs_fs_info *fs_info, u64 logical)
+{
+	struct cache_extent *ce;
+	struct map_lookup *map;
+
+	ce = search_cache_extent(&fs_info->mapping_tree.cache_tree, logical);
+	if (!ce || ce->start > logical) {
+		error("no chunk map found for logical %llu", logical);
+		return -ENOENT;
+	}
+	map = container_of(ce, struct map_lookup, ce);
+	if (!(map->type & BTRFS_BLOCK_GROUP_METADATA)) {
+		error(
+"logical %llu is not in a metadata chunk, found chunk %llu len %llu flags 0x%llx",
+		      logical, ce->start, ce->size, map->type);
+		return -EINVAL;
+	}
+	return 0;
+}
+
 /*
  * Print all tree blocks recorded.
  * All tree block bytenr record will also be freed in this function.
@@ -228,6 +249,9 @@ static int dump_print_tree_blocks(struct btrfs_fs_info *fs_info,
 			goto next;
 		}
 
+		ret = check_metadata_logical(fs_info, bytenr);
+		if (ret < 0)
+			goto next;
 		eb = read_tree_block(fs_info, bytenr, &check);
 		if (!extent_buffer_uptodate(eb)) {
 			error("failed to read tree block %llu", bytenr);
-- 
2.53.0

Re: [PATCH] btrfs-progs: dump-tree: add extra chunk type checks

[Sun YangKai]

On 2026/2/23 06:14, Qu Wenruo wrote:
> There is a report in the mailing list where a seemingly confused end
> user is passing a physical bytenr into "btrfs inspect dump-tree".
> 
> The best case is that bytenr has no corresponding chunk and will be
> rejected early.
> 
> But if there is a chunk cover that bytenr, and that chunk is a data
> chunk, "btrfs ins dump-tree" will mostly result a checksum mismatch,
> e.g.:
> 
>   $ btrfs ins dump-tree -b 13631488 ~/test.img
>   btrfs-progs v6.19
>   checksum verify failed on 13631488 wanted 0xcdcdcdcd found 0xf9d338a0
>   ERROR: failed to read tree block 13631488
> 
> Note that 13631488 is the logical bytenr of the first data chunk.
> 
> This can be confusing for end users, as they won't know if it's really
> some corrupted tree blocks, or they are just passing wrong numbers.
> 
> Enhance this by introducing a new check_metadata_logical() helper, to
> make sure a chunk exists and the type is also correct.
> 
> Now the error would be more obvious to end users:
> 
>   $ ./btrfs ins dump-tree -b 13631488 ~/test.img
>   btrfs-progs v6.19
>   ERROR: logical 13631488 is not in a metadata chunk, found chunk 13631488 len 8388608 flags 0x1

Would it make sense to use something like `type DATA` instead of
`flags 0x1` in the error message? I believe this would be more helpful 
for users.

Thanks,
Sun Yangkai

> 
> Link: https://lore.kernel.org/linux-btrfs/77535c20-da3e-4dfb-b3d7-9426c25b5da3@free.fr/
> Signed-off-by: Qu Wenruo <wqu@suse.com>
> ---
>   cmds/inspect-dump-tree.c | 24 ++++++++++++++++++++++++
>   1 file changed, 24 insertions(+)
> 
> diff --git a/cmds/inspect-dump-tree.c b/cmds/inspect-dump-tree.c
> index 2bc3f68179a5..11eb950dbdcb 100644
> --- a/cmds/inspect-dump-tree.c
> +++ b/cmds/inspect-dump-tree.c
> @@ -33,6 +33,7 @@
>   #include "kernel-shared/print-tree.h"
>   #include "kernel-shared/extent_io.h"
>   #include "kernel-shared/tree-checker.h"
> +#include "kernel-shared/volumes.h"
>   #include "common/defs.h"
>   #include "common/extent-cache.h"
>   #include "common/messages.h"
> @@ -194,6 +195,26 @@ static int dump_add_tree_block(struct cache_tree *tree, u64 bytenr)
>   	return ret;
>   }
>   
> +static int check_metadata_logical(struct btrfs_fs_info *fs_info, u64 logical)
> +{
> +	struct cache_extent *ce;
> +	struct map_lookup *map;
> +
> +	ce = search_cache_extent(&fs_info->mapping_tree.cache_tree, logical);
> +	if (!ce || ce->start > logical) {
> +		error("no chunk map found for logical %llu", logical);
> +		return -ENOENT;
> +	}
> +	map = container_of(ce, struct map_lookup, ce);
> +	if (!(map->type & BTRFS_BLOCK_GROUP_METADATA)) {
> +		error(
> +"logical %llu is not in a metadata chunk, found chunk %llu len %llu flags 0x%llx",
> +		      logical, ce->start, ce->size, map->type);
> +		return -EINVAL;
> +	}
> +	return 0;
> +}
> +
>   /*
>    * Print all tree blocks recorded.
>    * All tree block bytenr record will also be freed in this function.
> @@ -228,6 +249,9 @@ static int dump_print_tree_blocks(struct btrfs_fs_info *fs_info,
>   			goto next;
>   		}
>   
> +		ret = check_metadata_logical(fs_info, bytenr);
> +		if (ret < 0)
> +			goto next;
>   		eb = read_tree_block(fs_info, bytenr, &check);
>   		if (!extent_buffer_uptodate(eb)) {
>   			error("failed to read tree block %llu", bytenr);