Re: [libvirt] new NULL-dereference in qemu_driver.c

Daniel P. Berrange wrote: > On Tue, Apr 27, 2010 at 06:45:16PM +0200, Jim Meyering wrote: >> I ran clang on the very latest and it spotted this problem: >> >From qemu_driver.c, around line 11100, >> >> else { >> /* qemu is a little funny with running guests and the restoration >> * of snapshots. If the snapshot was taken online, >> * then after a "loadvm" monitor command, the VM is set running >> * again. If the snapshot was taken offline, then after a "loadvm" >> * monitor command the VM is left paused. Unpausing it leads to >> * the memory state *before* the loadvm with the disk *after* the >> * loadvm, which obviously is bound to corrupt something. >> * Therefore we destroy the domain and set it to "off" in this case. >> */ >> >> if (virDomainObjIsActive(vm)) { >> qemudShutdownVMDaemon(driver, vm); >> event = virDomainEventNewFromObj(vm, >> VIR_DOMAIN_EVENT_STOPPED, >> VIR_DOMAIN_EVENT_STOPPED_FROM_SNAPSHOT); >> if (!vm->persistent) { >> if (qemuDomainObjEndJob(vm) > 0) >> virDomainRemoveInactive(&driver->domains, vm); >> vm = NULL; > > This needs to add 'goto endjob' or possibly 'goto cleanup' No point in endjob, since it does nothing when vm == NULL. Here's a tentative patch for that and another, similar problem (haven't even compiled it or run it through clang, but have to run). Will follow up tomorrow.

Subject: [PATCH] qemuDomainSnapshotCreateXML: avoid NULL dereference * src/qemu/qemu_driver.c (qemuDomainSnapshotCreateXML): When setting "vm" to NULL, jump over vm-dereferencing code to "cleanup". (qemuDomainRevertToSnapshot): Likewise.