9:36 a.m.
Ján Tomko (2):
conf: Fix cpumask leak in virDomainDefFree
conf: Fix leaks in virNetworkObjUpdateParseFile
src/conf/domain_conf.c | 2 +-
src/conf/network_conf.c | 19 +++++++++++--------
2 files changed, 12 insertions(+), 9 deletions(-)
--
1.7.8.6
9:36 a.m.
New subject: [libvirt] [PATCH 1/2] conf: Fix cpumask leak in virDomainDefFree
def->cpumask is a bitmap and needs to be freed by virBitmapFree.
---
src/conf/domain_conf.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7273790..c29d2ef 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1738,7 +1738,7 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainClockDefClear(&def->clock);
VIR_FREE(def->name);
- VIR_FREE(def->cpumask);
+ virBitmapFree(def->cpumask);
VIR_FREE(def->emulator);
VIR_FREE(def->description);
VIR_FREE(def->title);
--
1.7.8.6
10:40 a.m.
New subject: [libvirt] [PATCH 1/2] conf: Fix cpumask leak in virDomainDefFree
On 25.01.2013 16:36, Ján Tomko wrote:
def->cpumask is a bitmap and needs to be freed by virBitmapFree.
---
src/conf/domain_conf.c | 2 +-
1 files changed, 1 insertions(+), 1 deletions(-)
diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 7273790..c29d2ef 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1738,7 +1738,7 @@ void virDomainDefFree(virDomainDefPtr def)
virDomainClockDefClear(&def->clock);
VIR_FREE(def->name);
- VIR_FREE(def->cpumask);
+ virBitmapFree(def->cpumask);
VIR_FREE(def->emulator);
VIR_FREE(def->description);
VIR_FREE(def->title);
ACK
Michal
4:59 a.m.
New subject: [libvirt] [PATCH 1/2] conf: Fix cpumask leak in virDomainDefFree
On 01/25/13 17:40, Michal Privoznik wrote:
On 25.01.2013 16:36, Ján Tomko wrote:
> def->cpumask is a bitmap and needs to be freed by virBitmapFree.
> ---
> src/conf/domain_conf.c | 2 +-
> 1 files changed, 1 insertions(+), 1 deletions(-)
>
ACK
Michal
Thanks, pushed now.
9:36 a.m.
New subject: [libvirt] [PATCH 2/2] conf: Fix leaks in virNetworkObjUpdateParseFile
Free the bitmap before calling virBitmapParse, which will overwrite it.
Also free xml.
---
src/conf/network_conf.c | 19 +++++++++++--------
1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index c93916d..013333c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1855,14 +1855,16 @@ virNetworkObjUpdateParseFile(const char *filename,
ctxt->node = node;
class_id = virXPathString("string(./class_id[1]/@bitmap)", ctxt);
- if (class_id &&
- virBitmapParse(class_id, 0,
- &net->class_id, CLASS_ID_BITMAP_SIZE) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Malformed 'class_id' attribute: %s"),
- class_id);
- VIR_FREE(class_id);
- goto cleanup;
+ if (class_id) {
+ virBitmapFree(net->class_id);
+ if (virBitmapParse(class_id, 0,
+ &net->class_id, CLASS_ID_BITMAP_SIZE) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Malformed 'class_id' attribute:
%s"),
+ class_id);
+ VIR_FREE(class_id);
+ goto cleanup;
+ }
}
VIR_FREE(class_id);
@@ -1896,6 +1898,7 @@ virNetworkObjUpdateParseFile(const char *filename,
cleanup:
xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xml);
return ret;
}
--
1.7.8.6
10:40 a.m.
New subject: [libvirt] [PATCH 2/2] conf: Fix leaks in virNetworkObjUpdateParseFile
On 25.01.2013 16:36, Ján Tomko wrote:
Free the bitmap before calling virBitmapParse, which will overwrite
it.
Also free xml.
---
src/conf/network_conf.c | 19 +++++++++++--------
1 files changed, 11 insertions(+), 8 deletions(-)
ACK
Michal
1:15 p.m.
New subject: [libvirt] [PATCH 2/2] conf: Fix leaks in virNetworkObjUpdateParseFile
On 01/25/2013 10:36 AM, Ján Tomko wrote:
Free the bitmap before calling virBitmapParse, which will overwrite
it.
Also free xml.
---
src/conf/network_conf.c | 19 +++++++++++--------
1 files changed, 11 insertions(+), 8 deletions(-)
diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
index c93916d..013333c 100644
--- a/src/conf/network_conf.c
+++ b/src/conf/network_conf.c
@@ -1855,14 +1855,16 @@ virNetworkObjUpdateParseFile(const char *filename,
ctxt->node = node;
class_id = virXPathString("string(./class_id[1]/@bitmap)", ctxt);
- if (class_id &&
- virBitmapParse(class_id, 0,
- &net->class_id, CLASS_ID_BITMAP_SIZE) < 0) {
- virReportError(VIR_ERR_INTERNAL_ERROR,
- _("Malformed 'class_id' attribute: %s"),
- class_id);
- VIR_FREE(class_id);
- goto cleanup;
+ if (class_id) {
+ virBitmapFree(net->class_id);
If there is a situation where this function can be called and
net->class_id is already filled in, then doesn't that also mean that
net->floor_sum could have already been set?
If that's the case, then we need to also set net->floor_sum to 0, in
case it was previously non-0 and the new status doesn't have anything
set (implying 0).
If not, then this virBitmapFree() is a NOP.
+ if (virBitmapParse(class_id, 0,
+ &net->class_id, CLASS_ID_BITMAP_SIZE) < 0) {
+ virReportError(VIR_ERR_INTERNAL_ERROR,
+ _("Malformed 'class_id' attribute:
%s"),
+ class_id);
+ VIR_FREE(class_id);
+ goto cleanup;
+ }
}
VIR_FREE(class_id);
@@ -1896,6 +1898,7 @@ virNetworkObjUpdateParseFile(const char *filename,
cleanup:
xmlXPathFreeContext(ctxt);
+ xmlFreeDoc(xml);
return ret;
}
8:19 a.m.
New subject: [libvirt] [PATCH 2/2] conf: Fix leaks in virNetworkObjUpdateParseFile
On 01/25/13 20:15, Laine Stump wrote:
On 01/25/2013 10:36 AM, Ján Tomko wrote:
> Free the bitmap before calling virBitmapParse, which will overwrite it.
>
> Also free xml.
> ---
> src/conf/network_conf.c | 19 +++++++++++--------
> 1 files changed, 11 insertions(+), 8 deletions(-)
>
> diff --git a/src/conf/network_conf.c b/src/conf/network_conf.c
> index c93916d..013333c 100644
> --- a/src/conf/network_conf.c
> +++ b/src/conf/network_conf.c
> @@ -1855,14 +1855,16 @@ virNetworkObjUpdateParseFile(const char *filename,
>
> ctxt->node = node;
> class_id = virXPathString("string(./class_id[1]/@bitmap)", ctxt);
> - if (class_id &&
> - virBitmapParse(class_id, 0,
> - &net->class_id, CLASS_ID_BITMAP_SIZE) < 0) {
> - virReportError(VIR_ERR_INTERNAL_ERROR,
> - _("Malformed 'class_id' attribute:
%s"),
> - class_id);
> - VIR_FREE(class_id);
> - goto cleanup;
> + if (class_id) {
> + virBitmapFree(net->class_id);
If there is a situation where this function can be called and
net->class_id is already filled in, then doesn't that also mean that
net->floor_sum could have already been set?
If that's the case, then we need to also set net->floor_sum to 0, in
case it was previously non-0 and the new status doesn't have anything
set (implying 0).
If not, then this virBitmapFree() is a NOP.
This function only gets called on the network driver initialization,
shortly after net->class_id gets allocated and filled with 111 in
virNetworkAssignDef (see valgrind log below). net->floor_sum is always
zero at this point.
It looks like networkFindActiveConfigs (which calls this function) is
missing from networkReload.
Also, net->class_id or net->floor_sum might get changed by this
function, even if the rest of the status file could not be parsed.
Jan
==22881== 8,216 (24 direct, 8,192 indirect) bytes in 1 blocks are
definitely lost in loss record 852 of 860
==22881== at 0x4C2A462: calloc (vg_replace_malloc.c:593)
==22881== by 0x528E0C1: virAlloc (viralloc.c:100)
==22881== by 0x528FB9E: virBitmapNew (virbitmap.c:74)
==22881== by 0x530F61F: virNetworkAssignDef (network_conf.c:347)
==22881== by 0x530F821: virNetworkLoadConfig (network_conf.c:2392)
==22881== by 0x530F9AA: virNetworkLoadAllConfigs (network_conf.c:2437)
==22881== by 0x5104D9: networkStartup (bridge_driver.c:419)
==22881== by 0x533EBF7: virStateInitialize (libvirt.c:822)
==22881== by 0x428E8C: daemonRunStateInit (libvirtd.c:877)
==22881== by 0x52CC815: virThreadHelper (virthreadpthread.c:161)
==22881== by 0x805DEC5: start_thread (pthread_create.c:305)
==22881== by 0x87636EC: clone (clone.S:115)
4315
days inactive
4319
days old
7 comments
3 participants
participants (3)
-
Ján Tomko -
Laine Stump -
Michal Privoznik