[libvirt] [PATCHv3 0/6] Fix memory corruption/crash in the connection close callback

[Eric Blake]

On 04/08/2013 07:04 AM, Peter Krempa wrote:
>> Aiee, perhaps a race between a thread freeing a domain object (and the
>> private data) and another thread that happened to acquire the domain
>> object pointer before it was freed? Let me verify if that is possible.
> 
> Ufff. The domain objects in the qemu driver don't use reference counting
> to track the lifecycles. Thus it's (Theoretically) possible to acquire a
> lock of a domain object in one thread while another thread happens to
> free the domain object.
> 
> I have a reproducer for this issue:

Thanks; I can confirm under valgrind that we have a use after free, with
all sorts of nasty heap corruption potential, after instrumenting my
source a bit more:

diff --git i/src/conf/domain_conf.c w/src/conf/domain_conf.c
index 03e5740..0572822 100644
--- i/src/conf/domain_conf.c
+++ w/src/conf/domain_conf.c
@@ -2240,7 +2240,11 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
     virUUIDFormat(dom->def->uuid, uuidstr);
     virObjectUnlock(dom);

+    printf(" DEBUG: about to remove dom %s from list\n", uuidstr);
+    sleep(2);
+
     virObjectLock(doms);
+    printf(" DEBUG: locked, removing dom %s from list\n", uuidstr);
     virHashRemoveEntry(doms->objs, uuidstr);
     virObjectUnlock(doms);
 }
diff --git i/src/qemu/qemu_driver.c w/src/qemu/qemu_driver.c
index 2c0d7d1..98a49e9 100644
--- i/src/qemu/qemu_driver.c
+++ w/src/qemu/qemu_driver.c
@@ -2297,9 +2297,15 @@ static int qemuDomainGetInfo(virDomainPtr dom,
     int err;
     unsigned long long balloon;

+    char uuidstr[VIR_UUID_STRING_BUFLEN];
     if (!(vm = qemuDomObjFromDomain(dom)))
         goto cleanup;

+    virUUIDFormat(dom->uuid, uuidstr);
+    printf(" DEBUG: about to look up info on %s\n", uuidstr);
+    sleep(5);
+    printf(" DEBUG: looking up info on %s\n", uuidstr);
+
     info->state = virDomainObjGetState(vm, NULL);

     if (!virDomainObjIsActive(vm)) {



> and use a bash oneliner to trigger the issue:
> 
> virsh undefine domain & sleep .1; virsh dominfo domain

valgrind shows the culprit:

 DEBUG: about to remove dom 51c6fc83-65a4-e627-b698-042b00145202 from list
 DEBUG: about to look up info on 51c6fc83-65a4-e627-b698-042b00145202
 DEBUG: locked, removing dom 51c6fc83-65a4-e627-b698-042b00145202 from list
 DEBUG: looking up info on 51c6fc83-65a4-e627-b698-042b00145202
==10033== Thread 6:
==10033== Invalid read of size 4
==10033==    at 0x50FF3BB: virDomainObjGetState (domain_conf.c:16264)
==10033==    by 0x1B044EF0: qemuDomainGetInfo (qemu_driver.c:2309)
==10033==    by 0x515A08C: virDomainGetInfo (libvirt.c:4240)
==10033==    by 0x1243CC: remoteDispatchDomainGetInfo
(remote_dispatch.h:1987)
==10033==    by 0x1242B1: remoteDispatchDomainGetInfoHelper
(remote_dispatch.h:1963)
==10033==    by 0x51D2EE9: virNetServerProgramDispatchCall
(virnetserverprogram.c:439)
==10033==    by 0x51D2A60: virNetServerProgramDispatch
(virnetserverprogram.c:305)
==10033==    by 0x51D922C: virNetServerProcessMsg (virnetserver.c:162)
==10033==    by 0x51D931B: virNetServerHandleJob (virnetserver.c:183)
==10033==    by 0x50B97FD: virThreadPoolWorker (virthreadpool.c:144)
==10033==    by 0x50B91CE: virThreadHelper (virthreadpthread.c:161)
==10033==    by 0x7CAA850: start_thread (pthread_create.c:301)
==10033==  Address 0x1daf3a1c is 60 bytes inside a block of size 136 free'd
==10033==    at 0x4A063F0: free (vg_replace_malloc.c:446)
==10033==    by 0x506C0BD: virFree (viralloc.c:443)
==10033==    by 0x50A7366: virObjectUnref (virobject.c:272)
==10033==    by 0x50D1112: virDomainObjListDataFree (domain_conf.c:891)
==10033==    by 0x5088F3F: virHashRemoveEntry (virhash.c:468)
==10033==    by 0x50D48CA: virDomainObjListRemove (domain_conf.c:2248)
==10033==    by 0x1AFF1A4A: qemuDomainRemoveInactive (qemu_domain.c:1864)
==10033==    by 0x1B04ED30: qemuDomainUndefineFlags (qemu_driver.c:5704)
==10033==    by 0x5166490: virDomainUndefineFlags (libvirt.c:8214)
==10033==    by 0x131732: remoteDispatchDomainUndefineFlags
(remote_dispatch.h:7066)
==10033==    by 0x13161F: remoteDispatchDomainUndefineFlagsHelper
(remote_dispatch.h:7044)
==10033==    by 0x51D2EE9: virNetServerProgramDispatchCall
(virnetserverprogram.c:439)



Once again, I'm trying to ascertain how far back this issue appears.

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 621 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20130408/1ed594ba/attachment-0001.sig>