Blob Blame History Raw
From 8e0614121dfd5061407bdcdae574a76d3f17c349 Mon Sep 17 00:00:00 2001
Message-Id: <8e0614121dfd5061407bdcdae574a76d3f17c349.1385135432.git.jdenemar@redhat.com>
From: Michal Privoznik <mprivozn@redhat.com>
Date: Fri, 15 Nov 2013 13:52:27 +0100
Subject: [PATCH] qemuMonitorIO: Don't use @mon after it's unrefed

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

The aim of virObject refing and urefing is to tell where the object is
to be used and when is no longer needed. Hence any object shouldn't be
used after it has been unrefed, as we might be the last to hold the
reference. The better way is to call virObjectUnref() *after* the last
object usage. In this specific case, the monitor EOF handler was called
after the qemuMonitorIO called virObjectUnref. Not only that @mon was
disposed (which is not used in the handler anyway) but the @mon->vm
which is causing a SIGSEGV:

2013-11-15 10:17:54.425+0000: 20110: error : qemuMonitorIO:688 : internal error: early end of file from monitor: possible problem:
qemu-kvm: -incoming tcp:01.01.01.0:49152: Failed to bind socket: Cannot assign requested address

Program received signal SIGSEGV, Segmentation fault.
qemuProcessHandleMonitorEOF (mon=<optimized out>, vm=0x7fb728004170) at qemu/qemu_process.c:299
299         if (priv->beingDestroyed) {
(gdb) p *priv
Cannot access memory at address 0x0
(gdb) p vm
$1 = (virDomainObj *) 0x7fb728004170
(gdb) p *vm
$2 = {parent = {parent = {magic = 3735928559, refs = 0, klass = 0xdeadbeef}, lock = {lock = {__data = {__lock = 2, __count = 0, __owner = 20110, __nusers = 1, __kind = 0, __spins = 0, __list = {__prev = 0x0,
            __next = 0x0}}, __size = "\002\000\000\000\000\000\000\000\216N\000\000\001", '\000' <repeats 26 times>, __align = 2}}}, pid = 0, state = {state = 0, reason = 0}, autostart = 0, persistent = 0,
  updated = 0, def = 0x0, newDef = 0x0, snapshots = 0x0, current_snapshot = 0x0, hasManagedSave = false, privateData = 0x0, privateDataFreeFunc = 0x0, taint = 304}

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

Conflicts:
	src/qemu/qemu_monitor.c: Context as 1cfd5a00 is not bacported yet.
Signed-off-by: Jiri Denemark <jdenemar@redhat.com>
---
 src/qemu/qemu_monitor.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/src/qemu/qemu_monitor.c b/src/qemu/qemu_monitor.c
index 4384ab4..adc9cca 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -727,9 +727,9 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) {
         /* Make sure anyone waiting wakes up now */
         virCondSignal(&mon->notify);
         virObjectUnlock(mon);
-        virObjectUnref(mon);
         VIR_DEBUG("Triggering EOF callback");
         (eofNotify)(mon, vm);
+        virObjectUnref(mon);
     } else if (error) {
         void (*errorNotify)(qemuMonitorPtr, virDomainObjPtr)
             = mon->cb->errorNotify;
@@ -738,9 +738,9 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) {
         /* Make sure anyone waiting wakes up now */
         virCondSignal(&mon->notify);
         virObjectUnlock(mon);
-        virObjectUnref(mon);
         VIR_DEBUG("Triggering error callback");
         (errorNotify)(mon, vm);
+        virObjectUnref(mon);
     } else {
         virObjectUnlock(mon);
         virObjectUnref(mon);
-- 
1.8.4.4