[Qemu-devel] [PATCH v2] cpu hot-del: leak fix by free the relevant members

[liujunjie]

THese leaks are found by ASAN with CPU hot-add and hot-del actions,
such as:
==14127==ERROR: LeakSanitizer: detected memory leaks

Direct leak of 4096 byte(s) in 1 object(s) allocated from:
    #0 0x7fc321cb6ec0 in posix_memalign 
(/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7ec0)
    #1 0xf756b9 in qemu_try_memalign util/oslib-posix.c:110
    #2 0xf7575b in qemu_memalign util/oslib-posix.c:126
    #3 0x7769cb in kvm_arch_init_vcpu /root/qemu/target/i386/kvm.c:1103
    #4 0x4cc4e6 in qemu_kvm_cpu_thread_fn /root/qemu/cpus.c:1201
    #5 0x7fc31cb28dc4 in start_thread (/usr/lib64/libpthread.so.0+0x7dc4)

Direct leak of 4096 byte(s) in 1 object(s) allocated from:
    #0 0x7fc321cb6560 in calloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7560)
    #1 0x7fc31cf944d6 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x4f4d6)
    #2 0x4cc4e6 in qemu_kvm_cpu_thread_fn /root/qemu/cpus.c:1201
    #3 0x7fc31cb28dc4 in start_thread (/usr/lib64/libpthread.so.0+0x7dc4)

Direct leak of 184 byte(s) in 1 object(s) allocated from:
    #0 0x7fc321cb6560 in calloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7560)
    #1 0x7fc31cf944d6 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x4f4d6)
    #2 0x4cda83 in qemu_init_vcpu /root/qemu/cpus.c:1993
    #3 0x6aedbd in x86_cpu_realizefn /root/qemu/target/i386/cpu.c:4739
    #4 0x8edfc8 in device_set_realized hw/core/qdev.c:826
    #5 0xcff60c in property_set_bool qom/object.c:1928
    #6 0xd09bd2 in object_property_set_qobject qom/qom-qobject.c:27
    #7 0xd048e3 in object_property_set_bool qom/object.c:1188
    #8 0x79ad83 in qdev_device_add /root/qemu/qdev-monitor.c:627
    #9 0x79b50c in qmp_device_add /root/qemu/qdev-monitor.c:807
    #10 0xf4ca5d in do_qmp_dispatch qapi/qmp-dispatch.c:119
    #11 0xf4ce3c in qmp_dispatch qapi/qmp-dispatch.c:168
    #12 0x4e2a5a in monitor_qmp_dispatch_one /root/qemu/monitor.c:4088
    #13 0x4e2baf in monitor_qmp_bh_dispatcher /root/qemu/monitor.c:4146
    #14 0xf67ad1 in aio_bh_poll util/async.c:118
    #15 0xf724a3 in aio_dispatch util/aio-posix.c:436
    #16 0xf67270 in aio_ctx_dispatch util/async.c:261
    #17 0x7fc31cf8e999 in g_main_context_dispatch 
(/usr/lib64/libglib-2.0.so.0+0x49999)

Direct leak of 64 byte(s) in 2 object(s) allocated from:
    #0 0x7fc321cb6560 in calloc (/usr/local/gcc-6-4/lib64/libasan.so.3+0xc7560)
    #1 0x7fc31cf944d6 in g_malloc0 (/usr/lib64/libglib-2.0.so.0+0x4f4d6)
    #2 0x4cda1f in qemu_init_vcpu /root/qemu/cpus.c:1997
    #3 0x6aedbd in x86_cpu_realizefn /root/qemu/target/i386/cpu.c:4739
    #4 0x8edfc8 in device_set_realized hw/core/qdev.c:826
    #5 0xcff60c in property_set_bool qom/object.c:1928
    #6 0xd09bd2 in object_property_set_qobject qom/qom-qobject.c:27
    #7 0xd048e3 in object_property_set_bool qom/object.c:1188
    #8 0x79ad83 in qdev_device_add /root/qemu/qdev-monitor.c:627
    #9 0x79b50c in qmp_device_add /root/qemu/qdev-monitor.c:807
    #10 0xf4ca5d in do_qmp_dispatch qapi/qmp-dispatch.c:119
    #11 0xf4ce3c in qmp_dispatch qapi/qmp-dispatch.c:168
    #12 0x4e2a5a in monitor_qmp_dispatch_one /root/qemu/monitor.c:4088
    #13 0x4e2baf in monitor_qmp_bh_dispatcher /root/qemu/monitor.c:4146
    #14 0xf67ad1 in aio_bh_poll util/async.c:118
    #15 0xf724a3 in aio_dispatch util/aio-posix.c:436
    #16 0xf67270 in aio_ctx_dispatch util/async.c:261
    #17 0x7fc31cf8e999 in g_main_context_dispatch 
(/usr/lib64/libglib-2.0.so.0+0x49999)

SUMMARY: AddressSanitizer: 8440 byte(s) leaked in 5 allocation(s).

The relevant members in CPU Structure are freed to fix leak. Meanwhile, 
although the
VMChangeStateEntry added in kvm_arch_init_vcpu does not be reportd by ASAN 
since it still
in vm_change_state_head, it's not longer used after hot-del, so free it, too.

Signed-off-by: liujunjie <address@hidden>
Signed-off-by: linzhecheng <address@hidden>
---
 accel/kvm/kvm-all.c  |  3 +++
 cpus.c               |  6 ++++++
 include/sysemu/kvm.h |  1 +
 target/i386/cpu.h    |  2 ++
 target/i386/kvm.c    | 19 ++++++++++++++++++-
 5 files changed, 30 insertions(+), 1 deletion(-)

diff --git a/accel/kvm/kvm-all.c b/accel/kvm/kvm-all.c
index ffee68e..a0491dc 100644
--- a/accel/kvm/kvm-all.c
+++ b/accel/kvm/kvm-all.c
@@ -305,6 +305,9 @@ int kvm_destroy_vcpu(CPUState *cpu)
     vcpu->vcpu_id = kvm_arch_vcpu_id(cpu);
     vcpu->kvm_fd = cpu->kvm_fd;
     QLIST_INSERT_HEAD(&kvm_state->kvm_parked_vcpus, vcpu, node);
+#ifdef TARGET_I386
+    kvm_arch_destroy_vcpu(cpu);
+#endif
 err:
     return ret;
 }
diff --git a/cpus.c b/cpus.c
index d1f1629..cbe28d6 100644
--- a/cpus.c
+++ b/cpus.c
@@ -1842,6 +1842,12 @@ void cpu_remove_sync(CPUState *cpu)
     qemu_mutex_unlock_iothread();
     qemu_thread_join(cpu->thread);
     qemu_mutex_lock_iothread();
+    g_free(cpu->thread);
+    cpu->thread = NULL;
+    g_free(cpu->halt_cond);
+    cpu->halt_cond = NULL;
+    g_free(cpu->cpu_ases);
+    cpu->cpu_ases = NULL;
 }
 
 /* For temporary buffers for forming a name */
diff --git a/include/sysemu/kvm.h b/include/sysemu/kvm.h
index 0b64b8e..eb277f4 100644
--- a/include/sysemu/kvm.h
+++ b/include/sysemu/kvm.h
@@ -380,6 +380,7 @@ unsigned long kvm_arch_vcpu_id(CPUState *cpu);
 #ifdef TARGET_I386
 #define KVM_HAVE_MCE_INJECTION 1
 void kvm_arch_on_sigbus_vcpu(CPUState *cpu, int code, void *addr);
+void kvm_arch_destroy_vcpu(CPUState *cs);
 #endif
 
 void kvm_arch_init_irq_routing(KVMState *s);
diff --git a/target/i386/cpu.h b/target/i386/cpu.h
index 6645046..d6b8137 100644
--- a/target/i386/cpu.h
+++ b/target/i386/cpu.h
@@ -32,6 +32,7 @@
 #endif
 
 #include "exec/cpu-defs.h"
+typedef struct vm_change_state_entry VMChangeStateEntry;
 
 /* The x86 has a strong memory model with some store-after-load re-ordering */
 #define TCG_GUEST_DEFAULT_MO      (TCG_MO_ALL & ~TCG_MO_ST_LD)
@@ -1330,6 +1331,7 @@ typedef struct CPUX86State {
     uint64_t xss;
 
     TPRAccess tpr_access_type;
+    VMChangeStateEntry *vmstate;
 } CPUX86State;
 
 struct kvm_msrs;
diff --git a/target/i386/kvm.c b/target/i386/kvm.c
index 44f7073..1ae5330 100644
--- a/target/i386/kvm.c
+++ b/target/i386/kvm.c
@@ -1041,7 +1041,7 @@ int kvm_arch_init_vcpu(CPUState *cs)
         }
     }
 
-    qemu_add_vm_change_state_handler(cpu_update_state, env);
+    env->vmstate = qemu_add_vm_change_state_handler(cpu_update_state, env);
 
     c = cpuid_find_entry(&cpuid_data.cpuid, 1, 0);
     if (c) {
@@ -1115,6 +1115,23 @@ int kvm_arch_init_vcpu(CPUState *cs)
     return r;
 }
 
+void kvm_arch_destroy_vcpu(CPUState *cs)
+{
+    X86CPU *cpu = X86_CPU(cs);
+    CPUX86State *env = &cpu->env;
+
+    if (has_xsave && env->kvm_xsave_buf) {
+        qemu_vfree(env->kvm_xsave_buf);
+        env->kvm_xsave_buf = NULL;
+    }
+    g_free(cpu->kvm_msr_buf);
+    cpu->kvm_msr_buf = NULL;
+    if (env->vmstate) {
+        qemu_del_vm_change_state_handler(env->vmstate);
+        env->vmstate = NULL;
+    }
+}
+
 void kvm_arch_reset_vcpu(X86CPU *cpu)
 {
     CPUX86State *env = &cpu->env;
-- 
1.8.3.1