[libvirt] [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 | 18 ++++++------------ 1 file changed, 6 insertions(+), 12 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 32b0522..f4bbd74 100644 --- 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); -- 1.8.1.5