On Wed, Jul 04, 2018 at 05:49:50PM +0800, Qu Wenruo wrote: > On 2018年07月04日 16:39, Gu, Jinxiang wrote: > >> -----Original Message----- > >> From: Nikolay Borisov [mailto:nborisov@xxxxxxxx] > >> Sent: Wednesday, July 04, 2018 4:15 PM > >> To: Gu, Jinxiang/顾 金香 <gujx@xxxxxxxxxxxxxx>; linux-btrfs@xxxxxxxxxxxxxxx > >> Cc: quwenruo.btrfs@xxxxxxx; wen.xu@xxxxxxxxxx; xuwen.sjtu@xxxxxxxxx > >> Subject: Re: [PATCH v2] btrfs: Add chunk type check in read a chunk > >> On 3.07.2018 09:20, Gu Jinxiang wrote: > >>> Reported in https://bugzilla.kernel.org/show_bug.cgi?id=199839, > >>> which has a invalid chunk, not return error opportunlly. > >>> > >>> Add chunk type check in btrfs_check_chunk_valid, > >>> to make error be returned in advance. > >>> > >>> Reported-by: Xu Wen <wen.xu@xxxxxxxxxx> > >>> Signed-off-by: Gu Jinxiang <gujx@xxxxxxxxxxxxxx> > >>> --- > >>> changelog: > >>> v2: > >>> deal with comment by Qu, add precise check for chunk type. > >>> > >>> fs/btrfs/volumes.c | 27 +++++++++++++++++++++++++++ > >>> 1 file changed, 27 insertions(+) > >>> > >>> diff --git a/fs/btrfs/volumes.c b/fs/btrfs/volumes.c > >>> index e034ad9e23b4..fcdb1a75d653 100644 > >>> --- a/fs/btrfs/volumes.c > >>> +++ b/fs/btrfs/volumes.c > >>> @@ -6401,6 +6401,8 @@ static int btrfs_check_chunk_valid(struct btrfs_fs_info *fs_info, > >>> u16 num_stripes; > >>> u16 sub_stripes; > >>> u64 type; > >>> + u64 features; > >>> + int mixed = 0; > >>> > >>> length = btrfs_chunk_length(leaf, chunk); > >>> stripe_len = btrfs_chunk_stripe_len(leaf, chunk); > >>> @@ -6439,6 +6441,31 @@ static int btrfs_check_chunk_valid(struct btrfs_fs_info *fs_info, > >>> btrfs_chunk_type(leaf, chunk)); > >>> return -EIO; > >>> } > >>> + > >>> + if (!(type & BTRFS_BLOCK_GROUP_TYPE_MASK)) { > >>> + btrfs_err(fs_info, "missing chunk type flag: %llu", type); > >>> + return -EIO; > >>> + } > >>> + > >>> + if ((type & BTRFS_BLOCK_GROUP_SYSTEM) && > >>> + (type & (BTRFS_BLOCK_GROUP_METADATA | BTRFS_BLOCK_GROUP_DATA))) { > >>> + btrfs_err(fs_info, "duplicate chunk type flag: %llu", type); > >> > >> This is not really a duplicate chunk type flag but instead an invalid > >> combination. SYSTEM is always a dedicated chunk, rephrase the error > >> message. > > > > Yes. This message should be updated. > > But [PATCH 1/5] btrfs: tree-checker: Verify block_group_item sent by Qu, checks the chunk type > > in tree checker, which be called when read a bio. So this patch should not be merged. > > Nope, this patch is still needed. > > Although I added block group -> chunk checker, and it indeed detects all > the crafted images, doing chunk verification is nothing wrong. > > If chunk verification can detect anything wrong, we can error out even > earlier. > > So I still think this patch is needed. > (Although I'd like to move a lot of chunk verification to tree-checker > later) I'd rather add that to the tree-checker.c now and the enhanced checks. btrfs_check_chunk_valid is self-contained and does not use anything that's local to volumes.c. -- To unsubscribe from this list: send the line "unsubscribe linux-btrfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
