[libvirt] [PATCH] qemu: Don't corrupt pointer in qemuDomainSaveMemory()

The code that was split out into the qemuDomainSaveMemory expands the pointer containing the XML description of the domain that it gets from higher layers. If the pointer changes the old one is invalid and the upper layer function tries to free it causing an abort. This patch changes the expansion of the original string to a new alocation and copy of the contents. --- src/qemu/qemu_driver.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c index 978af57..7b8eec6 100644 --- a/src/qemu/qemu_driver.c +++ b/src/qemu/qemu_driver.c @@ -2768,7 +2768,7 @@ static int qemuDomainSaveMemory(struct qemud_driver *driver, virDomainObjPtr vm, const char *path, - const char *xml, + const char *domXML, int compressed, bool was_running, unsigned int flags, @@ -2785,6 +2785,7 @@ qemuDomainSaveMemory(struct qemud_driver *driver, unsigned long long pad; unsigned long long offset; size_t len; + char *xml = NULL; memset(&header, 0, sizeof(header)); memcpy(header.magic, QEMUD_SAVE_PARTIAL, sizeof(header.magic)); @@ -2793,7 +2794,7 @@ qemuDomainSaveMemory(struct qemud_driver *driver, header.compressed = compressed; - len = strlen(xml) + 1; + len = strlen(domXML) + 1; offset = sizeof(header) + len; /* Due to way we append QEMU state on our header with dd, @@ -2807,10 +2808,12 @@ qemuDomainSaveMemory(struct qemud_driver *driver, pad = 1024; pad += (QEMU_MONITOR_MIGRATE_TO_FILE_BS - ((offset + pad) % QEMU_MONITOR_MIGRATE_TO_FILE_BS)); - if (VIR_EXPAND_N(xml, len, pad) < 0) { + if (VIR_ALLOC_N(xml, len + pad) < 0) { virReportOOMError(); goto cleanup; } + strcpy(xml, domXML); + offset += pad; header.xml_len = len; @@ -2878,6 +2881,7 @@ cleanup: VIR_FORCE_CLOSE(fd); virFileWrapperFdCatchError(wrapperFd); virFileWrapperFdFree(wrapperFd); + VIR_FREE(xml); if (ret != 0 && needUnlink) unlink(path); -- 1.8.0

On 11/06/2012 03:07 AM, Peter Krempa wrote:
The code that was split out into the qemuDomainSaveMemory expands the pointer containing the XML description of the domain that it gets from higher layers. If the pointer changes the old one is invalid and the upper layer function tries to free it causing an abort.
This patch changes the expansion of the original string to a new alocation and copy of the contents.
s/alocation/allocation/
--- src/qemu/qemu_driver.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
ACK. Good find, and hidden enough that I can see how I missed it in the first review. -- Eric Blake eblake@redhat.com +1-919-301-3266 Libvirt virtualization library http://libvirt.org

On 11/06/12 14:21, Eric Blake wrote:
On 11/06/2012 03:07 AM, Peter Krempa wrote:
The code that was split out into the qemuDomainSaveMemory expands the pointer containing the XML description of the domain that it gets from higher layers. If the pointer changes the old one is invalid and the upper layer function tries to free it causing an abort.
This patch changes the expansion of the original string to a new alocation and copy of the contents.
s/alocation/allocation/
--- src/qemu/qemu_driver.c | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-)
ACK. Good find, and hidden enough that I can see how I missed it in the first review.
I fixed the commit message and pushed the patch. Thanks. Peter
participants (2)
-
Eric Blake
-
Peter Krempa