[PATCH 0/2] Fix two memleaks in virQEMUCapsLoadMachines()
*** BLURB HERE *** Michal Prívozník (2): qemu_capabilities: Don't leak @str in virQEMUCapsLoadMachines() qemu_capabilities: Parse "deprecated" in virQEMUCapsLoadMachines() properly src/qemu/qemu_capabilities.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) -- 2.26.2
If parsing "maxCpus" attribute of <machine/> element fails an error is printed but the corresponding string is not freed. While it is very unlikely to happen (parsed XML is not user provided and we are the ones generating it), it is possible. Instead of freeing the variable in the error path explicitly, let's declare it as g_autofree. And while I'm at it, let's bring it into the loop where it's used. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index d41b4a4753..4a6ef2e383 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4012,7 +4012,6 @@ virQEMUCapsLoadMachines(virQEMUCapsAccelPtr caps, { g_autofree char *xpath = g_strdup_printf("./machine[@type='%s']", typeStr); g_autofree xmlNodePtr *nodes = NULL; - char *str = NULL; size_t i; int n; @@ -4029,6 +4028,8 @@ virQEMUCapsLoadMachines(virQEMUCapsAccelPtr caps, caps->machineTypes = g_new0(virQEMUCapsMachineType, caps->nmachineTypes); for (i = 0; i < n; i++) { + g_autofree char *str = NULL; + if (!(caps->machineTypes[i].name = virXMLPropString(nodes[i], "name"))) { virReportError(VIR_ERR_INTERNAL_ERROR, "%s", _("missing machine name in QEMU capabilities cache")); -- 2.26.2
A <machine/> element can have "deprecated" attribute that corresponds to 'deprecated' member of _virQEMUCapsMachineType struct. But the member is of boolean type. Therefore, the string returned by virXMLPropString() must be freed. Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4a6ef2e383..ebd6607888 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4063,7 +4063,11 @@ virQEMUCapsLoadMachines(virQEMUCapsAccelPtr caps, caps->machineTypes[i].defaultCPU = virXMLPropString(nodes[i], "defaultCPU"); caps->machineTypes[i].defaultRAMid = virXMLPropString(nodes[i], "defaultRAMid"); - caps->machineTypes[i].deprecated = virXMLPropString(nodes[i], "deprecated"); + + str = virXMLPropString(nodes[i], "deprecated"); + if (STREQ_NULLABLE(str, "yes")) + caps->machineTypes[i].deprecated = true; + VIR_FREE(str); } return 0; -- 2.26.2
On Tue, Feb 09, 2021 at 04:42:38PM +0100, Michal Privoznik wrote:
A <machine/> element can have "deprecated" attribute that corresponds to 'deprecated' member of _virQEMUCapsMachineType struct. But the member is of boolean type. Therefore, the string returned by virXMLPropString() must be freed.
Signed-off-by: Michal Privoznik <mprivozn@redhat.com> --- src/qemu/qemu_capabilities.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/qemu/qemu_capabilities.c b/src/qemu/qemu_capabilities.c index 4a6ef2e383..ebd6607888 100644 --- a/src/qemu/qemu_capabilities.c +++ b/src/qemu/qemu_capabilities.c @@ -4063,7 +4063,11 @@ virQEMUCapsLoadMachines(virQEMUCapsAccelPtr caps,
caps->machineTypes[i].defaultCPU = virXMLPropString(nodes[i], "defaultCPU"); caps->machineTypes[i].defaultRAMid = virXMLPropString(nodes[i], "defaultRAMid"); - caps->machineTypes[i].deprecated = virXMLPropString(nodes[i], "deprecated"); + + str = virXMLPropString(nodes[i], "deprecated"); + if (STREQ_NULLABLE(str, "yes")) + caps->machineTypes[i].deprecated = true; + VIR_FREE(str);
Doh, subtle bug Reviewed-by: Daniel P. Berrangé <berrange@redhat.com> Regards, Daniel -- |: https://berrange.com -o- https://www.flickr.com/photos/dberrange :| |: https://libvirt.org -o- https://fstop138.berrange.com :| |: https://entangle-photo.org -o- https://www.instagram.com/dberrange :|
On a Tuesday in 2021, Michal Privoznik wrote:
*** BLURB HERE ***
Michal Prívozník (2): qemu_capabilities: Don't leak @str in virQEMUCapsLoadMachines() qemu_capabilities: Parse "deprecated" in virQEMUCapsLoadMachines() properly
src/qemu/qemu_capabilities.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-)
Reviewed-by: Ján Tomko <jtomko@redhat.com> Jano
participants (3)
-
Daniel P. Berrangé -
Ján Tomko -
Michal Privoznik