[libvirt] Double deference error in libvirt_virConnectDomainEventRegisterAny

[Daniel P. Berrange]

On Fri, May 26, 2017 at 04:39:17PM +0800, fangying wrote:
> Hi,
>     We'd like to report a double dereference error of 'pyobj_cbData' in libvirt_virConnectDomainEventRegisterAny.
>     The bug can be triggered in the situation where 'domainEventRegisterAny' (the python interface of libvirt_virConnectDomainEventRegisterAny)
> is invoked and network connection is coincidently lost (likely libvirtd restarted) at same time.
> 
>     We get the following stacktrace when the bug is hit.
>     Program terminated with signal 6, Aborted.
>     #0  0x00007fc45cba15d7 in raise () from /usr/lib64/libc.so.6
>     #1  0x00007fc45cba2cc8 in abort () from /usr/lib64/libc.so.6
>     #2  0x00007fc45cbe12f7 in __libc_message () from /usr/lib64/libc.so.6
>     #3  0x00007fc45cbe86d3 in _int_free () from /usr/lib64/libc.so.6
>     #4  0x00007fc45d8d292c in PyDict_Fini () from /usr/lib64/libpython2.7.so.1.0
>     #5  0x00007fc45d94f46a in Py_Finalize () from /usr/lib64/libpython2.7.so.1.0
>     #6  0x00007fc45d960735 in Py_Main () from /usr/lib64/libpython2.7.so.1.0
>     #7  0x00007fc45cb8daf5 in __libc_start_main () from /usr/lib64/libc.so.6
>     #8  0x0000000000400721 in _start ()
> 
>     The double dereference of 'pyobj_cbData' is triggered in the following way:
>     (1) libvirt_virConnectDomainEventRegisterAny is invoked.
>     (2) the event is successfully added to the event callback list (virDomainEventStateRegisterClient in
>         remoteConnectDomainEventRegisterAny returns 1 which means ok).
>     (3) when function remoteConnectDomainEventRegisterAny is hit, network connection disconnected coincidently
>         (or libvirtd is restarted) in the context of function 'call' then the connection is lost and the
>         function 'call' failed, the branch virObjectEventStateDeregisterID is therefore taken.
>     (4) 'pyobj_conn' is dereferenced the 1st time in libvirt_virConnectDomainEventFreeFunc.
>     (5) 'pyobj_cbData' (refered to pyobj_conn) is dereferenced the 2nd time in libvirt_virConnectDomainEventRegisterAny.
>     (6) the double free error is triggered.

IOW,  even when virConnectDomainEventRegisterAny returns -1 there is still
a scenario in which it will trigger the free callback.

This is a bug in the libvirt C impl of virConnectDomainEventRegisterAny,
not the python bindings. If this API returns -1, it *must* guarantee
that the callbacks are never called.


>     static void
>     libvirt_virConnectDomainEventFreeFunc(void *opaque)
>     {
>         PyObject *pyobj_conn = (PyObject*)opaque;
>         LIBVIRT_ENSURE_THREAD_STATE;
>         Py_DECREF(pyobj_conn);          /* 1st dereference comes here */
>         LIBVIRT_RELEASE_THREAD_STATE;
>     }
> 
>     static PyObject *
>     libvirt_virConnectDomainEventRegisterAny(PyObject *self ATTRIBUTE_UNUSED,
>                                              PyObject *args) {
>         ...
>         Py_INCREF(pyobj_cbData);
> 
>         LIBVIRT_BEGIN_ALLOW_THREADS;
>         ret = virConnectDomainEventRegisterAny(conn, dom, eventID,
>                                                cb, pyobj_cbData,
>                                                libvirt_virConnectDomainEventFreeFunc);
> 
>         if (ret < 0) {
>             Py_DECREF(pyobj_cbData);   /* 2nd dereference comes here */
>         }
>     }
> 
>     Currently we cannot find a good solution to fix this problem,
> could anyone guide us to fix it ?

You need to look at the  libvirt code to fix it, rather than python
code. The python code expectations are correct in assuming that
when ret == -1, it must free the data.

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|