[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

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



On 01/10/2013 07:28 PM, Eric Blake wrote:
> On 01/10/2013 05:14 PM, John Ferlan wrote:
>>
>> Still get a Coverity error, but I think it has something to do with the
>> VIR_FREE(*priv); done in the esxFreePrivate(). I'm beginning to believe
>> it's a Coverity "issue".  I have no idea why what I had done previously
>> was able to bypass the error.  I'm trying to figure out the right
>> mechanism to ask in the Coverity world.
> 
> Can you paste the actual Coverity analysis on this code?  Knowing what
> call stack it is tracing through will help others look at this setup to
> see why it is still complaining.
> 
The full analysis is a bit large so I cut it down a bit.  The attached
esx.html has the trace in question - the links within won't work, but it
hopefully gives you a picture of the trace.  The key is that the
esxFreePrivate(&priv); frees both the data in the structure and the
structure itself which is what I believe is confusing coverity.  If I
add a VIR_FREE(priv) after this call - then the error goes away even
though it's redundant with the VIR_FREE(*priv) in esxFreePrivate().


Title: Coverity error reader: /home/jferlan/libvirt.cov.curr/src/esx/esx_driver.c
891  	/*
892  	 * URI format: {vpx|esx|gsx}://[<username>@]<hostname>[:<port>]/[<path>][?<query parameter>...]
893  	 *             <path> = [<folder>/...]<datacenter>/[<folder>/...]<computeresource>[/<hostsystem>]
894  	 *
895  	 * If no port is specified the default port is set dependent on the scheme and
896  	 * transport parameter:
897  	 * - vpx+http  80
898  	 * - vpx+https 443
899  	 * - esx+http  80
900  	 * - esx+https 443
901  	 * - gsx+http  8222
902  	 * - gsx+https 8333
903  	 *
904  	 * For a vpx:// connection <path> references a host managed by the vCenter.
905  	 * In case the host is part of a cluster then <computeresource> is the cluster
906  	 * name. Otherwise <computeresource> and <hostsystem> are equal and the later
907  	 * can be omitted. As datacenters and computeresources can be organized in
908  	 * folders those have to be included in <path>.
909  	 *
910  	 * Optional query parameters:
911  	 * - transport={http|https}
912  	 * - vcenter={<vcenter>|*}             only useful for an esx:// connection
913  	 * - no_verify={0|1}
914  	 * - auto_answer={0|1}
915  	 * - proxy=[{http|socks|socks4|socks4a|socks5}://]<hostname>[:<port>]
916  	 *
917  	 * If no transport parameter is specified https is used.
918  	 *
919  	 * The vcenter parameter is only necessary for migration, because the vCenter
920  	 * server is in charge to initiate a migration between two ESX hosts. The
921  	 * vcenter parameter can be set to an explicitly hostname or to *. If set to *,
922  	 * the driver will check if the ESX host is managed by a vCenter and connect to
923  	 * it. If the ESX host is not managed by a vCenter an error is reported.
924  	 *
925  	 * If the no_verify parameter is set to 1, this disables libcurl client checks
926  	 * of the server's certificate. The default value it 0.
927  	 *
928  	 * If the auto_answer parameter is set to 1, the driver will respond to all
929  	 * virtual machine questions with the default answer, otherwise virtual machine
930  	 * questions will be reported as errors. The default value it 0.
931  	 *
932  	 * The proxy parameter allows to specify a proxy for to be used by libcurl.
933  	 * The default for the optional <type> part is http and socks is synonymous for
934  	 * socks5. The optional <port> part allows to override the default port 1080.
935  	 */
936  	static virDrvOpenStatus
937  	esxOpen(virConnectPtr conn, virConnectAuthPtr auth,
938  	        unsigned int flags)
939  	{
940  	    virDrvOpenStatus result = VIR_DRV_OPEN_ERROR;
941  	    char *plus;
942  	    esxPrivate *priv = NULL;
943  	    char *potentialVCenterIpAddress = NULL;
944  	    char vCenterIpAddress[NI_MAXHOST] = "";
945  	
(1) Event cond_false: Condition "__unsuppflags", taking false branch
(2) Event if_end: End of if statement
946  	    virCheckFlags(VIR_CONNECT_RO, VIR_DRV_OPEN_ERROR);
947  	
948  	    /* Decline if the URI is NULL or the scheme is NULL */
(3) Event cond_false: Condition "conn->uri == NULL", taking false branch
(4) Event cond_false: Condition "conn->uri->scheme == NULL", taking false branch
949  	    if (conn->uri == NULL || conn->uri->scheme == NULL) {
950  	        return VIR_DRV_OPEN_DECLINED;
(5) Event if_end: End of if statement
951  	    }
952  	
953  	    /* Decline if the scheme is not one of {vpx|esx|gsx} */
954  	    plus = strchr(conn->uri->scheme, '+');
955  	
(6) Event cond_true: Condition "plus == NULL", taking true branch
956  	    if (plus == NULL) {
(7) Event cond_false: Condition "c_strcasecmp(conn->uri->scheme, "vpx") != 0", taking false branch
957  	        if (STRCASENEQ(conn->uri->scheme, "vpx") &&
958  	            STRCASENEQ(conn->uri->scheme, "esx") &&
959  	            STRCASENEQ(conn->uri->scheme, "gsx")) {
960  	            return VIR_DRV_OPEN_DECLINED;
(8) Event if_end: End of if statement
961  	        }
(9) Event if_fallthrough: Falling through to end of if statement
962  	    } else {
963  	        if (plus - conn->uri->scheme != 3 ||
964  	            (STRCASENEQLEN(conn->uri->scheme, "vpx", 3) &&
965  	             STRCASENEQLEN(conn->uri->scheme, "esx", 3) &&
966  	             STRCASENEQLEN(conn->uri->scheme, "gsx", 3))) {
967  	            return VIR_DRV_OPEN_DECLINED;
968  	        }
969  	
970  	        virReportError(VIR_ERR_INVALID_ARG,
971  	                       _("Transport '%s' in URI scheme is not supported, try again "
972  	                         "without the transport part"), plus + 1);
973  	        return VIR_DRV_OPEN_ERROR;
(10) Event if_end: End of if statement
974  	    }
975  	
(11) Event cond_false: Condition "c_strcasecmp(conn->uri->scheme, "vpx") != 0", taking false branch
976  	    if (STRCASENEQ(conn->uri->scheme, "vpx") &&
977  	        conn->uri->path != NULL && STRNEQ(conn->uri->path, "/")) {
978  	        VIR_WARN("Ignoring unexpected path '%s' for non-vpx scheme '%s'",
979  	                 conn->uri->path, conn->uri->scheme);
(12) Event if_end: End of if statement
980  	    }
981  	
982  	    /* Require server part */
(13) Event cond_false: Condition "conn->uri->server == NULL", taking false branch
983  	    if (conn->uri->server == NULL) {
984  	        virReportError(VIR_ERR_INVALID_ARG, "%s",
985  	                       _("URI is missing the server part"));
986  	        return VIR_DRV_OPEN_ERROR;
(14) Event if_end: End of if statement
987  	    }
988  	
989  	    /* Require auth */
(15) Event cond_false: Condition "auth == NULL", taking false branch
(16) Event cond_false: Condition "auth->cb == NULL", taking false branch
990  	    if (auth == NULL || auth->cb == NULL) {
991  	        virReportError(VIR_ERR_INVALID_ARG, "%s",
992  	                       _("Missing or invalid auth pointer"));
993  	        return VIR_DRV_OPEN_ERROR;
(17) Event if_end: End of if statement
994  	    }
995  	
996  	    /* Allocate per-connection private data */
(18) Event alloc_arg: "virAlloc(void *, size_t)" allocates memory that is stored into "priv". [details]
(19) Event cond_false: Condition "virAlloc(&priv, 56UL /* sizeof (*priv) */) < 0", taking false branch
Also see events: [leaked_storage]
997  	    if (VIR_ALLOC(priv) < 0) {
998  	        virReportOOMError();
999  	        goto cleanup;
(20) Event if_end: End of if statement
1000 	    }
1001 	
(21) Event cond_true: Condition "esxUtil_ParseUri(&priv->parsedUri, conn->uri) < 0", taking true branch
1002 	    if (esxUtil_ParseUri(&priv->parsedUri, conn->uri) < 0) {
(22) Event goto: Jumping to label "cleanup"
1003 	        goto cleanup;
1004 	    }
1005 	
1006 	    priv->maxVcpus = -1;
1007 	    priv->supportsVMotion = esxVI_Boolean_Undefined;
1008 	    priv->supportsLongMode = esxVI_Boolean_Undefined;
1009 	    priv->usedCpuTimeCounterId = -1;
1010 	
1011 	    /*
1012 	     * Set the port dependent on the transport protocol if no port is
1013 	     * specified. This allows us to rely on the port parameter being
1014 	     * correctly set when building URIs later on, without the need to
1015 	     * distinguish between the situations port == 0 and port != 0
1016 	     */
1017 	    if (conn->uri->port == 0) {
1018 	        if (STRCASEEQ(conn->uri->scheme, "vpx") ||
1019 	            STRCASEEQ(conn->uri->scheme, "esx")) {
1020 	            if (STRCASEEQ(priv->parsedUri->transport, "https")) {
1021 	                conn->uri->port = 443;
1022 	            } else {
1023 	                conn->uri->port = 80;
1024 	            }
1025 	        } else { /* GSX */
1026 	            if (STRCASEEQ(priv->parsedUri->transport, "https")) {
1027 	                conn->uri->port = 8333;
1028 	            } else {
1029 	                conn->uri->port = 8222;
1030 	            }
1031 	        }
1032 	    }
1033 	
1034 	    if (STRCASEEQ(conn->uri->scheme, "esx") ||
1035 	        STRCASEEQ(conn->uri->scheme, "gsx")) {
1036 	        /* Connect to host */
1037 	        if (esxConnectToHost(priv, conn, auth,
1038 	                             &potentialVCenterIpAddress) < 0) {
1039 	            goto cleanup;
1040 	        }
1041 	
1042 	        /* Connect to vCenter */
1043 	        if (priv->parsedUri->vCenter != NULL) {
1044 	            if (STREQ(priv->parsedUri->vCenter, "*")) {
1045 	                if (potentialVCenterIpAddress == NULL) {
1046 	                    virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
1047 	                                   _("This host is not managed by a vCenter"));
1048 	                    goto cleanup;
1049 	                }
1050 	
1051 	                if (virStrcpyStatic(vCenterIpAddress,
1052 	                                    potentialVCenterIpAddress) == NULL) {
1053 	                    virReportError(VIR_ERR_INTERNAL_ERROR,
1054 	                                   _("vCenter IP address %s too big for destination"),
1055 	                                   potentialVCenterIpAddress);
1056 	                    goto cleanup;
1057 	                }
1058 	            } else {
1059 	                if (esxUtil_ResolveHostname(priv->parsedUri->vCenter,
1060 	                                            vCenterIpAddress, NI_MAXHOST) < 0) {
1061 	                    goto cleanup;
1062 	                }
1063 	
1064 	                if (potentialVCenterIpAddress != NULL &&
1065 	                    STRNEQ(vCenterIpAddress, potentialVCenterIpAddress)) {
1066 	                    virReportError(VIR_ERR_INTERNAL_ERROR,
1067 	                                   _("This host is managed by a vCenter with IP "
1068 	                                     "address %s, but a mismachting vCenter '%s' "
1069 	                                     "(%s) has been specified"),
1070 	                                   potentialVCenterIpAddress, priv->parsedUri->vCenter,
1071 	                                   vCenterIpAddress);
1072 	                    goto cleanup;
1073 	                }
1074 	            }
1075 	
1076 	            if (esxConnectToVCenter(priv, conn, auth,
1077 	                                    vCenterIpAddress,
1078 	                                    priv->host->ipAddress) < 0) {
1079 	                goto cleanup;
1080 	            }
1081 	        }
1082 	
1083 	        priv->primary = priv->host;
1084 	    } else { /* VPX */
1085 	        /* Connect to vCenter */
1086 	        if (esxConnectToVCenter(priv, conn, auth,
1087 	                                conn->uri->server,
1088 	                                NULL) < 0) {
1089 	            goto cleanup;
1090 	        }
1091 	
1092 	        priv->primary = priv->vCenter;
1093 	    }
1094 	
1095 	    /* Setup capabilities */
1096 	    priv->caps = esxCapsInit(priv);
1097 	
1098 	    if (priv->caps == NULL) {
1099 	        goto cleanup;
1100 	    }
1101 	
1102 	    conn->privateData = priv;
1103 	    priv = NULL;
1104 	    result = VIR_DRV_OPEN_SUCCESS;
1105 	
(23) Event label: Reached label "cleanup"
1106 	  cleanup:
1107 	    esxFreePrivate(&priv);
(24) Event cond_true: Condition "1", taking true branch
1108 	    VIR_FREE(potentialVCenterIpAddress);
1109 	
(25) Event leaked_storage: Variable "priv" going out of scope leaks the storage it points to.
Also see events: [alloc_arg]
1110 	    return result;
1111 	}
1112 	
1113 	
1114 	



















































[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]