On 07/28/2014 10:20 PM, Eric Blake wrote:
>
> Fix things by saving state any time we modify live XML, while
> delaying XML disk modifications until after the event completes.
Except that I didn't, which means I caused a use-after-free situation
which can crash libvirtd when doing blockcopy or active blockcommit:
> +++ b/src/qemu/qemu_driver.c
> @@ -14911,38 +14911,43 @@ qemuDomainBlockPivot(virConnectPtr conn,
> virSecurityManagerSetDiskLabel(driver->securityManager, vm->def, disk) < 0))
> goto cleanup;
>
> - /* Attempt the pivot. */
> + /* Attempt the pivot. Record the attempt now, to prevent duplicate
> + * attempts; but the actual disk change will be made when emitting
> + * the event.
What I missed was that a couple lines before, the code did 'disk->src =
disk->mirror', which means that the qemu event handler...
> @@ -1027,29 +1029,58 @@ qemuProcessHandleBlockJob(qemuMonitorPtr mon ATTRIBUTE_UNUSED,
> + /* If we completed a block pull or commit, then update the XML
> + * to match. */
> + if (status == VIR_DOMAIN_BLOCK_JOB_COMPLETED) {
> + bool has_mirror = !!disk->mirror;
> +
> + if (disk->mirrorState == VIR_DOMAIN_DISK_MIRROR_PIVOT) {
> + /* XXX We want to revoke security labels and disk
> + * lease, as well as audit that revocation, before
> + * dropping the original source. But it gets tricky
> + * if both source and mirror share common backing
> + * files (we want to only revoke the non-shared
> + * portion of the chain); so for now, we leak the
> + * access to the original. */
> + virStorageSourceFree(disk->src);
> + disk->src = disk->mirror;
is promptly calling virStorageSourceFree() on the same pointer that it
is about to assign into disk->src, as well as leaking the original
disk->src.
Fix coming up.
--
Eric Blake eblake redhat com +1-919-301-3266
Libvirt virtualization library http://libvirt.org
Attachment:
signature.asc
Description: OpenPGP digital signature
-- libvir-list mailing list libvir-list@xxxxxxxxxx https://www.redhat.com/mailman/listinfo/libvir-list