[libvirt] [PATCH 0/6] Coverity patches to resolve RESOURCE_LEAK
Another six pathes to fix resource leak. But this may not be the end. Wang Rui (6): tests: Resolve Coverity RESOURCE_LEAK in commandhelper test_conf: Resolve Coverity RESOURCE_LEAK remote: Resolve Coverity RESOURCE_LEAK qemu_process: Resolve Coverity RESOURCE_LEAK vircgroup: Resolve Coverity RESOURCE_LEAK lxc_container: Resolve Coverity RESOURCE_LEAK daemon/remote.c | 4 +++- src/lxc/lxc_container.c | 4 ++++ src/qemu/qemu_process.c | 1 + src/util/vircgroup.c | 2 +- tests/commandhelper.c | 15 +++++++++------ tests/test_conf.c | 4 ++-- 6 files changed, 20 insertions(+), 10 deletions(-) -- 1.7.12.4
Coverity determined that 'log' and 'newenv' were not freed in some cases. Free them in 'error' branch and normal branch. Signed-off-by: Wang Rui <moon.wangrui@huawei.com> --- tests/commandhelper.c | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/tests/commandhelper.c b/tests/commandhelper.c index 796b89d..0bba0d6 100644 --- a/tests/commandhelper.c +++ b/tests/commandhelper.c @@ -61,7 +61,7 @@ int main(int argc, char **argv) { size_t i, n; int open_max; char **origenv; - char **newenv; + char **newenv = NULL; char *cwd; FILE *log = fopen(abs_builddir "/commandhelper.log", "w"); @@ -80,7 +80,7 @@ int main(int argc, char **argv) { } if (VIR_ALLOC_N_QUIET(newenv, n) < 0) - return EXIT_FAILURE; + goto error; origenv = environ; n = i = 0; @@ -100,7 +100,7 @@ int main(int argc, char **argv) { open_max = sysconf(_SC_OPEN_MAX); if (open_max < 0) - return EXIT_FAILURE; + goto error; for (i = 0; i < open_max; i++) { int f; int closed; @@ -114,15 +114,13 @@ int main(int argc, char **argv) { fprintf(log, "DAEMON:%s\n", getpgrp() == getsid(0) ? "yes" : "no"); if (!(cwd = getcwd(NULL, 0))) - return EXIT_FAILURE; + goto error; if (strlen(cwd) > strlen(".../commanddata") && STREQ(cwd + strlen(cwd) - strlen("/commanddata"), "/commanddata")) strcpy(cwd, ".../commanddata"); fprintf(log, "CWD:%s\n", cwd); VIR_FREE(cwd); - VIR_FORCE_FCLOSE(log); - if (argc > 1 && STREQ(argv[1], "--close-stdin")) { if (freopen("/dev/null", "r", stdin) != stdin) goto error; @@ -154,9 +152,14 @@ int main(int argc, char **argv) { fprintf(stderr, "END STDERR\n"); fflush(stderr); + VIR_FORCE_FCLOSE(log); + VIR_FREE(newenv); + return EXIT_SUCCESS; error: + VIR_FORCE_FCLOSE(log); + VIR_FREE(newenv); return EXIT_FAILURE; } -- 1.7.12.4
If the condition 'ret < 0' is true, the code will jump to 'cleanup' and 'conf' won't be freed. Signed-off-by: Wang Rui <moon.wangrui@huawei.com> --- tests/test_conf.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/test_conf.c b/tests/test_conf.c index 05704df..4d05d8d 100644 --- a/tests/test_conf.c +++ b/tests/test_conf.c @@ -11,7 +11,7 @@ int main(int argc, char **argv) { int ret, exit_code = EXIT_FAILURE; - virConfPtr conf; + virConfPtr conf = NULL; int len = 10000; char *buffer = NULL; @@ -34,7 +34,6 @@ int main(int argc, char **argv) fprintf(stderr, "Failed to serialize %s back\n", argv[1]); goto cleanup; } - virConfFree(conf); if (fwrite(buffer, 1, len, stdout) != len) { fprintf(stderr, "Write failed: %s\n", strerror(errno)); goto cleanup; @@ -44,5 +43,6 @@ int main(int argc, char **argv) cleanup: VIR_FREE(buffer); + virConfFree(conf); return exit_code; } -- 1.7.12.4
Need to free 'uri_out' on error path. Signed-off-by: Wang Rui <moon.wangrui@huawei.com> --- daemon/remote.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/daemon/remote.c b/daemon/remote.c index 89714ca..0ea2815 100644 --- a/daemon/remote.c +++ b/daemon/remote.c @@ -2305,8 +2305,10 @@ remoteDispatchDomainMigratePrepare2(virNetServerPtr server ATTRIBUTE_UNUSED, rv = 0; cleanup: - if (rv < 0) + if (rv < 0) { virNetMessageSaveError(rerr); + VIR_FREE(uri_out); + } return rv; } -- 1.7.12.4
If virSecurityManagerClearSocketLabel() fails, 'agent' won't be freed before jumping to cleanup. Signed-off-by: Wang Rui <moon.wangrui@huawei.com> --- src/qemu/qemu_process.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/qemu/qemu_process.c b/src/qemu/qemu_process.c index f68dfbe..79f4238 100644 --- a/src/qemu/qemu_process.c +++ b/src/qemu/qemu_process.c @@ -264,6 +264,7 @@ qemuConnectAgent(virQEMUDriverPtr driver, virDomainObjPtr vm) vm->def) < 0) { VIR_ERROR(_("Failed to clear security context for agent for %s"), vm->def->name); + qemuAgentClose(agent); goto cleanup; } -- 1.7.12.4
Need to free 'root' and 'opts' before 'return -1' if symlink fails. Signed-off-by: Wang Rui <moon.wangrui@huawei.com> --- src/util/vircgroup.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/util/vircgroup.c b/src/util/vircgroup.c index 8b554a9..a64f081 100644 --- a/src/util/vircgroup.c +++ b/src/util/vircgroup.c @@ -3757,7 +3757,7 @@ virCgroupIsolateMount(virCgroupPtr group, const char *oldroot, _("Unable to symlink directory %s to %s"), group->controllers[i].mountPoint, group->controllers[i].linkPoint); - return -1; + goto cleanup; } } } -- 1.7.12.4
Memory is allocated for 'mnt_src' by VIR_STRDUP in the loop. Next loop it will be allocated again. So we need to free 'mnt_src' before continue the loop. Signed-off-by: Wang Rui <moon.wangrui@huawei.com> --- src/lxc/lxc_container.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lxc/lxc_container.c b/src/lxc/lxc_container.c index 1cf2c8f..2a3ec48 100644 --- a/src/lxc/lxc_container.c +++ b/src/lxc/lxc_container.c @@ -886,12 +886,14 @@ static int lxcContainerMountBasicFS(bool userns_enabled, if (ret == 0) { VIR_DEBUG("Skipping '%s' which isn't mounted in host", mnt->dst); + VIR_FREE(mnt_src); continue; } } if (mnt->skipUserNS && userns_enabled) { VIR_DEBUG("Skipping due to user ns enablement"); + VIR_FREE(mnt_src); continue; } @@ -930,6 +932,8 @@ static int lxcContainerMountBasicFS(bool userns_enabled, MS_BIND|MS_REMOUNT|MS_RDONLY); goto cleanup; } + + VIR_FREE(mnt_src); } rc = 0; -- 1.7.12.4
On 09/01/2014 08:08 AM, Wang Rui wrote:
Another six pathes to fix resource leak. But this may not be the end.
Wang Rui (6): tests: Resolve Coverity RESOURCE_LEAK in commandhelper test_conf: Resolve Coverity RESOURCE_LEAK remote: Resolve Coverity RESOURCE_LEAK qemu_process: Resolve Coverity RESOURCE_LEAK vircgroup: Resolve Coverity RESOURCE_LEAK lxc_container: Resolve Coverity RESOURCE_LEAK
daemon/remote.c | 4 +++- src/lxc/lxc_container.c | 4 ++++ src/qemu/qemu_process.c | 1 + src/util/vircgroup.c | 2 +- tests/commandhelper.c | 15 +++++++++------ tests/test_conf.c | 4 ++-- 6 files changed, 20 insertions(+), 10 deletions(-)
ACK series - made a couple of adjustments prior to push.. Patch1 didn't apply cleanly after '0c07d360f' - refactored a bit to change "error" to "cleanup" and then use 'ret = EXIT_FAILURE' at initialization time and 'ret = EXIT_SUCCESS' when everything succeeds, allowing just a return ret Patch6 needed a few extra spaces in the last VIR_FREE John
participants (2)
-
John Ferlan -
Wang Rui