[libvirt] [PATCH] virDomainNetDefParseXML: avoid leak upon multiple "filterref"
The offending code below appears in this loop: virNWFilterHashTablePtr filterparams = NULL; ... cur = node->children; while (cur != NULL) { ... } so the first assignment works fine, but second and subsequent ones leak the buffer returned by each preceding virNWFilterParseParamAttributes call.
From 8659fb1ae879befe360e1ec7b8b62434c22698cd Mon Sep 17 00:00:00 2001 From: Jim Meyering <meyering@redhat.com> Date: Tue, 18 May 2010 10:19:30 +0200 Subject: [PATCH] virDomainNetDefParseXML: avoid leak upon multiple "filterref"
* src/conf/domain_conf.c (virDomainNetDefParseXML): Don't leak memory when parsing two or more "filterref" elements. --- src/conf/domain_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-) diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3e45f79..0c717f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1905,6 +1905,7 @@ virDomainNetDefParseXML(virCapsPtr caps, model = virXMLPropString(cur, "type"); } else if (xmlStrEqual (cur->name, BAD_CAST "filterref")) { filter = virXMLPropString(cur, "filter"); + free(filterparams); filterparams = virNWFilterParseParamAttributes(cur); } else if ((flags & VIR_DOMAIN_XML_INTERNAL_STATUS) && xmlStrEqual(cur->name, BAD_CAST "state")) { -- 1.7.1.250.g7d1e8
On 05/18/2010 02:22 AM, Jim Meyering wrote:
* src/conf/domain_conf.c (virDomainNetDefParseXML): Don't leak memory when parsing two or more "filterref" elements. --- src/conf/domain_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3e45f79..0c717f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1905,6 +1905,7 @@ virDomainNetDefParseXML(virCapsPtr caps, model = virXMLPropString(cur, "type"); } else if (xmlStrEqual (cur->name, BAD_CAST "filterref")) { filter = virXMLPropString(cur, "filter"); + free(filterparams); filterparams = virNWFilterParseParamAttributes(cur);
ACK. -- Eric Blake eblake@redhat.com +1-801-349-2682 Libvirt virtualization library http://libvirt.org
On Tue, 2010-05-18 at 08:22 -0600, Eric Blake wrote:
On 05/18/2010 02:22 AM, Jim Meyering wrote:
* src/conf/domain_conf.c (virDomainNetDefParseXML): Don't leak memory when parsing two or more "filterref" elements. --- src/conf/domain_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3e45f79..0c717f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1905,6 +1905,7 @@ virDomainNetDefParseXML(virCapsPtr caps, model = virXMLPropString(cur, "type"); } else if (xmlStrEqual (cur->name, BAD_CAST "filterref")) { filter = virXMLPropString(cur, "filter"); + free(filterparams); filterparams = virNWFilterParseParamAttributes(cur);
ACK.
Don't want this to be the only 'free', make it a VIR_FREE(). Stefan
-- libvir-list mailing list libvir-list@redhat.com https://www.redhat.com/mailman/listinfo/libvir-list
Stefan Berger wrote:
On Tue, 2010-05-18 at 08:22 -0600, Eric Blake wrote:
On 05/18/2010 02:22 AM, Jim Meyering wrote:
* src/conf/domain_conf.c (virDomainNetDefParseXML): Don't leak memory when parsing two or more "filterref" elements. --- src/conf/domain_conf.c | 1 + 1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c index 3e45f79..0c717f2 100644 --- a/src/conf/domain_conf.c +++ b/src/conf/domain_conf.c @@ -1905,6 +1905,7 @@ virDomainNetDefParseXML(virCapsPtr caps, model = virXMLPropString(cur, "type"); } else if (xmlStrEqual (cur->name, BAD_CAST "filterref")) { filter = virXMLPropString(cur, "filter"); + free(filterparams); filterparams = virNWFilterParseParamAttributes(cur);
ACK.
Don't want this to be the only 'free', make it a VIR_FREE().
The only difference is that VIR_FREE also zeros its argument. That's not needed when the next statement sets that same variable. However, considering there are already 254 uses of VIR_FREE in that file, and none of "free", I can't resist making it an even 0xFF and have switched it to conform.
participants (3)
-
Eric Blake -
Jim Meyering -
Stefan Berger