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.

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