esx: Fix memory leaks in error paths related to transferred ownership

Appending an item to a list transfers ownership of that item to the
list owner. But an error can occur in between item allocation and
appending it to the list. In this case the item has to be freed
explicitly. This was not done in some special cases resulting in
possible memory leaks.

Reported by Coverity.

diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c
index d9f53f7418dca696cb6570dd852ba3644166f066..b3f19481c3163dc216e7ad26af949d95151d9e98 100644 (file)
--- a/src/esx/esx_driver.c
+++ b/src/esx/esx_driver.c
@@ -3489,6 +3489,7 @@ esxDomainSetAutostart(virDomainPtr domain, int autostart)
     esxVI_AutoStartPowerInfo *powerInfoList = NULL;
     esxVI_AutoStartPowerInfo *powerInfo = NULL;
     esxVI_AutoStartPowerInfo *newPowerInfo = NULL;
+    bool newPowerInfo_isAppended = false;
 
     if (esxVI_EnsureSession(priv->primary) < 0) {
         return -1;
@@ -3546,9 +3547,7 @@ esxDomainSetAutostart(virDomainPtr domain, int autostart)
     if (esxVI_AutoStartPowerInfo_Alloc(&newPowerInfo) < 0 ||
         esxVI_Int_Alloc(&newPowerInfo->startOrder) < 0 ||
         esxVI_Int_Alloc(&newPowerInfo->startDelay) < 0 ||
-        esxVI_Int_Alloc(&newPowerInfo->stopDelay) < 0 ||
-        esxVI_AutoStartPowerInfo_AppendToList(&spec->powerInfo,
-                                              newPowerInfo) < 0) {
+        esxVI_Int_Alloc(&newPowerInfo->stopDelay) < 0) {
         goto cleanup;
     }
 
@@ -3560,6 +3559,13 @@ esxDomainSetAutostart(virDomainPtr domain, int autostart)
     newPowerInfo->stopDelay->value = -1; /* use system default */
     newPowerInfo->stopAction = (char *)"none";
 
+    if (esxVI_AutoStartPowerInfo_AppendToList(&spec->powerInfo,
+                                              newPowerInfo) < 0) {
+        goto cleanup;
+    }
+
+    newPowerInfo_isAppended = true;
+
     if (esxVI_ReconfigureAutostart
           (priv->primary,
            priv->primary->hostSystem->configManager->autoStartManager,
@@ -3581,6 +3587,10 @@ esxDomainSetAutostart(virDomainPtr domain, int autostart)
     esxVI_AutoStartDefaults_Free(&defaults);
     esxVI_AutoStartPowerInfo_Free(&powerInfoList);
 
+    if (!newPowerInfo_isAppended) {
+        esxVI_AutoStartPowerInfo_Free(&newPowerInfo);
+    }
+
     return result;
 }
 

diff --git a/src/esx/esx_vi.c b/src/esx/esx_vi.c
index 42e0976fe478dc88e5179b15ebfb5bf0e0c31d74..5b5ab696d0aba01058800fb6913114eef7f0f8c3 100644 (file)
--- a/src/esx/esx_vi.c
+++ b/src/esx/esx_vi.c
@@ -1882,7 +1882,9 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
 {
     int result = -1;
     esxVI_ObjectSpec *objectSpec = NULL;
+    bool objectSpec_isAppended = false;
     esxVI_PropertySpec *propertySpec = NULL;
+    bool propertySpec_isAppended = false;
     esxVI_PropertyFilterSpec *propertyFilterSpec = NULL;
 
     if (objectContentList == NULL || *objectContentList != NULL) {
@@ -1951,10 +1953,20 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
 
     if (esxVI_PropertyFilterSpec_Alloc(&propertyFilterSpec) < 0 ||
         esxVI_PropertySpec_AppendToList(&propertyFilterSpec->propSet,
-                                        propertySpec) < 0 ||
-        esxVI_ObjectSpec_AppendToList(&propertyFilterSpec->objectSet,
-                                      objectSpec) < 0 ||
-        esxVI_RetrieveProperties(ctx, propertyFilterSpec,
+                                        propertySpec) < 0) {
+        goto cleanup;
+    }
+
+    propertySpec_isAppended = true;
+
+    if (esxVI_ObjectSpec_AppendToList(&propertyFilterSpec->objectSet,
+                                      objectSpec) < 0) {
+        goto cleanup;
+    }
+
+    objectSpec_isAppended = true;
+
+    if (esxVI_RetrieveProperties(ctx, propertyFilterSpec,
                                  objectContentList) < 0) {
         goto cleanup;
     }
@@ -1994,13 +2006,24 @@ esxVI_LookupObjectContentByType(esxVI_Context *ctx,
      * Remove values given by the caller from the data structures to prevent
      * them from being freed by the call to esxVI_PropertyFilterSpec_Free().
      */
-    objectSpec->obj = NULL;
-    objectSpec->selectSet = NULL;
+    if (objectSpec != NULL) {
+        objectSpec->obj = NULL;
+        objectSpec->selectSet = NULL;
+    }
+
     if (propertySpec != NULL) {
         propertySpec->type = NULL;
         propertySpec->pathSet = NULL;
     }
 
+    if (!objectSpec_isAppended) {
+        esxVI_ObjectSpec_Free(&objectSpec);
+    }
+
+    if (!propertySpec_isAppended) {
+        esxVI_PropertySpec_Free(&propertySpec);
+    }
+
     esxVI_PropertyFilterSpec_Free(&propertyFilterSpec);
 
     return result;
@@ -3885,7 +3908,9 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx,
 {
     int result = -1;
     esxVI_ObjectSpec *objectSpec = NULL;
+    bool objectSpec_isAppended = false;
     esxVI_PropertySpec *propertySpec = NULL;
+    bool propertySpec_isAppended = false;
     esxVI_PropertyFilterSpec *propertyFilterSpec = NULL;
     esxVI_ManagedObjectReference *propertyFilter = NULL;
     char *version = NULL;
@@ -3927,10 +3952,20 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx,
                                        "info.state") < 0 ||
         esxVI_PropertyFilterSpec_Alloc(&propertyFilterSpec) < 0 ||
         esxVI_PropertySpec_AppendToList(&propertyFilterSpec->propSet,
-                                        propertySpec) < 0 ||
-        esxVI_ObjectSpec_AppendToList(&propertyFilterSpec->objectSet,
-                                      objectSpec) < 0 ||
-        esxVI_CreateFilter(ctx, propertyFilterSpec, esxVI_Boolean_True,
+                                        propertySpec) < 0) {
+        goto cleanup;
+    }
+
+    propertySpec_isAppended = true;
+
+    if (esxVI_ObjectSpec_AppendToList(&propertyFilterSpec->objectSet,
+                                      objectSpec) < 0) {
+        goto cleanup;
+    }
+
+    objectSpec_isAppended = true;
+
+    if (esxVI_CreateFilter(ctx, propertyFilterSpec, esxVI_Boolean_True,
                            &propertyFilter) < 0) {
         goto cleanup;
     }
@@ -4066,6 +4101,14 @@ esxVI_WaitForTaskCompletion(esxVI_Context *ctx,
         propertySpec->type = NULL;
     }
 
+    if (!objectSpec_isAppended) {
+        esxVI_ObjectSpec_Free(&objectSpec);
+    }
+
+    if (!propertySpec_isAppended) {
+        esxVI_PropertySpec_Free(&propertySpec);
+    }
+
     esxVI_PropertyFilterSpec_Free(&propertyFilterSpec);
     esxVI_ManagedObjectReference_Free(&propertyFilter);
     VIR_FREE(version);