[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH for-2.11 4/5] qcow2: Add bounds check to get_ref
From: |
Max Reitz |
Subject: |
Re: [Qemu-devel] [PATCH for-2.11 4/5] qcow2: Add bounds check to get_refblock_offset() |
Date: |
Tue, 14 Nov 2017 16:40:55 +0100 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.4.0 |
On 2017-11-14 16:38, Alberto Garcia wrote:
> On Tue 14 Nov 2017 04:27:56 PM CET, Max Reitz wrote:
>>>> +static int64_t get_refblock_offset(BlockDriverState *bs, uint64_t offset)
>>>> +{
>>>> + BDRVQcow2State *s = bs->opaque;
>>>> + uint32_t index = offset_to_reftable_index(s, offset);
>>>> + int64_t covering_refblock_offset = 0;
>>>> +
>>>> + if (index < s->refcount_table_size) {
>>>> + covering_refblock_offset = s->refcount_table[index] &
>>>> REFT_OFFSET_MASK;
>>>> + }
>>>> + if (!covering_refblock_offset) {
>>>> + qcow2_signal_corruption(bs, true, -1, -1, "Refblock at %#" PRIx64
>>>> " is "
>>>> + "not covered by the refcount structures",
>>>> + offset);
>>>> + return -EIO;
>>>> + }
>>>> +
>>>> + return covering_refblock_offset;
>>>> +}
>>>
>>> Isn't it simpler to do something like this instead?
>>>
>>> if (index >= s->refcount_table_size) {
>>> qcow2_signal_corruption(...);
>>> return -EIO;
>>> }
>>> return s->refcount_table[index] & REFT_OFFSET_MASK;
>>
>> But that doesn't cover the case were s->refcount_table[index] &
>> REFT_OFFSET_MASK is 0.
>
> Ah, you're right. That would be detected by the qcow2_cache_get() call
> though, but it's fine to do the check here as well.
After patch 5, yes, but it would lead to a failed assertion.
Before this series, there is no protection in place; the cache will
happily serve you any empty entry (because offset 0 is used for empty
entries) and pretend it's the correct block.
Only when you try to dirty it is when you run into problems (because
then you'll get an assertion failure).
Max
> Reviewed-by: Alberto Garcia <address@hidden>
>
> Berto
>
signature.asc
Description: OpenPGP digital signature
- Re: [Qemu-devel] [PATCH for-2.11 2/5] qcow2: Unaligned zero cluster in handle_alloc(), (continued)
[Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache, Max Reitz, 2017/11/10
Re: [Qemu-devel] [PATCH for-2.11 5/5] qcow2: Refuse to get unaligned offsets from cache, Alberto Garcia, 2017/11/14
Re: [Qemu-devel] [PATCH for-2.11 0/5] qcow2: Fixes for corrupted images, Max Reitz, 2017/11/10
Re: [Qemu-devel] [PATCH for-2.11 0/5] qcow2: Fixes for corrupted images, Max Reitz, 2017/11/15