Re: [libvirt] [PATCH 1/2] Fix multiple potential NULL pointer references in monitor usage

Any method which intends to invoke a monitor command must have a check for virDomainObjIsActive() before using the monitor to ensure that priv->mon != NULL. There is one subtle edge case in this though. If a method invokes multiple monitor commands, and calls qemuDomainObjExitMonitor() in between two of these commands then there is no guarentee that priv->mon != NULL anymore. This is because the QEMU process may exit or die at any time, and because qemuDomainObjEnterMonitor() releases the lock on virDomainObj, it is possible for the background thread to close the monitor handle and thus qemuDomainObjExitMonitor will release the last reference allowing priv->mon to become NULL.

@@ -5444,15 +5471,15 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) int i, rc; int ret = -1; + qemuDomainObjEnterMonitor(vm); + /* We need different branches here, because we want to offline * in reverse order to onlining, so any partial fail leaves us in a * reasonably sensible state */ if (nvcpus > vm->def->vcpus) { for (i = vm->def->vcpus ; i < nvcpus ; i++) { /* Online new CPU */ - qemuDomainObjEnterMonitor(vm); rc = qemuMonitorSetCPU(priv->mon, i, 1); - qemuDomainObjExitMonitor(vm); if (rc == 0) goto unsupported; if (rc < 0) @@ -5463,9 +5490,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) } else { for (i = vm->def->vcpus - 1 ; i >= nvcpus ; i--) { /* Offline old CPU */ - qemuDomainObjEnterMonitor(vm); rc = qemuMonitorSetCPU(priv->mon, i, 0); - qemuDomainObjExitMonitor(vm); if (rc == 0) goto unsupported; if (rc < 0) @@ -5478,6 +5503,7 @@ static int qemudDomainHotplugVcpus(virDomainObjPtr vm, unsigned int nvcpus) ret = 0; cleanup: + qemuDomainObjExitMonitor(vm);