Re: [Qemu-devel] [PATCH] vdi: Fix memory leak

[Kevin Wolf <kwolf@redhat.com>]

Am 22.11.2011 19:37, schrieb Stefan Weil:
> Am 22.11.2011 17:16, schrieb Kevin Wolf:
>> The block map is allocated in vdi_open, but was never freed.
>>
>> Signed-off-by: Kevin Wolf<kwolf@redhat.com>
>> ---
>> Applies on top if the migration blocker series.
>>
>>   block/vdi.c |    3 +++
>>   1 files changed, 3 insertions(+), 0 deletions(-)
>>
>> diff --git a/block/vdi.c b/block/vdi.c
>> index 7dda522..02da6b4 100644
>> --- a/block/vdi.c
>> +++ b/block/vdi.c
>> @@ -949,6 +949,9 @@ static int vdi_create(const char *filename, QEMUOptionParameter *options)
>>   static void vdi_close(BlockDriverState *bs)
>>   {
>>       BDRVVdiState *s = bs->opaque;
>> +
>> +    g_free(s->bmap);
>> +
>>       migrate_del_blocker(s->migration_blocker);
>>       error_free(s->migration_blocker);
>>   }
> 
> If vdi_close is called after a jump to label fail_free_bmap,
> g_free(s->bmap) will be called twice.

bdrv_close() may only ever be called on a successfully opened image.

In fact, block.c already ensures this. The BDRVVdiState would already be
freed in the failing bdrv_open_common() and bs->drv would be set to
NULL, so that an invalid bdrv_close() would end up doing nothing.

> Setting s->bmap = NULL after g_free in fail_free_bmap
> would be safer.
> 
> Otherwise your patch is fine. If you send an update, you
> can add
> 
> Reviewed-by: Stefan Weil <sw@weilnetz.de>

Thanks.

Kevin