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@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