On 9.04.2018 10:47, Qu Wenruo wrote: > When manually patching super blocks, current validation check is pretty > weak (limited to magic number and csum) and doesn't provide extra check > for some obvious corruption (like invalid sectorsize). > > This patch will enhance dump-super by: > > 1) Refactor print function and add checker helper macro > Instead of manually call printf() and takes 2 lines to just print one > member, introduce several helper macros to print/check value. > Most of the callers just need 1 line. > (Although the macro and helper itself offsets the saved lines) > > 2) Add extra check for some members > The following members will be checked, and if invalid the suffix > " [INVALID]" will be pended: > bytenr: alignment check > sys_array_size: maximum check > root: alignment check > root_level: maximum check > chunk_root: alignment check > chunk_root_level: maximum check > chunk_root_generation: maximum check against root generation > log_root: alignment check > log_root_level: maximum check > log_root_transid: maximum check against root generation + 1 > bytes_used: alignment check > sectorsize: power of 2 and 4K~64K range check > nodesize: the same as sectorsize, plus minimum check > stripesize: the same as kernel check > cache_generation: maximum check against root generation > uuid_tree_generation: maximum check against root generation > > 3) output sequence change > Put bytenr/level/generation together for the same root (tree root, > chunk root, and log root). > > 4) Minor output change > To make dev_item macro easier, the following output is changed: > dev_item.devid -> dev_item.id > dev_item.dev_group -> dev_item.group > > Signed-off-by: Qu Wenruo <w...@suse.com> > --- > cmds-inspect-dump-super.c | 168 ++++++++++++++++++++++---------------- > 1 file changed, 99 insertions(+), 69 deletions(-) > > diff --git a/cmds-inspect-dump-super.c b/cmds-inspect-dump-super.c > index 24588c87cce6..8000d2ace663 100644 > --- a/cmds-inspect-dump-super.c > +++ b/cmds-inspect-dump-super.c > @@ -312,11 +312,42 @@ static void print_readable_super_flag(u64 flag) > super_flags_num, BTRFS_SUPER_FLAG_SUPP); > } > > +/* Helper to print member in %llu */ > +#define print_super(sb, member, spacer) > \ > + printf("%s%s%llu\n", #member, spacer, (u64) btrfs_super_##member(sb)); > + > +/* Helper to print member in %llx */ > +#define print_super_hex(sb, member, spacer) \ > + printf("%s%s%llx\n", #member, spacer, (u64) btrfs_super_##member(sb));
nit: I like the idea of the patch, however, I think the presence of the "spacer" argument is a bit annoying. Why don't you make all the print out to be aligned to the same level i.e hardcode \t\t\ or \t\t or whatever? > + > +/* > + * Helper macro to wrap member checker > + * > + * Only support %llu output for member. > + */ > +#define print_check_super(sb, member, spacer, bad_condition) \ > +({ \ > + u64 value; \ > + \ > + value = btrfs_super_##member(sb); \ > + printf("%s%s%llu", #member, spacer, (u64) value); \ > + if (bad_condition) \ > + printf(" [INVALID]"); \ > + printf("\n"); \ > +}) > + > +/* Helper to print sb->dev_item members */ > +#define print_super_dev(sb, member, spacer) \ > + printf("dev_item.%s%s%llu\n", #member, spacer, \ > + (u64) btrfs_stack_device_##member(&sb->dev_item)); > + > static void dump_superblock(struct btrfs_super_block *sb, int full) > { > int i; > char *s, buf[BTRFS_UUID_UNPARSED_SIZE]; > + int max_level = BTRFS_MAX_LEVEL; /* to save several chars */ > u8 *p; > + u64 super_gen; > u32 csum_size; > u16 csum_type; > > @@ -348,10 +379,16 @@ static void dump_superblock(struct btrfs_super_block > *sb, int full) > printf(" [DON'T MATCH]"); > putchar('\n'); > > - printf("bytenr\t\t\t%llu\n", > - (unsigned long long)btrfs_super_bytenr(sb)); > - printf("flags\t\t\t0x%llx\n", > - (unsigned long long)btrfs_super_flags(sb)); > + /* > + * Use btrfs minimal sector size as basic check for bytenr, since we > + * can't trust sector size from super block. > + * This 4K check should works fine for most architecture, and will be > + * just a little loose for 64K page system. > + * > + * And such 4K check will be used for other members too. > + */ > + print_check_super(sb, bytenr, "\t\t\t", (!IS_ALIGNED(value, SZ_4K))); > + print_super_hex(sb, flags, "\t\t\t"); > print_readable_super_flag(btrfs_super_flags(sb)); > > printf("magic\t\t\t"); > @@ -372,53 +409,56 @@ static void dump_superblock(struct btrfs_super_block > *sb, int full) > putchar(isprint(s[i]) ? s[i] : '.'); > putchar('\n'); > > - printf("generation\t\t%llu\n", > - (unsigned long long)btrfs_super_generation(sb)); > - printf("root\t\t\t%llu\n", (unsigned long long)btrfs_super_root(sb)); > - printf("sys_array_size\t\t%llu\n", > - (unsigned long long)btrfs_super_sys_array_size(sb)); > - printf("chunk_root_generation\t%llu\n", > - (unsigned long long)btrfs_super_chunk_root_generation(sb)); > - printf("root_level\t\t%llu\n", > - (unsigned long long)btrfs_super_root_level(sb)); > - printf("chunk_root\t\t%llu\n", > - (unsigned long long)btrfs_super_chunk_root(sb)); > - printf("chunk_root_level\t%llu\n", > - (unsigned long long)btrfs_super_chunk_root_level(sb)); > - printf("log_root\t\t%llu\n", > - (unsigned long long)btrfs_super_log_root(sb)); > - printf("log_root_transid\t%llu\n", > - (unsigned long long)btrfs_super_log_root_transid(sb)); > - printf("log_root_level\t\t%llu\n", > - (unsigned long long)btrfs_super_log_root_level(sb)); > - printf("total_bytes\t\t%llu\n", > - (unsigned long long)btrfs_super_total_bytes(sb)); > - printf("bytes_used\t\t%llu\n", > - (unsigned long long)btrfs_super_bytes_used(sb)); > - printf("sectorsize\t\t%llu\n", > - (unsigned long long)btrfs_super_sectorsize(sb)); > - printf("nodesize\t\t%llu\n", > - (unsigned long long)btrfs_super_nodesize(sb)); > + print_check_super(sb, sys_array_size, "\t\t", > + (value > BTRFS_SYSTEM_CHUNK_ARRAY_SIZE)); > + > + super_gen = btrfs_super_generation(sb); > + print_check_super(sb, root, "\t\t\t", (!IS_ALIGNED(value, SZ_4K))); > + print_check_super(sb, root_level, "\t\t", (value >= max_level)); > + print_super(sb, generation, "\t\t"); > + > + print_check_super(sb, chunk_root, "\t\t", (!IS_ALIGNED(value, SZ_4K))); > + print_check_super(sb, chunk_root_level, "\t", (value >= max_level)); > + /* > + * Here we trust super generation, and use it as checker for other > + * tree roots. Applies to all other trees. > + */ > + print_check_super(sb, chunk_root_generation, "\t", (value > super_gen)); > + > + print_check_super(sb, log_root, "\t\t", (!IS_ALIGNED(value, SZ_4K))); > + print_check_super(sb, log_root_level, "\t\t", (value >= max_level)); > + print_check_super(sb, log_root_transid, "\t", (value > super_gen + 1)); > + > + /* > + * For total bytes, it's possible that old kernel is using unaligned > + * size, not critical so don't do 4K check here. > + */ > + print_super(sb, total_bytes, "\t\t"); > + > + /* Used bytes must be aligned to 4K */ > + print_check_super(sb, bytes_used, "\t\t", (!IS_ALIGNED(value, SZ_4K))); > + print_check_super(sb, sectorsize, "\t\t", > + (!(is_power_of_2(value) && value >= SZ_4K && > + value <= SZ_64K))); > + print_check_super(sb, nodesize, "\t\t", > + (!(is_power_of_2(value) && value >= SZ_4K && > + value <= SZ_64K && > + value > btrfs_super_sectorsize(sb)))); > printf("leafsize (deprecated)\t%u\n", > le32_to_cpu(sb->__unused_leafsize)); > - printf("stripesize\t\t%llu\n", > - (unsigned long long)btrfs_super_stripesize(sb)); > - printf("root_dir\t\t%llu\n", > - (unsigned long long)btrfs_super_root_dir(sb)); > - printf("num_devices\t\t%llu\n", > - (unsigned long long)btrfs_super_num_devices(sb)); > - printf("compat_flags\t\t0x%llx\n", > - (unsigned long long)btrfs_super_compat_flags(sb)); > - printf("compat_ro_flags\t\t0x%llx\n", > - (unsigned long long)btrfs_super_compat_ro_flags(sb)); > + > + /* Not really used, just check it the same way as kernel */ > + print_check_super(sb, stripesize, "\t\t", (!is_power_of_2(value))); > + print_super(sb, root_dir, "\t\t"); > + print_super(sb, num_devices, "\t\t"); > + print_super_hex(sb, compat_flags, "\t\t"); > + print_super_hex(sb, compat_ro_flags, "\t\t"); > print_readable_compat_ro_flag(btrfs_super_compat_ro_flags(sb)); > - printf("incompat_flags\t\t0x%llx\n", > - (unsigned long long)btrfs_super_incompat_flags(sb)); > + print_super_hex(sb, incompat_flags, "\t\t"); > print_readable_incompat_flag(btrfs_super_incompat_flags(sb)); > - printf("cache_generation\t%llu\n", > - (unsigned long long)btrfs_super_cache_generation(sb)); > - printf("uuid_tree_generation\t%llu\n", > - (unsigned long long)btrfs_super_uuid_tree_generation(sb)); > + print_check_super(sb, cache_generation, "\t", > + (value > super_gen && value != (u64)-1)); > + print_check_super(sb, uuid_tree_generation, "\t", (value > super_gen)); > > uuid_unparse(sb->dev_item.uuid, buf); > printf("dev_item.uuid\t\t%s\n", buf); > @@ -428,28 +468,18 @@ static void dump_superblock(struct btrfs_super_block > *sb, int full) > !memcmp(sb->dev_item.fsid, sb->fsid, BTRFS_FSID_SIZE) ? > "[match]" : "[DON'T MATCH]"); > > - printf("dev_item.type\t\t%llu\n", (unsigned long long) > - btrfs_stack_device_type(&sb->dev_item)); > - printf("dev_item.total_bytes\t%llu\n", (unsigned long long) > - btrfs_stack_device_total_bytes(&sb->dev_item)); > - printf("dev_item.bytes_used\t%llu\n", (unsigned long long) > - btrfs_stack_device_bytes_used(&sb->dev_item)); > - printf("dev_item.io_align\t%u\n", (unsigned int) > - btrfs_stack_device_io_align(&sb->dev_item)); > - printf("dev_item.io_width\t%u\n", (unsigned int) > - btrfs_stack_device_io_width(&sb->dev_item)); > - printf("dev_item.sector_size\t%u\n", (unsigned int) > - btrfs_stack_device_sector_size(&sb->dev_item)); > - printf("dev_item.devid\t\t%llu\n", > - btrfs_stack_device_id(&sb->dev_item)); > - printf("dev_item.dev_group\t%u\n", (unsigned int) > - btrfs_stack_device_group(&sb->dev_item)); > - printf("dev_item.seek_speed\t%u\n", (unsigned int) > - btrfs_stack_device_seek_speed(&sb->dev_item)); > - printf("dev_item.bandwidth\t%u\n", (unsigned int) > - btrfs_stack_device_bandwidth(&sb->dev_item)); > - printf("dev_item.generation\t%llu\n", (unsigned long long) > - btrfs_stack_device_generation(&sb->dev_item)); > + /* For embedded device item, don't do extra check, just like kernel */ > + print_super_dev(sb, type, "\t\t"); > + print_super_dev(sb, total_bytes, "\t"); > + print_super_dev(sb, bytes_used, "\t"); > + print_super_dev(sb, io_align, "\t"); > + print_super_dev(sb, io_width, "\t"); > + print_super_dev(sb, sector_size, "\t"); > + print_super_dev(sb, id, "\t"); > + print_super_dev(sb, group, "\t"); > + print_super_dev(sb, seek_speed, "\t"); > + print_super_dev(sb, bandwidth, "\t"); > + print_super_dev(sb, generation, "\t"); > if (full) { > printf("sys_chunk_array[%d]:\n", BTRFS_SYSTEM_CHUNK_ARRAY_SIZE); > print_sys_chunk_array(sb); > -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html