11:36 a.m.
Detected by Coverity.
* daemon/remote.c: Calling allocation function 'get_nonnull_domain' to
allocate memory to 'dom', but haven't released it in 'cleanup'
section.
* src/qemu/qemu_command.c: Taking 'if (!port)' true branch then 'def =
NULL',
the codes jump into 'error' section, the function
'virDomainDefFree(def)'
willn't release it.
* src/remote/remote_driver.c: Calling allocation function 'get_nonnull_domain' to
allocate memory to 'dom', but haven't released it in 'no_memory'
section.
* src/util/pci.c: Calling allocation function 'pciGetVirtualFunctions' on
'virt_fns', but haven't released it in 'out' section.
* src/vmx/vmx.c: Calling allocation function 'virConfReadMem' to allocate
memory to 'conf' and then taking 'if (VIR_ALLOC(def) < 0)' true
branch,
the codes haven't released it before 'return NULL'.
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
daemon/remote.c | 2 ++
src/qemu/qemu_command.c | 1 -
src/remote/remote_driver.c | 2 ++
src/util/pci.c | 1 +
src/vmx/vmx.c | 1 +
5 files changed, 6 insertions(+), 1 deletions(-)
11:36 a.m.
New subject: [libvirt] [PATCH 1/5] daemon: avoid memory leak
* daemon/remote.c: fix memory leak.
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
daemon/remote.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 38bbb10..45244f8 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -997,6 +997,8 @@ cleanup:
VIR_FREE(ret->params.params_val);
}
}
+ if (dom)
+ virDomainFree(dom);
VIR_FREE(params);
return rv;
}
--
1.7.1
2:03 p.m.
New subject: [libvirt] [PATCH 1/5] daemon: avoid memory leak
On 09/18/2011 10:36 AM, ajia(a)redhat.com wrote:
* daemon/remote.c: fix memory leak.
Your commit message didn't mention which function you fixed,...
Signed-off-by: Alex Jia<ajia(a)redhat.com>
---
daemon/remote.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/daemon/remote.c b/daemon/remote.c
index 38bbb10..45244f8 100644
--- a/daemon/remote.c
+++ b/daemon/remote.c
@@ -997,6 +997,8 @@ cleanup:
and git didn't list it either, so I had to look. I tweaked the commit
message to mention remoteDispatchDomainBlockStatsFlags, and that the
leak was introduced in commit efa7fc9.
VIR_FREE(ret->params.params_val);
}
}
+ if (dom)
+ virDomainFree(dom);
VIR_FREE(params);
return rv;
ACK and pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
9:38 p.m.
New subject: [libvirt] [PATCH 1/5] daemon: avoid memory leak
On 09/20/2011 03:03 AM, Eric Blake wrote:
On 09/18/2011 10:36 AM, ajia(a)redhat.com wrote:
> * daemon/remote.c: fix memory leak.
Your commit message didn't mention which function you fixed,...
Oh, I think 'git' will do this , and sorry about any inconvenient for you.
Thanks,
Alex
>
> Signed-off-by: Alex Jia<ajia(a)redhat.com>
> ---
> daemon/remote.c | 2 ++
> 1 files changed, 2 insertions(+), 0 deletions(-)
>
> diff --git a/daemon/remote.c b/daemon/remote.c
> index 38bbb10..45244f8 100644
> --- a/daemon/remote.c
> +++ b/daemon/remote.c
> @@ -997,6 +997,8 @@ cleanup:
and git didn't list it either, so I had to look. I tweaked the commit
message to mention remoteDispatchDomainBlockStatsFlags, and that the
leak was introduced in commit efa7fc9.
> VIR_FREE(ret->params.params_val);
> }
> }
> + if (dom)
> + virDomainFree(dom);
> VIR_FREE(params);
> return rv;
ACK and pushed.
11:36 a.m.
New subject: [libvirt] [PATCH 2/5] qemu: avoid memory leak
* src/qemu/qemu_command.c: fix memory leak.
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
src/qemu/qemu_command.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e8b1157..45d55fc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6540,7 +6540,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
host = disk->src;
port = strchr(host, ':');
if (!port) {
- def = NULL;
qemuReportError(VIR_ERR_INTERNAL_ERROR,
_("cannot parse nbd filename
'%s'"), disk->src);
goto error;
--
1.7.1
2:25 p.m.
New subject: [libvirt] [PATCH 2/5] qemu: avoid memory leak
On 09/18/2011 10:36 AM, ajia(a)redhat.com wrote:
* src/qemu/qemu_command.c: fix memory leak.
Signed-off-by: Alex Jia<ajia(a)redhat.com>
---
src/qemu/qemu_command.c | 1 -
1 files changed, 0 insertions(+), 1 deletions(-)
diff --git a/src/qemu/qemu_command.c b/src/qemu/qemu_command.c
index e8b1157..45d55fc 100644
--- a/src/qemu/qemu_command.c
+++ b/src/qemu/qemu_command.c
@@ -6540,7 +6540,6 @@ virDomainDefPtr qemuParseCommandLine(virCapsPtr caps,
host = disk->src;
port = strchr(host, ':');
if (!port) {
- def = NULL;
Took me a few minutes of research to find when this stray line was
introduced - we've had a leak on error since commit 036ad50 (0.8.7).
ACK and pushed.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:36 a.m.
New subject: [libvirt] [PATCH 3/5] remote: avoid memory leak
* src/remote/remote_driver.c: fix memory leak.
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
src/remote/remote_driver.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 9d34b7e..9f30432 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -3294,6 +3294,8 @@ no_memory:
VIR_FREE(subject->identities);
VIR_FREE(subject);
}
+ if (dom)
+ virDomainFree(dom);
return;
}
--
1.7.1
2:34 p.m.
New subject: [libvirt] [PATCH 3/5] remote: avoid memory leak
On 09/18/2011 10:36 AM, ajia(a)redhat.com wrote:
* src/remote/remote_driver.c: fix memory leak.
Signed-off-by: Alex Jia<ajia(a)redhat.com>
---
src/remote/remote_driver.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/src/remote/remote_driver.c b/src/remote/remote_driver.c
index 9d34b7e..9f30432 100644
--- a/src/remote/remote_driver.c
+++ b/src/remote/remote_driver.c
@@ -3294,6 +3294,8 @@ no_memory:
VIR_FREE(subject->identities);
VIR_FREE(subject);
}
+ if (dom)
+ virDomainFree(dom);
ACK that this is a leak, in remoteDomainBuildEventGraphics and
introduced by commit 987e31e (v0.8.0). However, if we get here, we are
guaranteed dom is non-NULL, so I removed the redundant if() before pushing.
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:36 a.m.
New subject: [libvirt] [PATCH 4/5] util: avoid memory leak
* src/util/pci.c: fix memory leak.
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
src/util/pci.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index 9873f33..8d8e157 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -1965,6 +1965,7 @@ out:
for (i = 0; i < num_virt_fns; i++)
VIR_FREE(virt_fns[i]);
+ VIR_FREE(virt_fns);
VIR_FREE(vf_bdf);
return ret;
--
1.7.1
2:41 p.m.
New subject: [libvirt] [PATCH 4/5] util: avoid memory leak
On 09/18/2011 10:36 AM, ajia(a)redhat.com wrote:
* src/util/pci.c: fix memory leak.
Signed-off-by: Alex Jia<ajia(a)redhat.com>
---
src/util/pci.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/util/pci.c b/src/util/pci.c
index 9873f33..8d8e157 100644
--- a/src/util/pci.c
+++ b/src/util/pci.c
@@ -1965,6 +1965,7 @@ out:
for (i = 0; i< num_virt_fns; i++)
VIR_FREE(virt_fns[i]);
+ VIR_FREE(virt_fns);
ACK and pushed; leak introduced in pciGetVirtualFunctionIndex,
introduced by commit 17d64ca (unreleased).
--
Eric Blake eblake(a)redhat.com +1-801-349-2682
Libvirt virtualization library http://libvirt.org
11:36 a.m.
New subject: [libvirt] [PATCH 5/5] vmx: avoid memory leak
* src/vmx/vmx.c: fix memory leak.
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
src/vmx/vmx.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index dff3599..1ab15b7 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1253,6 +1253,7 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char
*vmx)
/* Allocate domain def */
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
+ virConfFree(conf);
return NULL;
}
--
1.7.1
11:47 a.m.
New subject: [libvirt] [PATCH 5/5] vmx: avoid memory leak
2011/9/18 <ajia(a)redhat.com>:
* src/vmx/vmx.c: fix memory leak.
Signed-off-by: Alex Jia <ajia(a)redhat.com>
---
src/vmx/vmx.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
index dff3599..1ab15b7 100644
--- a/src/vmx/vmx.c
+++ b/src/vmx/vmx.c
@@ -1253,6 +1253,7 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const char
*vmx)
/* Allocate domain def */
if (VIR_ALLOC(def) < 0) {
virReportOOMError();
+ virConfFree(conf);
return NULL;
}
Your patch is correct.
But I'd suggest to replace the 'return NULL' with a 'goto cleanup'
here instead of adding a virConfFree, as this is more in line with the
surrounding code.
Either way ACK.
--
Matthias Bolte
http://photron.blogspot.com
9:31 p.m.
New subject: [libvirt] [PATCH 5/5] vmx: avoid memory leak
On 09/19/2011 12:47 AM, Matthias Bolte wrote:
2011/9/18<ajia(a)redhat.com>:
> * src/vmx/vmx.c: fix memory leak.
>
> Signed-off-by: Alex Jia<ajia(a)redhat.com>
> ---
> src/vmx/vmx.c | 1 +
> 1 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/src/vmx/vmx.c b/src/vmx/vmx.c
> index dff3599..1ab15b7 100644
> --- a/src/vmx/vmx.c
> +++ b/src/vmx/vmx.c
> @@ -1253,6 +1253,7 @@ virVMXParseConfig(virVMXContext *ctx, virCapsPtr caps, const
char *vmx)
> /* Allocate domain def */
> if (VIR_ALLOC(def)< 0) {
> virReportOOMError();
> + virConfFree(conf);
> return NULL;
> }
Your patch is correct.
But I'd suggest to replace the 'return NULL' with a 'goto cleanup'
here instead of adding a virConfFree, as this is more in line with the
surrounding code.
Either way ACK.
Yeah, 'def' has a initial value 'NULL', so 'goto cleanup'
is perfected
instead of adding a virConfFree.
Matthias, thanks for your good suggestion, I will commit v2 patch for this.
Regards,
Alex
4812
days inactive
4814
days old
12 comments
4 participants
participants (4)
-
ajia＠redhat.com -
Alex Jia
-
Eric Blake -
Matthias Bolte