[PATCH] Fix potential crash when saving guests

["Daniel P. Berrange" <berrange@xxxxxxxxxx>]

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
---
 src/qemu/qemu_driver.c |    9 ++++++++-
 1 files changed, 8 insertions(+), 1 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index d63f57d..363a361 100644
--- 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)
-- 
1.7.4.4

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list