[libvirt] [PATCH 1/3] util: fix crash in virClassIsDerivedFrom for CloseCallbacks objects

[Peter Krempa]

On Mon, Jun 06, 2016 at 20:59:22 +0300, 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.

Do you happen to have any kind of reproducer for this crash?

> 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 | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/src/util/virclosecallbacks.c b/src/util/virclosecallbacks.c
> index 82d4071..2fab56b 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);
> @@ -177,6 +178,7 @@ virCloseCallbacksUnset(virCloseCallbacksPtr closeCallbacks,
>          goto cleanup;
>  
>      virObjectUnref(vm);
> +    virObjectUnref(closeCallbacks);

If this cleared the last reference, closeCallbacks is invalid at that
point.

>      ret = 0;
>   cleanup:
>      virObjectUnlock(closeCallbacks);

But this would dereference and use it.

Peter