From: Michal Privoznik <mprivozn@redhat.com>
Date: Fri, 15 Oct 2021 11:39:36 +0000 (+0200)
Subject: virnetsocket: Don't free virCommand in virNetSocketNewConnectCommand()
X-Git-Tag: v7.9.0-rc1~95
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=44d8324f13006cdd4cca0d0fc4c52b7ca80b2455;p=thirdparty%2Flibvirt.git

virnetsocket: Don't free virCommand in virNetSocketNewConnectCommand()

The aim of virNetSocketNewConnectCommand() is to execute passed
command and attach socket pair/pipe to it so that client socket
can be opened (this is used for connections with alternative
transports, e.g. ssh). The virCommand is created in a caller and
then passed to virNetSocketNewConnectCommand() where it is freed
using virCommandFree(). This approach is wrong on two levels:

1) The deallocation happens on a different level than allocation,
2) There's a WIN32 stub that just reports an error and doesn't
   free the command.

However, with g_autoptr() trickery the command can be freed in
caller.

Signed-off-by: Michal Privoznik <mprivozn@redhat.com>
Reviewed-by: Ján Tomko <jtomko@redhat.com>
---

diff --git a/src/rpc/virnetsocket.c b/src/rpc/virnetsocket.c
index 212089520d..50c0c4ecc8 100644
--- a/src/rpc/virnetsocket.c
+++ b/src/rpc/virnetsocket.c
@@ -821,8 +821,6 @@ int virNetSocketNewConnectCommand(virCommand *cmd,
     if (!(*retsock = virNetSocketNew(NULL, NULL, true, sv[0], errfd[0], pid, false)))
         goto error;
 
-    virCommandFree(cmd);
-
     return 0;
 
  error:
@@ -832,7 +830,6 @@ int virNetSocketNewConnectCommand(virCommand *cmd,
     VIR_FORCE_CLOSE(errfd[1]);
 
     virCommandAbort(cmd);
-    virCommandFree(cmd);
 
     return -1;
 }
@@ -856,7 +853,7 @@ int virNetSocketNewConnectSSH(const char *nodename,
                               const char *command,
                               virNetSocket **retsock)
 {
-    virCommand *cmd;
+    g_autoptr(virCommand) cmd = NULL;
 
     *retsock = NULL;
 
@@ -1154,7 +1151,7 @@ virNetSocketNewConnectLibssh(const char *host G_GNUC_UNUSED,
 int virNetSocketNewConnectExternal(const char **cmdargv,
                                    virNetSocket **retsock)
 {
-    virCommand *cmd;
+    g_autoptr(virCommand) cmd = NULL;
 
     *retsock = NULL;
 
diff --git a/tests/virnetsockettest.c b/tests/virnetsockettest.c
index d24baa9a5c..33b8755ab7 100644
--- a/tests/virnetsockettest.c
+++ b/tests/virnetsockettest.c
@@ -388,7 +388,8 @@ static int testSocketCommandNormal(const void *data G_GNUC_UNUSED)
     char buf[100];
     size_t i;
     int ret = -1;
-    virCommand *cmd = virCommandNewArgList("/bin/cat", "/dev/zero", NULL);
+    g_autoptr(virCommand) cmd = virCommandNewArgList("/bin/cat", "/dev/zero", NULL);
+
     virCommandAddEnvPassCommon(cmd);
 
     if (virNetSocketNewConnectCommand(cmd, &csock) < 0)
@@ -415,7 +416,8 @@ static int testSocketCommandFail(const void *data G_GNUC_UNUSED)
     virNetSocket *csock = NULL; /* Client socket */
     char buf[100];
     int ret = -1;
-    virCommand *cmd = virCommandNewArgList("/bin/cat", "/dev/does-not-exist", NULL);
+    g_autoptr(virCommand) cmd = virCommandNewArgList("/bin/cat", "/dev/does-not-exist", NULL);
+
     virCommandAddEnvPassCommon(cmd);
 
     if (virNetSocketNewConnectCommand(cmd, &csock) < 0)