Fix potential crash when saving guests

The qemudDomainSaveFlag method will call EndJob on the 'vm'
object it is passed in. This can result in the 'vm' object
being free'd if the last reference is removed. Thus no caller
of 'qemudDomainSaveFlag' must *ever* reference 'vm' again
upon return.

Unfortunately qemudDomainSave and qemuDomainManagedSave
both call 'virDomainObjUnlock', which can result in a
crash. This is non-deterministic since it involves a race
with the monitor I/O thread.

Fix this by making qemudDomainSaveFlag responsible for
calling virDomainObjUnlock instead.

* src/qemu/qemu_driver.c: Fix potential use after free
  when saving guests

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d63f57df3ad6b0b045d3e9ac33e95f180a666654..363a361b21eb562071d361ccee56e83f75e1d5aa 100644 (file)
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -2091,7 +2091,10 @@ qemuCompressProgramName(int compress)
 }
 
 /* This internal function expects the driver lock to already be held on
- * entry and the vm must be active.
+ * entry and the vm must be active + locked. Vm will be unlocked and
+ * potentially free'd after this returns (eg transient VMs are freed
+ * shutdown). So 'vm' must not be referenced by the caller after
+ * this returns (whether returning success or failure).
  */
 static int qemudDomainSaveFlag(struct qemud_driver *driver, virDomainPtr dom,
                                virDomainObjPtr vm, const char *path,
@@ -2318,6 +2321,8 @@ cleanup:
         unlink(path);
     if (event)
         qemuDomainEventQueue(driver, event);
+    if (vm)
+        virDomainObjUnlock(vm);
     return ret;
 }
 
@@ -2380,6 +2385,7 @@ static int qemudDomainSave(virDomainPtr dom, const char *path)
     }
 
     ret = qemudDomainSaveFlag(driver, dom, vm, path, compressed);
+    vm = NULL;
 
 cleanup:
     if (vm)
@@ -2436,6 +2442,7 @@ qemuDomainManagedSave(virDomainPtr dom, unsigned int flags)
 
     compressed = QEMUD_SAVE_FORMAT_RAW;
     ret = qemudDomainSaveFlag(driver, dom, vm, name, compressed);
+    vm = NULL;
 
 cleanup:
     if (vm)