07-Sep-16 12:45, Michal Privoznik пишет:
On 07.09.2016 11:41, Maxim Nestratov wrote:
06-Sep-16 17:42, Michal Privoznik пишет:
On 05.09.2016 18:42, Maxim Nestratov wrote:
There is a possibility that qemu driver frees by unreferencing its
closeCallbacks pointer as it has the only reference to the object,
while in fact not all users of CloseCallbacks called thier
virCloseCallbacksUnset.
Backtrace is the following:
Thread #1:
0 in pthread_cond_wait@@GLIBC_2.3.2 () from /lib64/libpthread.so.0
1 in virCondWait (c=<optimized out>, m=<optimized out>)
at util/virthread.c:154
2 in virThreadPoolFree (pool=0x7f0810110b50)
at util/virthreadpool.c:266
3 in qemuStateCleanup () at qemu/qemu_driver.c:1116
4 in virStateCleanup () at libvirt.c:808
5 in main (argc=<optimized out>, argv=<optimized out>)
at libvirtd.c:1660
Thread #2:
0 in virClassIsDerivedFrom (klass=0xdeadbeef, parent=0x7f0837c694d0)
at util/virobject.c:169
1 in virObjectIsClass (anyobj=anyobj@entry=0x7f08101d4760,
klass=<optimized out>) at util/virobject.c:365
2 in virObjectLock (anyobj=0x7f08101d4760) at util/virobject.c:317
3 in virCloseCallbacksUnset (closeCallbacks=0x7f08101d4760,
vm=vm@entry=0x7f08101d47b0, cb=cb@entry=0x7f081d078fc0
<qemuProcessAutoDestroy>) at util/virclosecallbacks.c:163
4 in qemuProcessAutoDestroyRemove
(driver=driver@entry=0x7f081018be50, vm=vm@entry=0x7f08101d47b0) at
qemu/qemu_process.c:6368
5 in qemuProcessStop (driver=driver@entry=0x7f081018be50,
vm=vm@entry=0x7f08101d47b0,
reason=reason@entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN,
asyncJob=asyncJob@entry=QEMU_ASYNC_JOB_NONE, flags=flags@entry=0) at
qemu/qemu_process.c:5854
6 in processMonitorEOFEvent (vm=0x7f08101d47b0,
driver=0x7f081018be50) at qemu/qemu_driver.c:4585
7 qemuProcessEventHandler (data=<optimized out>,
opaque=0x7f081018be50) at qemu/qemu_driver.c:4629
8 in virThreadPoolWorker (opaque=opaque@entry=0x7f0837c4f820) at
util/virthreadpool.c:145
9 in virThreadHelper (data=<optimized out>) at util/virthread.c:206
10 in start_thread () from /lib64/libpthread.so.0
Let's reference CloseCallbacks object in virCloseCallbacksSet and
unreference in virCloseCallbacksUnset.
Signed-off-by: Maxim Nestratov <mnestratov@xxxxxxxxxxxxx>
---
src/util/virclosecallbacks.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c
index 82d4071..891a92b 100644
--- a/src/util/virclosecallbacks.c
+++ b/src/util/virclosecallbacks.c
@@ -141,6 +141,7 @@ virCloseCallbacksSet(virCloseCallbacksPtr
closeCallbacks,
virObjectRef(vm);
}
+ virObjectRef(closeCallbacks);
ret = 0;
cleanup:
virObjectUnlock(closeCallbacks);
@@ -180,6 +181,8 @@ virCloseCallbacksUnset(virCloseCallbacksPtr
closeCallbacks,
ret = 0;
cleanup:
virObjectUnlock(closeCallbacks);
+ if (!ret)
+ virObjectUnref(closeCallbacks);
Might as well put this a few lines above, just before 'ret = 0' line.
ACK with that changed.
Michal
No, we can't do this, as it could be the last reference to
closeCallbacks and after derefencing it, virObjectUnlock would touch
freed memory.
Ah, good point. Stick to the original then.
Michal
Pushed now. Thanks.
Maxim
--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list