On 18 January 2014 08:30, Gonglei (Arei) <arei.gong...@huawei.com> wrote:
> Ping?
>
>> -----Original Message-----
>> From: Gonglei (Arei)
>> Sent: Friday, January 10, 2014 4:59 PM
>> To: qemu-devel@nongnu.org
>> Cc: Luonengjun; Huangweidong (Hardware); chenliang (T)
>> Subject: [PATCH] migration: Fix free XBZRLE decoded_buf wrong
>>
>> Hi,
>>
>> When qemu do live migration with xbzrle, qemu malloc decoded_buf
>> at destniation end but free it at source end.It will crash qemu
>> by double free error in some scenarios.
>>
>> Signed-off-by: chenliang <chenlian...@huawei.com>
>> ---
>> arch_init.c | 9 ++++++++-
>> include/migration/migration.h | 1 +
>> migration.c | 1 +
>> 3 files changed, 10 insertions(+), 1 deletions(-)
>>
>> diff --git a/arch_init.c b/arch_init.c
>> index e0acbc5..0453f84 100644
>> --- a/arch_init.c
>> +++ b/arch_init.c
>> @@ -572,6 +572,14 @@ uint64_t ram_bytes_total(void)
>> return total;
>> }
>>
>> +void free_xbzrle_decoded_buf(void)
>> +{
>> + if (XBZRLE.decoded_buf) {
>> + g_free(XBZRLE.decoded_buf);
>> + XBZRLE.decoded_buf = NULL;
>> + }
g_free(NULL) is guaranteed to do nothing, so you don't
need the if() here.
>> +}
>> +
>> static void migration_end(void)
>> {
>> if (migration_bitmap) {
>> @@ -585,7 +593,6 @@ static void migration_end(void)
>> g_free(XBZRLE.cache);
>> g_free(XBZRLE.encoded_buf);
>> g_free(XBZRLE.current_buf);
>> - g_free(XBZRLE.decoded_buf);
>> XBZRLE.cache = NULL;
>> }
I wonder if it would be better to split the XBZRLE data structure
into part used by the source side and part used by the destination
side; the current arrangement looks extremely prone to bugs
like this one where somebody forgets that some of the fields
are not relevant to whichever of src/dst the code path they're
writing is used on.
thanks
-- PMM
