[libvirt] [PATCH v5 2/4] blockjob: properly track blockcopy xml changes on disk

[Eric Blake]

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

-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 539 bytes
Desc: OpenPGP digital signature
URL: <http://listman.redhat.com/archives/libvir-list/attachments/20140806/13ba75cd/attachment-0001.sig>