From 5a791c899552628154320c6dfa068f51ee60c0a8 Mon Sep 17 00:00:00 2001
From: Michal Privoznik <mprivozn@redhat.com>
Date: Fri, 8 Mar 2013 13:09:32 +0100
Subject: [PATCH] qemuDomainBlockStatsFlags: Guard disk lookup with a domain
 job

When there are two concurrent threads, we may dereference a NULL
pointer, even though it has been checked before:

1. Thread1: starts executing qemuDomainBlockStatsFlags() with nparams != 0.
            It finds given disk and successfully pass check for disk->info.alias
            not being NULL.
2. Thread2: starts executing qemuDomainDetachDeviceFlags() on the very same
            disk as Thread1 is working on.
3. Thread1: gets to qemuDomainObjBeginJob() where it sets a job on a
            domain.
4. Thread2: also tries to set a job. However, we are not guaranteed which
            thread wins. So assume it's Thread2 who can continue.
5. Thread2: does the actual detach and frees disk->info.alias
6. Thread2: quits the job
7. Thread1: now successfully acquires the job, and accesses a NULL pointer.
---
 src/qemu/qemu_driver.c | 44 ++++++++++++++++++------------------------
 1 file changed, 19 insertions(+), 25 deletions(-)

diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index 32b05229fc..f4bbd74062 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -8496,31 +8496,6 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
     if (!(vm = qemuDomObjFromDomain(dom)))
         goto cleanup;
 
-    if (!virDomainObjIsActive(vm)) {
-        virReportError(VIR_ERR_OPERATION_INVALID,
-                       "%s", _("domain is not running"));
-        goto cleanup;
-    }
-
-    if (*nparams != 0) {
-        if ((i = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
-            virReportError(VIR_ERR_INVALID_ARG,
-                           _("invalid path: %s"), path);
-            goto cleanup;
-        }
-        disk = vm->def->disks[i];
-
-        if (!disk->info.alias) {
-             virReportError(VIR_ERR_INTERNAL_ERROR,
-                            _("missing disk device alias name for %s"),
-                            disk->dst);
-             goto cleanup;
-        }
-    }
-
-    priv = vm->privateData;
-    VIR_DEBUG("priv=%p, params=%p, flags=%x", priv, params, flags);
-
     if (qemuDomainObjBeginJob(driver, vm, QEMU_JOB_QUERY) < 0)
         goto cleanup;
 
@@ -8530,6 +8505,25 @@ qemuDomainBlockStatsFlags(virDomainPtr dom,
         goto endjob;
     }
 
+    if (*nparams != 0) {
+        if ((i = virDomainDiskIndexByName(vm->def, path, false)) < 0) {
+            virReportError(VIR_ERR_INVALID_ARG,
+                           _("invalid path: %s"), path);
+            goto endjob;
+        }
+        disk = vm->def->disks[i];
+
+        if (!disk->info.alias) {
+             virReportError(VIR_ERR_INTERNAL_ERROR,
+                            _("missing disk device alias name for %s"),
+                            disk->dst);
+             goto endjob;
+        }
+    }
+
+    priv = vm->privateData;
+    VIR_DEBUG("priv=%p, params=%p, flags=%x", priv, params, flags);
+
     qemuDomainObjEnterMonitor(driver, vm);
     tmp = *nparams;
     ret = qemuMonitorGetBlockStatsParamsNumber(priv->mon, nparams);