On Tue, Jul 16, 2019 at 05:00:34PM +0800, Qu Wenruo wrote: > This patch will introduce ROOT_ITEM check, which includes: > - Key->objectid and key->offset check > Currently only some easy check, e.g. 0 as rootid is invalid. > > - Item size check > Root item size is fixed. > > - Generation checks > Generation, v2_generaetion and last_snapshot should not pass super > generation + 1 > > - Level and alignment check > Level should be in [0, 7], and bytenr must be aligned to sector size. > > - Flags check
Nice. I found some small things that I can fix, no need to resend. > Link: https://bugzilla.kernel.org/show_bug.cgi?id=203261 > Signed-off-by: Qu Wenruo <w...@suse.com> > --- > fs/btrfs/tree-checker.c | 92 +++++++++++++++++++++++++++++++++++++++++ > 1 file changed, 92 insertions(+) > > diff --git a/fs/btrfs/tree-checker.c b/fs/btrfs/tree-checker.c > index a4c7f7ed8490..71bbbce3076d 100644 > --- a/fs/btrfs/tree-checker.c > +++ b/fs/btrfs/tree-checker.c > @@ -810,6 +810,95 @@ static int check_inode_item(struct extent_buffer *leaf, > return 0; > } > > +static int check_root_item(struct extent_buffer *leaf, struct btrfs_key *key, > + int slot) > +{ > + struct btrfs_fs_info *fs_info = leaf->fs_info; > + struct btrfs_root_item ri; > + u64 valid_root_flags = BTRFS_ROOT_SUBVOL_RDONLY | > + BTRFS_ROOT_SUBVOL_DEAD; You can use 'const' here. > + > + /* No Such Tree id*/ > + if (key->objectid == 0) { > + generic_err(leaf, slot, "invalid root id 0"); > + return -EUCLEAN; > + } > + > + /* > + * Some older kernel may create ROOT_ITEM with non-zero offset, > + * so here we only check offset for reloc tree whose key->offset > + * must be a valid tree. > + */ > + if (key->objectid == BTRFS_TREE_RELOC_OBJECTID && key->offset == 0) { > + generic_err(leaf, slot, "invalid root id 0 for reloc tree"); > + return -EUCLEAN; > + } > + > + if (btrfs_item_size_nr(leaf, slot) != sizeof(ri)) { > + generic_err(leaf, slot, > + "invalid root item size, have %u expect %lu", > + btrfs_item_size_nr(leaf, slot), sizeof(ri)); sizeof needs %zu > + } > + > + read_extent_buffer(leaf, &ri, btrfs_item_ptr_offset(leaf, slot), > + sizeof(ri)); > + > + /* Generateion related */ typo here and a few more times below > + if (btrfs_root_generation(&ri) > > + btrfs_super_generation(fs_info->super_copy) + 1) { > + generic_err(leaf, slot, > + "invalid root generaetion, have %llu expect (0, %llu]", > + btrfs_root_generation(&ri), > + btrfs_super_generation(fs_info->super_copy) + 1); > + return -EUCLEAN; > + } > + if (btrfs_root_generation_v2(&ri) > > + btrfs_super_generation(fs_info->super_copy) + 1) { > + generic_err(leaf, slot, > + "invalid root v2 generaetion, have %llu expect (0, %llu]", So (0, %llu] here means that it must be greater than zero, right? I'm not sure that everyone uses the same notation for open/closed notation. > + btrfs_root_generation_v2(&ri), > + btrfs_super_generation(fs_info->super_copy) + 1); > + return -EUCLEAN; > + } > + if (btrfs_root_last_snapshot(&ri) > > + btrfs_super_generation(fs_info->super_copy) + 1) { > + generic_err(leaf, slot, > + "invalid root last_snapshot, have %llu expect (0, %llu]", > + btrfs_root_last_snapshot(&ri), > + btrfs_super_generation(fs_info->super_copy) + 1); > + return -EUCLEAN; > + } > + > + /* Alignment and level check */ > + if (!IS_ALIGNED(btrfs_root_bytenr(&ri), fs_info->sectorsize)) { > + generic_err(leaf, slot, > + "invalid root bytenr, have %llu expect to be aligned to > %u", > + btrfs_root_bytenr(&ri), fs_info->sectorsize); > + return -EUCLEAN; > + } > + if (btrfs_root_level(&ri) >= BTRFS_MAX_LEVEL) { > + generic_err(leaf, slot, > + "invalid root level, have %u expect [0, %u]", > + btrfs_root_level(&ri), BTRFS_MAX_LEVEL - 1); > + return -EUCLEAN; > + } > + if (ri.drop_level >= BTRFS_MAX_LEVEL) { > + generic_err(leaf, slot, > + "invalid root level, have %u expect [0, %u]", > + ri.drop_level, BTRFS_MAX_LEVEL - 1); > + return -EUCLEAN; > + } > + > + /* Flags check */ > + if (btrfs_root_flags(&ri) & ~valid_root_flags) { > + generic_err(leaf, slot, > + "invalid root flags, have 0x%llx expect mask 0x%llu", 0x%llx > + btrfs_root_flags(&ri), valid_root_flags); > + return -EUCLEAN; > + } > + return 0; > +} > + > /* > * Common point to switch the item-specific validation. > */ > @@ -845,6 +934,9 @@ static int check_leaf_item(struct extent_buffer *leaf, > case BTRFS_INODE_ITEM_KEY: > ret = check_inode_item(leaf, key, slot); > break; > + case BTRFS_ROOT_ITEM_KEY: > + ret = check_root_item(leaf, key, slot); > + break; > } > return ret; > } > -- > 2.22.0
