Re: [libvirt] [PATCH 2/3] virnetsocket: Resolve Coverity RESOURCE_LEAK
On 22.08.2014 17:28, John Ferlan wrote:
Since '1b807f92d' - Coverity complains that in the error
paths of
both virFork() and virProcessWait() that the 'passfd' will not be closed
Signed-off-by: John Ferlan <jferlan(a)redhat.com>
---
src/rpc/virnetsocket.c | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index f913365..ce908fa 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -593,8 +593,10 @@ int virNetSocketNewConnectUNIX(const char *path,
* behaviour on sockets according to POSIX, so it doesn't
* work outside Linux.
*/
- if ((pid = virFork()) < 0)
+ if ((pid = virFork()) < 0) {
+ VIR_FORCE_CLOSE(passfd);
goto error;
+ }
if (pid == 0) {
umask(0077);
@@ -604,8 +606,10 @@ int virNetSocketNewConnectUNIX(const char *path,
_exit(EXIT_SUCCESS);
}
- if (virProcessWait(pid, &status, false) < 0)
+ if (virProcessWait(pid, &status, false) < 0) {
+ VIR_FORCE_CLOSE(passfd);
goto error;
+ }
if (status != EXIT_SUCCESS) {
/*
Unfortunately not enough context is shown to express myself, so I'll paste interesting
knobs from the function:
if (listen(passfd, 0) < 0) {
virReportSystemError(errno, "%s",
_("Failed to listen on socket that's about
"
"to be passed to the daemon"));
goto error;
}
if (connect(fd, &remoteAddr.data.sa, remoteAddr.len) < 0) {
virReportSystemError(errno, _("Failed to connect socket to
'%s'"),
path);
goto error;
}
if (virNetSocketForkDaemon(binary, passfd) < 0)
goto error;
}
localAddr.len = sizeof(localAddr.data);
if (getsockname(fd, &localAddr.data.sa, &localAddr.len) < 0) {
virReportSystemError(errno, "%s", _("Unable to get local socket
name"));
goto error;
}
if (!(*retsock = virNetSocketNew(&localAddr, &remoteAddr, true, fd, -1, 0)))
goto error;
return 0;
error:
VIR_FREE(buf);
VIR_FORCE_CLOSE(fd);
if (spawnDaemon)
unlink(path);
return -1;
}
Here, if any of listen() or connect() fail, the control continues on the error label and
the @passfd is leaked again.
virNetSocketForkDaemon() is different - it closes passfd on failure. So I suggest moving
VIR_FORCE_CLOSE() under the error label.
Michal