From 5f78e7c3588d19b63acd717a53a62fca1971fc9a Mon Sep 17 00:00:00 2001
Message-Id: <5f78e7c3588d19b63acd717a53a62fca1971fc9a.1383922565.git.jdenemar@redhat.com>
From: Michal Privoznik <mprivozn@redhat.com>
Date: Fri, 8 Nov 2013 08:01:41 +0100
Subject: [PATCH] qemu: Avoid double free of VM

https://bugzilla.redhat.com/show_bug.cgi?id=1018267

One of my previous patches (c7ac2519b7f) did try to fix the issue when
domain dies too soon during migration. However, this clumsy approach was
missing removal of qemuProcessHandleMonitorDestroy resulting in double
unrefing of mon->vm and hence producing the daemon crash:

==11843== Invalid read of size 4
==11843==    at 0x50C28C5: virObjectUnref (virobject.c:255)
==11843==    by 0x1148F7DB: qemuMonitorDispose (qemu_monitor.c:258)
==11843==    by 0x50C2991: virObjectUnref (virobject.c:262)
==11843==    by 0x50C2D13: virObjectFreeCallback (virobject.c:388)
==11843==    by 0x509C37B: virEventPollCleanupHandles (vireventpoll.c:583)
==11843==    by 0x509C711: virEventPollRunOnce (vireventpoll.c:652)
==11843==    by 0x509A620: virEventRunDefaultImpl (virevent.c:274)
==11843==    by 0x520D21C: virNetServerRun (virnetserver.c:1112)
==11843==    by 0x11F368: main (libvirtd.c:1513)
==11843==  Address 0x13b88864 is 4 bytes inside a block of size 136 free'd
==11843==    at 0x4A07F5C: free (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so)
==11843==    by 0x5079A2F: virFree (viralloc.c:580)
==11843==    by 0x50C29E3: virObjectUnref (virobject.c:270)
==11843==    by 0x114770E4: qemuProcessHandleMonitorDestroy (qemu_process.c:1103)
==11843==    by 0x1148F7CB: qemuMonitorDispose (qemu_monitor.c:257)
==11843==    by 0x50C2991: virObjectUnref (virobject.c:262)
==11843==    by 0x50C2D13: virObjectFreeCallback (virobject.c:388)
==11843==    by 0x509C37B: virEventPollCleanupHandles (vireventpoll.c:583)
==11843==    by 0x509C711: virEventPollRunOnce (vireventpoll.c:652)
==11843==    by 0x509A620: virEventRunDefaultImpl (virevent.c:274)
==11843==    by 0x520D21C: virNetServerRun (virnetserver.c:1112)
==11843==    by 0x11F368: main (libvirtd.c:1513)

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
(cherry picked from commit 5a4c2374a2ca5d9d5c5d495f507abcef0f5aabe7)

Conflicts:
	src/qemu/qemu_process.c: Context, as 809ee6bad is not backported yet.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/qemu/qemu_process.c | 15 +++------------
 1 file changed, 3 insertions(+), 12 deletions(-)

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index b9f291d..e0097df 100644
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -1082,13 +1082,6 @@ error:
     return -1;
 }
 
-
-static void qemuProcessHandleMonitorDestroy(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
-                                            virDomainObjPtr vm)
-{
-    virObjectUnref(vm);
-}
-
 static int
 qemuProcessHandleTrayChange(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
                             virDomainObjPtr vm,
@@ -1345,7 +1338,6 @@ cleanup:
 
 
 static qemuMonitorCallbacks monitorCallbacks = {
-    .destroy = qemuProcessHandleMonitorDestroy,
     .eofNotify = qemuProcessHandleMonitorEOF,
     .errorNotify = qemuProcessHandleMonitorError,
     .diskSecretLookup = qemuProcessFindVolumeQcowPassphrase,
@@ -1382,7 +1374,7 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int logfd)
     }
 
     /* Hold an extra reference because we can't allow 'vm' to be
-     * deleted while the monitor is active */
+     * deleted unitl the monitor gets its own reference. */
     virObjectRef(vm);
 
     ignore_value(virTimeMillisNow(&priv->monStart));
@@ -1397,11 +1389,10 @@ qemuConnectMonitor(virQEMUDriverPtr driver, virDomainObjPtr vm, int logfd)
         ignore_value(qemuMonitorSetDomainLog(mon, logfd));
 
     virObjectLock(vm);
+    virObjectUnref(vm);
     priv->monStart = 0;
 
-    if (mon == NULL) {
-        virObjectUnref(vm);
-    } else if (!virDomainObjIsActive(vm)) {
+    if (!virDomainObjIsActive(vm)) {
         qemuMonitorClose(mon);
         mon = NULL;
     }
-- 
1.8.4.2