[libvirt] [PATCH] esx: Avoid Coverity warning about resource leak in esxOpen
John Ferlan
jferlan at redhat.com
Tue Jan 22 18:30:21 UTC 2013
This issue (and the same for hyperv) was tracked down to a Coverity
problem. For context see:
https://www.redhat.com/archives/libvir-list/2013-January/msg01504.html
Essentially the esxFreePrivate(&priv); and subsequent VIR_FREE(*priv)
caused the issue.
Also see: https://communities.coverity.com/thread/2655
John
On 01/10/2013 04:47 PM, Matthias Bolte wrote:
> 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;
>
More information about the libvir-list
mailing list