conf: Fix race between looking up a domain object and freeing it

This patch fixes crash of the daemon that happens due to the following race
condition:

Let's have two threads in the libvirtd daemon's qemu driver:
A - thread executing undefine on the same domain
B - thread executing a API call to get information about a domain

Assume following serialization of operations done by the threads:
1) A has the lock on the domain object and is executing some code prior to
   virDomainObjListRemove()
2) B takes the lock on the domain object list, looks up the domain object
pointer and blocks in the attempt to lock the domain object as A is holding the
lock
3) A reaches virDomainObjListRemove() and unlocks the lock on the domain object
4) A blocks on the attempt to get the domain list lock
5) B is able to lock the domain object now and unlocks the domain list
6) A is now able to lock the domain list, and sheds the last reference on the
domain object, this triggers the freeing function.
6) B starts executing the code on the pointer that is being freed
7) The libvirtd daemon crashes while attempting to access invalid pointer in
thread B.

This patch fixes the race by acquiring a reference on the domain object before
unlocking it in virDomainObjListRemove() and re-locks the object prior to
removing and freeing it. This ensures that no thread holds a lock on the domain
object at the time it is removed from the list, and that doing a list lookup
will never find a domain that is about to vanish.

This is a minimal fix of the problem, but a better solution will be to switch to
full reference counting for domain objects.

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 492e0b73a44e3bdfcc50bdbc715b1f8cd76ec594..e00a5321f9c655b9d3888c2e990707282afc7663 100644 (file)
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -2238,10 +2238,14 @@ void virDomainObjListRemove(virDomainObjListPtr doms,
     char uuidstr[VIR_UUID_STRING_BUFLEN];
 
     virUUIDFormat(dom->def->uuid, uuidstr);
+    virObjectRef(dom);
     virObjectUnlock(dom);
 
     virObjectLock(doms);
+    virObjectLock(dom);
     virHashRemoveEntry(doms->objs, uuidstr);
+    virObjectUnlock(dom);
+    virObjectUnref(dom);
     virObjectUnlock(doms);
 }