[libvirt] [PATCH] util: fix crash in virClassIsDerivedFrom for CloseCallbacks objects
Maxim Nestratov
mnestratov at virtuozzo.com
Wed Sep 7 09:53:09 UTC 2016
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 at 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 at entry=0x7f08101d47b0, cb=cb at entry=0x7f081d078fc0
>>>> <qemuProcessAutoDestroy>) at util/virclosecallbacks.c:163
>>>> 4 in qemuProcessAutoDestroyRemove
>>>> (driver=driver at entry=0x7f081018be50, vm=vm at entry=0x7f08101d47b0) at
>>>> qemu/qemu_process.c:6368
>>>> 5 in qemuProcessStop (driver=driver at entry=0x7f081018be50,
>>>> vm=vm at entry=0x7f08101d47b0,
>>>> reason=reason at entry=VIR_DOMAIN_SHUTOFF_SHUTDOWN,
>>>> asyncJob=asyncJob at entry=QEMU_ASYNC_JOB_NONE, flags=flags at 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 at 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 at virtuozzo.com>
>>>> ---
>>>> 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
More information about the libvir-list
mailing list