[libvirt] [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(a)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 30315b3..935f140 100644
--- a/src/qemu/qemu_monitor.c
+++ b/src/qemu/qemu_monitor.c
@@ -721,33 +721,33 @@ qemuMonitorIO(int watch, int fd, int events, void *opaque) {
/* We have to unlock to avoid deadlock against command thread,
* but is this safe ? I think it is, because the callback
* will try to acquire the virDomainObjPtr mutex next */
if (eof) {
qemuMonitorEofNotifyCallback eofNotify = mon->cb->eofNotify;
virDomainObjPtr vm = mon->vm;
/* Make sure anyone waiting wakes up now */
virCondSignal(&mon->notify);
virObjectUnlock(mon);
- virObjectUnref(mon);
VIR_DEBUG("Triggering EOF callback");
(eofNotify)(mon, vm, mon->callbackOpaque);
+ virObjectUnref(mon);
} else if (error) {
qemuMonitorErrorNotifyCallback errorNotify = mon->cb->errorNotify;
virDomainObjPtr vm = mon->vm;
/* Make sure anyone waiting wakes up now */
virCondSignal(&mon->notify);
virObjectUnlock(mon);
- virObjectUnref(mon);
VIR_DEBUG("Triggering error callback");
(errorNotify)(mon, vm, mon->callbackOpaque);
+ virObjectUnref(mon);
} else {
virObjectUnlock(mon);
virObjectUnref(mon);
}
}
static qemuMonitorPtr
qemuMonitorOpenInternal(virDomainObjPtr vm,
int fd,
--
1.8.3.2