On 2018年07月04日 16:39, Gu, Jinxiang wrote:
>
>
>> -----Original Message-----
>> From: Nikolay Borisov [mailto:nbori...@suse.com]
>> Sent: Wednesday, July 04, 2018 4:15 PM
>> To: Gu, Jinxiang/顾 金香 <g...@cn.fujitsu.com>; linux-btrfs@vger.kernel.org
>> Cc: quwenruo.bt...@gmx.com; wen...@gatech.edu; xuwen.s...@gmail.com
>> 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...@gatech.edu>
>>> Signed-off-by: Gu Jinxiang <g...@cn.fujitsu.com>
>>> ---
>>> 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)
Thanks,
Qu
>
>>
>>> + return -EIO;
>>> + }
>>> +
>>> + features = btrfs_super_incompat_flags(fs_info->super_copy);
>>> + if (features & BTRFS_FEATURE_INCOMPAT_MIXED_GROUPS)
>>> + mixed = 1;
>>> +
>>> + if (!mixed) {
>>> + if (type &
>>> + (BTRFS_BLOCK_GROUP_METADATA & BTRFS_BLOCK_GROUP_DATA)) {
>>> + btrfs_err(fs_info,
>>> + "mixed chunk type flag in non-mixed mode: %llu", type);
>>> + return -EIO;
>>> + }
>>> + }
>>> +
>>> if ((type & BTRFS_BLOCK_GROUP_RAID10 && sub_stripes != 2) ||
>>> (type & BTRFS_BLOCK_GROUP_RAID1 && num_stripes < 1) ||
>>> (type & BTRFS_BLOCK_GROUP_RAID5 && num_stripes < 2) ||
>>>
>>
>
>
>
> N�����r��y���b�X��ǧv�^�){.n�+����{�n�߲)���w* jg��� �����ݢj/���z�ޖ��2�ޙ���&�)ߡ�a���� �G���h��j:+v���w�٥
>
--
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
