Re: [Qemu-devel] [PATCH v4 05/10] qcow2: Fix refcount blocks beyond image end

[Max Reitz <mreitz@redhat.com>]

On 29.08.2014 21:50, Eric Blake wrote:
> On 08/27/2014 02:18 PM, Max Reitz wrote:
>> If the qcow2 check function detects a refcount block located beyond the
>> image end, grow the image appropriately. This cannot break anything and
>> is the logical fix for such a case.
>>
>> Signed-off-by: Max Reitz <mreitz@redhat.com>
>> ---
>>   block/qcow2-refcount.c | 62 ++++++++++++++++++++++++++++++++++++++++++++++----
>>   1 file changed, 58 insertions(+), 4 deletions(-)
>>
>> diff --git a/block/qcow2-refcount.c b/block/qcow2-refcount.c
>> index babe6cb..394a402 100644
>> --- a/block/qcow2-refcount.c
>> +++ b/block/qcow2-refcount.c
>> @@ -1505,7 +1505,8 @@ static int check_refblocks(BlockDriverState *bs, BdrvCheckResult *res,
>>                              int64_t *nb_clusters)
>>   {
>>       BDRVQcowState *s = bs->opaque;
>> -    int64_t i;
>> +    int64_t i, size;
>> +    int ret;
>>   
>>       for(i = 0; i < s->refcount_table_size; i++) {
> Is it worth fixing up the whitespace on this 'for' at any point in the
> series?

In v1 of this series I fixed several preexisting coding style issues. 
However, when Benoît requested smaller diffs (and keep the coding style 
fixes outside of the code moving patches), I decided to throw all of 
those out. I would have to write an explicit coding style fix patch, but 
it turned out there are a lot of style issues in qcow2-refcount.c.

Max

>> +            if (fix & BDRV_FIX_ERRORS) {
>> +                int64_t old_nb_clusters = *nb_clusters;
>> +
>> +                if (offset + s->cluster_size < offset ||
>> +                    offset + s->cluster_size > INT64_MAX)
> [1]
>
>> +
>> +                *refcount_table = g_try_realloc(*refcount_table,
>> +                        *nb_clusters * sizeof(uint16_t));
> I was about to complain that this multiply could overflow if
> *nb_clusters is more than 2**62 bits, until I double checked that due to
> the limit checking at [1], we know *nb_clusters is narrower.
>
> Reviewed-by: Eric Blake <eblake@redhat.com>
>