[libvirt] [PATCH] qemu: Avoid memleak on failure to format blockjobs
virXMLFormatElement() frees attrBuf on success, but not necessarily on failure. Most other callers of this function take the time to reset attrBuf afterwords, but qemuDomainObjPrivateXMLFOrmatBlockjobs() was relying on it succeeding, and could thus result in a memory leak. Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_domain.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index dd67be5e2a..ad7a6fe136 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2232,11 +2232,15 @@ qemuDomainObjPrivateXMLFormatBlockjobs(virBufferPtr buf, { virBuffer attrBuf = VIR_BUFFER_INITIALIZER; bool bj = qemuDomainHasBlockjob(vm, false); + ret = -1; virBufferAsprintf(&attrBuf, " active='%s'", virTristateBoolTypeToString(virTristateBoolFromBool(bj))); - return virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL); + ret = virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL); + cleanup: + virBufferFreeAndReset(&attrBuf); + return ret; } -- 2.17.2
On 10/18/18 3:08 PM, Eric Blake wrote:
virXMLFormatElement() frees attrBuf on success, but not necessarily on failure. Most other callers of this function take the time to reset attrBuf afterwords, but qemuDomainObjPrivateXMLFOrmatBlockjobs() was relying on it succeeding, and could thus result in a memory leak.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_domain.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index dd67be5e2a..ad7a6fe136 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2232,11 +2232,15 @@ qemuDomainObjPrivateXMLFormatBlockjobs(virBufferPtr buf, { virBuffer attrBuf = VIR_BUFFER_INITIALIZER; bool bj = qemuDomainHasBlockjob(vm, false); + ret = -1;
I need to quit editing patches in my mailer. As written, this obviously doesn't compile; s/ret/int ret/
virBufferAsprintf(&attrBuf, " active='%s'", virTristateBoolTypeToString(virTristateBoolFromBool(bj)));
- return virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL); + ret = virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL); + cleanup:
and this label is unused.
+ virBufferFreeAndReset(&attrBuf); + return ret; }
-- Eric Blake, Principal Software Engineer Red Hat, Inc. +1-919-301-3266 Virtualization: qemu.org | libvirt.org
On 10/18/2018 10:42 PM, Eric Blake wrote:
On 10/18/18 3:08 PM, Eric Blake wrote:
virXMLFormatElement() frees attrBuf on success, but not necessarily on failure. Most other callers of this function take the time to reset attrBuf afterwords, but qemuDomainObjPrivateXMLFOrmatBlockjobs() was relying on it succeeding, and could thus result in a memory leak.
Signed-off-by: Eric Blake <eblake@redhat.com> --- src/qemu/qemu_domain.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_domain.c b/src/qemu/qemu_domain.c index dd67be5e2a..ad7a6fe136 100644 --- a/src/qemu/qemu_domain.c +++ b/src/qemu/qemu_domain.c @@ -2232,11 +2232,15 @@ qemuDomainObjPrivateXMLFormatBlockjobs(virBufferPtr buf, { virBuffer attrBuf = VIR_BUFFER_INITIALIZER; bool bj = qemuDomainHasBlockjob(vm, false); + ret = -1;
I need to quit editing patches in my mailer. As written, this obviously doesn't compile; s/ret/int ret/
virBufferAsprintf(&attrBuf, " active='%s'", virTristateBoolTypeToString(virTristateBoolFromBool(bj)));
- return virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL); + ret = virXMLFormatElement(buf, "blockjobs", &attrBuf, NULL); + cleanup:
and this label is unused.
+ virBufferFreeAndReset(&attrBuf); + return ret; }
ACK with those cleanups squashed in. Michal
participants (2)
-
Eric Blake -
Michal Privoznik