[libvirt] [PATCH] vbox: fix a segfault when taking a snapshot

there is a segfault in the vbox driver when taking a snapshot in the following functions: - vboxDomainGetXMLDesc - vboxSnapshotGetReadWriteDisks - vboxSnapshotGetReadOnlyDisks The virStorageSourcePtr in virDomainDiskDef was not correctly allocated. --- src/vbox/vbox_tmpl.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/vbox/vbox_tmpl.c b/src/vbox/vbox_tmpl.c index 1ed2729..6365f2a 100644 --- a/src/vbox/vbox_tmpl.c +++ b/src/vbox/vbox_tmpl.c @@ -2872,10 +2872,12 @@ static char *vboxDomainGetXMLDesc(virDomainPtr dom, unsigned int flags) { /* Allocate mem, if fails return error */ if (VIR_ALLOC_N(def->disks, def->ndisks) >= 0) { for (i = 0; i < def->ndisks; i++) { - if (VIR_ALLOC(def->disks[i]) < 0) { + virDomainDiskDefPtr disk = virDomainDiskDefNew(); + if (!disk) { error = true; break; } + def->disks[i] = disk; } } else { error = true; @@ -7175,6 +7177,10 @@ int vboxSnapshotGetReadWriteDisks(virDomainSnapshotDefPtr def, /* Allocate mem, if fails return error */ if (VIR_ALLOC_N(def->disks, def->ndisks) < 0) goto cleanup; + for (i = 0; i < def->ndisks; i++) { + if (VIR_ALLOC(def->disks[i].src) < 0) + goto cleanup; + } if (!vboxGetMaxPortSlotValues(data->vboxObj, maxPortPerInst, maxSlotPerPort)) goto cleanup; @@ -7302,11 +7308,11 @@ int vboxSnapshotGetReadWriteDisks(virDomainSnapshotDefPtr def, ret = 0; cleanup: if (ret < 0) { - for (i = 0; i < def->dom->ndisks; i++) - VIR_FREE(def->dom->disks[i]); - VIR_FREE(def->dom->disks); - def->dom->ndisks = 0; - ret = -1; + for (i = 0; i < def->ndisks; i++) { + VIR_FREE(def->disks[i].src); + } + VIR_FREE(def->disks); + def->ndisks = 0; } VBOX_RELEASE(snap); return ret; @@ -7380,8 +7386,10 @@ int vboxSnapshotGetReadOnlyDisks(virDomainSnapshotPtr snapshot, /* Allocate mem, if fails return error */ if (VIR_ALLOC_N(def->dom->disks, def->dom->ndisks) >= 0) { for (i = 0; i < def->dom->ndisks; i++) { - if (VIR_ALLOC(def->dom->disks[i]) < 0) + virDomainDiskDefPtr diskDef = virDomainDiskDefNew(); + if (!diskDef) goto cleanup; + def->dom->disks[i] = diskDef; } } else { goto cleanup; @@ -7516,7 +7524,7 @@ int vboxSnapshotGetReadOnlyDisks(virDomainSnapshotPtr snapshot, cleanup: if (ret < 0) { for (i = 0; i < def->dom->ndisks; i++) - VIR_FREE(def->dom->disks[i]); + virDomainDiskDefFree(def->dom->disks[i]); VIR_FREE(def->dom->disks); def->dom->ndisks = 0; } -- 1.7.10.4

On 06/17/2014 02:28 AM, Yohan BELLEGUIC wrote:
there is a segfault in the vbox driver when taking a snapshot in the following functions: - vboxDomainGetXMLDesc - vboxSnapshotGetReadWriteDisks - vboxSnapshotGetReadOnlyDisks
The virStorageSourcePtr in virDomainDiskDef was not correctly allocated.
Aha; this happened because your commits for adding vbox snapshots (commit 4dc5d8f and friends) were written before my patch that redid disk allocation (commit bc3f5f1) but applied after, and Dan didn't know to rebase across the semantic difference.
--- src/vbox/vbox_tmpl.c | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-)
ACK and pushed. -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
participants (2)
-
Eric Blake
-
Yohan BELLEGUIC