Re: [Qemu-devel] [PULL 11/13] migration: add postcopy migration of dirty bitmaps

[John Snow]

On 04/27/2018 09:22 AM, Peter Maydell wrote:
> On 13 March 2018 at 21:14, John Snow <address@hidden> wrote:
>> From: Vladimir Sementsov-Ogievskiy <address@hidden>
>>
>> Postcopy migration of dirty bitmaps. Only named dirty bitmaps are migrated.
>>
>> If destination qemu is already containing a dirty bitmap with the same name
>> as a migrated bitmap (for the same node), then, if their granularities are
>> the same the migration will be done, otherwise the error will be generated.
>>
>> If destination qemu doesn't contain such bitmap it will be created.
>>
>> Signed-off-by: Vladimir Sementsov-Ogievskiy <address@hidden>
>> Reviewed-by: Dr. David Alan Gilbert <address@hidden>
>> Message-id: address@hidden
>> [Changed '+' to '*' as per list discussion. --js]
>> Signed-off-by: John Snow <address@hidden>
> 
>> +static int init_dirty_bitmap_migration(void)
>> +{
> 
> Hi; Coverity (CID1390625) complains about a possible dereference
> after NULL check in this function:
> 
>> +    BlockDriverState *bs;
>> +    BdrvDirtyBitmap *bitmap;
>> +    DirtyBitmapMigBitmapState *dbms;
>> +    BdrvNextIterator it;
>> +
>> +    dirty_bitmap_mig_state.bulk_completed = false;
>> +    dirty_bitmap_mig_state.prev_bs = NULL;
>> +    dirty_bitmap_mig_state.prev_bitmap = NULL;
>> +    dirty_bitmap_mig_state.no_bitmaps = false;
>> +
>> +    for (bs = bdrv_first(&it); bs; bs = bdrv_next(&it)) {
>> +        const char *drive_name = bdrv_get_device_or_node_name(bs);
>> +
>> +        /* skip automatically inserted nodes */
>> +        while (bs && bs->drv && bs->implicit) {
>> +            bs = backing_bs(bs);
>> +        }
> 
> The 'bs' test in this while() loop implies that we might
> leave the loop because bs == NULL...
> 
>> +
>> +        for (bitmap = bdrv_dirty_bitmap_next(bs, NULL); bitmap;
> 
> ...but this call to bdrv_dirty_bitmap_next() will always
> dereference bs, so if it's NULL we'll crash.
> 
>> +             bitmap = bdrv_dirty_bitmap_next(bs, bitmap))
> 
> thanks
> -- PMM
> 

I have some patches on the way to clean up this file. Sorry for the delay.