From 856a23c2bc9cc3a6188f329c6e0cd23ff8667f5b Mon Sep 17 00:00:00 2001 From: Matthias Bolte Date: Sun, 6 May 2012 19:33:59 +0200 Subject: [PATCH] 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. (cherry picked from commit 3b9a12958d822792c6f5309c2bf44acb4356c0c8) --- src/esx/esx_driver.c | 16 ++++++++--- src/esx/esx_vi.c | 63 +++++++++++++++++++++++++++++++++++++------- 2 files changed, 66 insertions(+), 13 deletions(-) diff --git a/src/esx/esx_driver.c b/src/esx/esx_driver.c index d9f53f7418..b3f19481c3 100644 --- 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 42e0976fe4..5b5ab696d0 100644 --- 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);