qemu: avoid corruption of domain hashtable and misuse of freed domains

This was also found while investigating

   https://bugzilla.redhat.com/show_bug.cgi?id=670848

An EOF on a domain's monitor socket results in an event being queued
to handle the EOF. The handler calls qemuProcessHandleMonitorEOF. If
it is a transient domain, this leads to a call to
virDomainRemoveInactive, which removes the domain from the driver's
hashtable and unref's it. Nowhere in this code is the qemu driver lock
acquired.

However, all modifications to the driver's domain hashtable *must* be
done while holding the driver lock, otherwise the hashtable can become
corrupt, and (even more likely) another thread could call a different
hashtable function and acquire a pointer to the domain that is in the
process of being destroyed.

To prevent such a disaster, qemuProcessHandleMonitorEOF must get the
qemu driver lock *before* it gets the DomainObj's lock, and hold it
until it is finished with the DomainObj. This guarantees that nobody
else modifies the hashtable at the same time, and that anyone who had
already gotten the DomainObj from the hashtable prior to this call has
finished with it before we remove/destroy it.

diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c
index 908472568cdfbaae607e420cebc4511c7209fd8c..ee1b4c4be4f3157cdd2ac78884c1bc2e1aed252d 100644 (file)
--- a/src/qemu/qemu_process.c
+++ b/src/qemu/qemu_process.c
@@ -107,11 +107,13 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
 
     VIR_DEBUG("Received EOF on %p '%s'", vm, vm->def->name);
 
+    qemuDriverLock(driver);
     virDomainObjLock(vm);
 
     if (!virDomainObjIsActive(vm)) {
         VIR_DEBUG("Domain %p is not active, ignoring EOF", vm);
         virDomainObjUnlock(vm);
+        qemuDriverUnlock(driver);
         return;
     }
 
@@ -137,10 +139,9 @@ qemuProcessHandleMonitorEOF(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
         virDomainObjUnlock(vm);
 
     if (event) {
-        qemuDriverLock(driver);
         qemuDomainEventQueue(driver, event);
-        qemuDriverUnlock(driver);
     }
+    qemuDriverUnlock(driver);
 }