[PATCH] esx: Avoid Coverity warning about resource leak in esxOpen

[Matthias Bolte <matthias.bolte@xxxxxxxxxxxxxx>]

Commit 4445e16bfa8056980ac643fabf17186f9e685925 changed the signature
of esxConnectToHost and esxConnectToVCenter by replacing the esxPrivate
pointer with virConnectPtr. The esxPrivate pointer was then retrieved
again from virConnectPtr's privateData. This resulted in a NULL pointer
dereference, because the privateData pointer was not yet initialized at
the point where esxConnectToHost and esxConnectToVCenter are called.

This was fixed in commit b126715a48cd0cbe32ec6468c267cd8cf2961c55 that
moved the initialization of privateData before the problematic calls.

Coverity tagged the esxPrivate pointer a potentially leaked because of
the conditional free call. But this is a false positive, there is not
actual leak.

Avoid this warning from Coverity by making the call to esxFreePrivate
unconditional and changing esxConnectToHost and esxConnectToVCenter back
to take a esxPrivate pointer directly. This allows to assign esxPrivate
to the virConnectPtr's privateData pointer as one of the last steps in
esxOpen making it more obvious that it is not initialized during the
earlier steps of esxOpen.
---

This patch is meant as a replacement this patch:

https://www.redhat.com/archives/libvir-list/2013-January/msg00530.html

 src/esx/esx_driver.c |   23 ++++++++++-------------
 1 file changed, 10 insertions(+), 13 deletions(-)

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index 1366c81..dad10a1 100644
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -650,7 +650,8 @@ esxCapsInit(esxPrivate *priv)
 
 
 static int
-esxConnectToHost(virConnectPtr conn,
+esxConnectToHost(esxPrivate *priv,
+                 virConnectPtr conn,
                  virConnectAuthPtr auth,
                  char **vCenterIpAddress)
 {
@@ -663,7 +664,6 @@ esxConnectToHost(virConnectPtr conn,
     esxVI_String *propertyNameList = NULL;
     esxVI_ObjectContent *hostSystem = NULL;
     esxVI_Boolean inMaintenanceMode = esxVI_Boolean_Undefined;
-    esxPrivate *priv = conn->privateData;
     esxVI_ProductVersion expectedProductVersion = STRCASEEQ(conn->uri->scheme, "esx")
         ? esxVI_ProductVersion_ESX
         : esxVI_ProductVersion_GSX;
@@ -785,7 +785,8 @@ esxConnectToHost(virConnectPtr conn,
 
 
 static int
-esxConnectToVCenter(virConnectPtr conn,
+esxConnectToVCenter(esxPrivate *priv,
+                    virConnectPtr conn,
                     virConnectAuthPtr auth,
                     const char *hostname,
                     const char *hostSystemIpAddress)
@@ -796,7 +797,6 @@ esxConnectToVCenter(virConnectPtr conn,
     char *unescapedPassword = NULL;
     char *password = NULL;
     char *url = NULL;
-    esxPrivate *priv = conn->privateData;
 
     if (hostSystemIpAddress == NULL &&
         (priv->parsedUri->path == NULL || STREQ(priv->parsedUri->path, "/"))) {
@@ -1008,8 +1008,6 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
     priv->supportsLongMode = esxVI_Boolean_Undefined;
     priv->usedCpuTimeCounterId = -1;
 
-    conn->privateData = priv;
-
     /*
      * Set the port dependent on the transport protocol if no port is
      * specified. This allows us to rely on the port parameter being
@@ -1036,7 +1034,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
     if (STRCASEEQ(conn->uri->scheme, "esx") ||
         STRCASEEQ(conn->uri->scheme, "gsx")) {
         /* Connect to host */
-        if (esxConnectToHost(conn, auth,
+        if (esxConnectToHost(priv, conn, auth,
                              &potentialVCenterIpAddress) < 0) {
             goto cleanup;
         }
@@ -1075,7 +1073,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
                 }
             }
 
-            if (esxConnectToVCenter(conn, auth,
+            if (esxConnectToVCenter(priv, conn, auth,
                                     vCenterIpAddress,
                                     priv->host->ipAddress) < 0) {
                 goto cleanup;
@@ -1085,7 +1083,7 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
         priv->primary = priv->host;
     } else { /* VPX */
         /* Connect to vCenter */
-        if (esxConnectToVCenter(conn, auth,
+        if (esxConnectToVCenter(priv, conn, auth,
                                 conn->uri->server,
                                 NULL) < 0) {
             goto cleanup;
@@ -1101,13 +1099,12 @@ esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
         goto cleanup;
     }
 
+    conn->privateData = priv;
+    priv = NULL;
     result = VIR_DRV_OPEN_SUCCESS;
 
   cleanup:
-    if (result == VIR_DRV_OPEN_ERROR) {
-        esxFreePrivate(&priv);
-    }
-
+    esxFreePrivate(&priv);
     VIR_FREE(potentialVCenterIpAddress);
 
     return result;
-- 
1.7.9.5

--
libvir-list mailing list
libvir-list@xxxxxxxxxx
https://www.redhat.com/mailman/listinfo/libvir-list