From d051e7f703dfc97e2754d93d46eab8f515162b4c Mon Sep 17 00:00:00 2001
From: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Date: Fri, 18 Jan 2019 11:10:41 +0300
Subject: [PATCH] rpc: virNetClientNew: fix socket leak on error path
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

if virNetClientNew finishes with error before sock is set
to client object then sock does not get unrefed. This is
unexpected by function clients like virNetClientNewUNIX.
Let's make sure sock gets unrefed on any error path.

Next some clients like virNetClientNewLibSSH2 try to unref
sock on virNetClientNew errors. This is not correct even
before this patch because in some cases virNetClientNew
unrefed sock on error path by itself. Let's give up
sock managment to virNetClientNew entirely.

Signed-off-by: Nikolay Shirokovskiy <nshirokovskiy@virtuozzo.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/rpc/virnetclient.c | 8 +++-----
 1 file changed, 3 insertions(+), 5 deletions(-)

diff --git a/src/rpc/virnetclient.c b/src/rpc/virnetclient.c
index 2aced79384..b7ffdcb05b 100644
--- a/src/rpc/virnetclient.c
+++ b/src/rpc/virnetclient.c
@@ -299,7 +299,7 @@ static virNetClientPtr virNetClientNew(virNetSocketPtr sock,
     int wakeupFD[2] = { -1, -1 };
 
     if (virNetClientInitialize() < 0)
-        return NULL;
+        goto error;
 
     if (pipe2(wakeupFD, O_CLOEXEC) < 0) {
         virReportSystemError(errno, "%s",
@@ -311,6 +311,7 @@ static virNetClientPtr virNetClientNew(virNetSocketPtr sock,
         goto error;
 
     client->sock = sock;
+    sock = NULL;
     client->wakeupReadFD = wakeupFD[0];
     client->wakeupSendFD = wakeupFD[1];
     wakeupFD[0] = wakeupFD[1] = -1;
@@ -327,6 +328,7 @@ static virNetClientPtr virNetClientNew(virNetSocketPtr sock,
     VIR_FORCE_CLOSE(wakeupFD[0]);
     VIR_FORCE_CLOSE(wakeupFD[1]);
     virObjectUnref(client);
+    virObjectUnref(sock);
     return NULL;
 }
 
@@ -513,7 +515,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
 
     if (!(ret = virNetClientNew(sock, NULL)))
         goto cleanup;
-    sock = NULL;
 
  cleanup:
     VIR_FREE(command);
@@ -522,7 +523,6 @@ virNetClientPtr virNetClientNewLibSSH2(const char *host,
     VIR_FREE(homedir);
     VIR_FREE(confdir);
     VIR_FREE(nc);
-    virObjectUnref(sock);
     return ret;
 
  no_memory:
@@ -619,7 +619,6 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
 
     if (!(ret = virNetClientNew(sock, NULL)))
         goto cleanup;
-    sock = NULL;
 
  cleanup:
     VIR_FREE(command);
@@ -628,7 +627,6 @@ virNetClientPtr virNetClientNewLibssh(const char *host,
     VIR_FREE(homedir);
     VIR_FREE(confdir);
     VIR_FREE(nc);
-    virObjectUnref(sock);
     return ret;
 
  no_memory:
-- 
GitLab