From: Oliver Kurth Date: Mon, 9 Sep 2019 18:23:49 +0000 (-0700) Subject: Fix Coverity scan issues in open-vm-tools. X-Git-Tag: stable-11.1.0~237 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=f8c20561d006a52c97999f7f946daae740830d65;p=thirdparty%2Fopen-vm-tools.git Fix Coverity scan issues in open-vm-tools. --- diff --git a/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c b/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c index b28325048..5c5ccc35f 100644 --- a/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c +++ b/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c @@ -1206,7 +1206,8 @@ GuestInfoSendNicInfo(ToolsAppCtx *ctx, // IN * @param[in] infoSize Size of disk info. * * @retval TRUE Update sent successfully. - * @retval FALSE Had trouble with serialization or transmission. + * @retval FALSE Had trouble with json string formatting, serialization or + * transmission. * ****************************************************************************** */ @@ -1222,7 +1223,7 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx, // IN char *infoReq; char *reply = NULL; size_t replyLen; - Bool status; + Bool status = FALSE; #ifdef _WIN32 Bool reportUUID = VMTools_ConfigGetBoolean(ctx->config, CONFGROUPNAME_GUESTINFO, @@ -1267,6 +1268,10 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx, // IN len = Str_Snprintf(tmpBuf, sizeof tmpBuf, headerFmt, GUEST_DISK_INFO_COMMAND, DISK_INFO_VERSION_1); + if (len <= 0) { + goto exit; + } + DynBuf_Append(&dynBuffer, tmpBuf, len); for (i = 0; i < pdi->numEntries; i++) { gchar *b64name; @@ -1292,12 +1297,20 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx, // IN b64name, pdi->partitionList[i].freeBytes, pdi->partitionList[i].totalBytes); + if (len <= 0) { + goto exit; + } + DynBuf_Append(&dynBuffer, tmpBuf, len); g_free(b64name); if (pdi->partitionList[i].fsType[0] != '\0') { len = Str_Snprintf(tmpBuf, sizeof tmpBuf, jsonPerDiskFsTypeFmt, pdi->partitionList[i].fsType); + if (len <= 0) { + goto exit; + } + DynBuf_Append(&dynBuffer, tmpBuf, len); } #ifdef _WIN32 @@ -1305,6 +1318,10 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx, // IN if (pdi->partitionList[i].uuid[0] != '\0') { len = Str_Snprintf(tmpBuf, sizeof tmpBuf, jsonPerDiskUUIDFmt, pdi->partitionList[i].uuid); + if (len <= 0) { + goto exit; + } + DynBuf_Append(&dynBuffer, tmpBuf, len); } } @@ -1316,11 +1333,19 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx, // IN sizeof jsonPerDiskDevArrHdrFmt - 1); len = Str_Snprintf(tmpBuf, sizeof tmpBuf, jsonPerDiskDeviceFmt, "", pdi->partitionList[i].diskDevNames[0]); + if (len <= 0) { + goto exit; + } + DynBuf_Append(&dynBuffer, tmpBuf, len); for (idx = 1; idx < pdi->partitionList[i].diskDevCnt; idx++) { len = Str_Snprintf(tmpBuf, sizeof tmpBuf, jsonPerDiskDeviceFmt, jsonPerDiskDeviceSep, pdi->partitionList[i].diskDevNames[idx]); + if (len <= 0) { + goto exit; + } + DynBuf_Append(&dynBuffer, tmpBuf, len); } DynBuf_Append(&dynBuffer, jsonPerDiskDevArrFmtFooter, @@ -1352,9 +1377,10 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx, // IN __FUNCTION__, status, reply ? reply : ""); } - DynBuf_Destroy(&dynBuffer); vm_free(reply); +exit: + DynBuf_Destroy(&dynBuffer); return status; } diff --git a/open-vm-tools/services/plugins/guestInfo/perfMonLinux.c b/open-vm-tools/services/plugins/guestInfo/perfMonLinux.c index f353a2626..688f6492c 100644 --- a/open-vm-tools/services/plugins/guestInfo/perfMonLinux.c +++ b/open-vm-tools/services/plugins/guestInfo/perfMonLinux.c @@ -352,6 +352,12 @@ GuestInfoStoreStatByID(GuestStatToolsID reportID, // IN: INT_AS_HASHKEY(reportID), (void **) &stat); + /* + * Caller must not pass in a reportID that does not exist in the table. + */ + ASSERT(stat != NULL); + + /* coverity[var_deref_model] */ GuestInfoStoreStat(stat, value); } @@ -513,13 +519,15 @@ GuestInfoProcSimpleValue(GuestStatToolsID reportID, // IN: HashTable_Lookup(collector->reportMap, INT_AS_HASHKEY(reportID), (void **) &stat); - ASSERT(stat); - if (stat == NULL) { - g_warning("%s: Error stat ID %d not found.\n", __FUNCTION__, reportID); - return success; - } + /* + * Caller must not pass in a reportID that does not exist in the table. + */ + ASSERT(stat != NULL); + ASSERT(stat->query != NULL); ASSERT(stat->query->sourceFile); + + /* coverity[var_deref_op] */ fp = Posix_Fopen(stat->query->sourceFile, "r"); if (fp == NULL) { g_warning("%s: Error opening %s.\n", @@ -589,14 +597,21 @@ GuestInfoDeriveSwapData(GuestInfoCollector *collector) // IN/OUT: INT_AS_HASHKEY(GuestStatID_SwapSpaceRemaining), (void **) &swapSpaceRemaining); + ASSERT(swapFilesMax != NULL && + swapFilesCurrent != NULL && + swapSpaceUsed != NULL && + swapSpaceRemaining != NULL); // Must be in the table + /* * Start by getting SwapTotal (from Id_SwapFilesCurrent). * Set Id_SwapFilesMax to that if it doesn't have its own opinion. */ - if ((swapFilesCurrent != NULL) && (swapFilesCurrent->err == 0)) { + /* coverity[var_deref_op] */ + if (swapFilesCurrent->err == 0) { swapTotal = swapFilesCurrent->value; - if ((swapFilesMax != NULL) && (swapFilesMax->err != 0)) { + /* coverity[var_deref_op] */ + if (swapFilesMax->err != 0) { swapFilesMax->value = swapTotal; swapFilesMax->count = 1; swapFilesMax->err = 0; @@ -607,12 +622,15 @@ GuestInfoDeriveSwapData(GuestInfoCollector *collector) // IN/OUT: * Set Id_SwapSpaceUsed to SwapTotal-SwapFree if it doesn't have its * own opinion. */ - if ((swapSpaceRemaining != NULL) && (swapSpaceRemaining->err == 0)) { + /* coverity[var_deref_op] */ + if (swapSpaceRemaining->err == 0) { swapFree = swapSpaceRemaining->value; ASSERT(swapTotal >= swapFree); swapUsed = (swapTotal >= swapFree) ? swapTotal - swapFree : 0; - if ((swapSpaceUsed != NULL) && (swapSpaceUsed->err != 0)) { + + /* coverity[var_deref_op] */ + if (swapSpaceUsed->err != 0) { swapSpaceUsed->value = swapUsed; swapSpaceUsed->count = 1; swapSpaceUsed->err = 0; @@ -649,13 +667,13 @@ GuestInfoDecreaseCpuRunQueueByOne(GuestInfoCollector *collector) // IN/OUT: INT_AS_HASHKEY(GuestStatID_Linux_CpuRunQueue), (void **) &stat); - ASSERT(stat != NULL); + ASSERT(stat != NULL); // Must be in the table ASSERT(stat->err == 0); ASSERT(stat->count == 1); - if (stat != NULL && stat->err == 0 && stat->count == 1) { - if (stat->value > 0) { - stat->value--; - } + + /* coverity[var_deref_op] */ + if (stat->value > 0) { + stat->value--; } } @@ -818,7 +836,7 @@ GuestInfoCollect(GuestInfoCollector *collector) // IN/OUT: /* Reset all values */ for (i = 0; i < collector->numStats; i++) { - GuestInfoStat *stat = &collector->stats[i]; + stat = &collector->stats[i]; stat->err = ENOENT; // There is no data here stat->count = 0; @@ -853,8 +871,9 @@ GuestInfoCollect(GuestInfoCollector *collector) // IN/OUT: INT_AS_HASHKEY(GuestStatID_MemPhysUsable), (void **) &stat); - ASSERT(stat != NULL); // Must be in table + ASSERT(stat != NULL); // Must be in the table + /* coverity[var_deref_op] */ if (stat->err == 0) { stat->value *= (pageSize / 1024); // Convert pages to KiB } else { @@ -864,7 +883,10 @@ GuestInfoCollect(GuestInfoCollector *collector) // IN/OUT: INT_AS_HASHKEY(GuestStatID_Linux_MemTotal), (void **) &memTotal); - if ((memTotal != NULL) && (memTotal->err == 0)) { + ASSERT(memTotal != NULL); // Must be in the table + + /* coverity[var_deref_op] */ + if (memTotal->err == 0) { stat->err = 0; stat->count = 1; stat->value = memTotal->value; @@ -909,7 +931,10 @@ GuestInfoLegacy(GuestInfoCollector *current, // IN: current collection INT_AS_HASHKEY(GuestStatID_MemPhysUsable), (void **) &stat); - if ((stat != NULL) && (stat->err == 0)) { + ASSERT(stat != NULL); // Must be in the table + + /* coverity[var_deref_op] */ + if (stat->err == 0) { legacy->memTotal = stat->value; legacy->flags |= MEMINFO_MEMTOTAL; } @@ -919,7 +944,10 @@ GuestInfoLegacy(GuestInfoCollector *current, // IN: current collection INT_AS_HASHKEY(GuestStatID_Linux_HugePagesTotal), (void **) &stat); - if ((stat != NULL) && (stat->err == 0)) { + ASSERT(stat != NULL); // Must be in the table + + /* coverity[var_deref_op] */ + if (stat->err == 0) { legacy->hugePagesTotal = stat->value; legacy->flags |= MEMINFO_HUGEPAGESTOTAL; } @@ -1072,10 +1100,14 @@ GuestInfoAppendRate(Bool emitNameSpace, // IN: INT_AS_HASHKEY(reportID), (void **) &previousStat); + ASSERT(currentStat != NULL && + previousStat != NULL); // Must be in the table + + /* coverity[var_deref_op] */ if (current->timeData && previous->timeData && - ((currentStat != NULL) && (currentStat->err == 0)) && - ((previousStat != NULL) && (previousStat->err == 0))) { + currentStat->err == 0 && + previousStat->err == 0) { double timeDelta = current->timeStamp - previous->timeStamp; double valueDelta; @@ -1113,7 +1145,7 @@ GuestInfoAppendRate(Bool emitNameSpace, // IN: errnoValue = 0; } - if (currentStat != NULL) { + { float valueFloat; void *valuePointer; size_t valueSize; @@ -1165,17 +1197,14 @@ GuestInfoDeriveMemNeeded(GuestInfoCollector *collector) // IN/OUT: GuestInfoStat *memAvail = NULL; GuestInfoStat *memPhysUsable = NULL; - HashTable_Lookup(collector->reportMap, - INT_AS_HASHKEY(GuestStatID_MemPhysUsable), - (void **) &memPhysUsable); - - ASSERT(memPhysUsable != NULL); - HashTable_Lookup(collector->reportMap, INT_AS_HASHKEY(GuestStatID_Linux_MemAvailable), (void **) &memAvail); - if ((memAvail != NULL) && (memAvail->err == 0)) { + ASSERT(memAvail != NULL); // Must be in the table + + /* coverity[var_deref_op] */ + if (memAvail->err == 0) { memAvailable = memAvail->value; } else { GuestInfoStat *memFree = NULL; @@ -1184,7 +1213,7 @@ GuestInfoDeriveMemNeeded(GuestInfoCollector *collector) // IN/OUT: GuestInfoStat *memActiveFile = NULL; GuestInfoStat *memSlabReclaim = NULL; GuestInfoStat *memInactiveFile = NULL; - GuestInfoStat *lowWaterMark = NULL; + GuestInfoStat *memLowWaterMark = NULL; HashTable_Lookup(collector->reportMap, INT_AS_HASHKEY(GuestStatID_MemFree), @@ -1206,21 +1235,28 @@ GuestInfoDeriveMemNeeded(GuestInfoCollector *collector) // IN/OUT: (void **) &memInactiveFile); HashTable_Lookup(collector->reportMap, INT_AS_HASHKEY(GuestStatID_Linux_LowWaterMark), - (void **) &lowWaterMark); - - if (((memFree != NULL) && (memFree->err == 0)) && - ((memCache != NULL) && (memCache->err == 0)) && - ((memBuffers != NULL) && (memBuffers->err == 0)) && - (memActiveFile != NULL) && - (memSlabReclaim != NULL) && - (memInactiveFile != NULL) && - ((lowWaterMark != NULL) && (lowWaterMark->err == 0))) { + (void **) &memLowWaterMark); + + ASSERT(memFree != NULL && + memCache != NULL && + memBuffers != NULL && + memActiveFile != NULL && + memSlabReclaim != NULL && + memInactiveFile != NULL && + memLowWaterMark != NULL); // Must be in the table + + /* coverity[var_deref_op] */ + if (memFree->err == 0 && + memCache->err == 0 && + memBuffers->err == 0 && + memLowWaterMark->err == 0) { uint64 pageCache; unsigned long kbPerPage = sysconf(_SC_PAGESIZE) / 1024UL; - uint64 lowWaterMarkValue = lowWaterMark->value * kbPerPage; + uint64 lowWaterMarkValue = memLowWaterMark->value * kbPerPage; memAvailable = memFree->value - lowWaterMarkValue; + /* coverity[var_deref_op] */ if ((memActiveFile->err == 0) && (memInactiveFile->err == 0)) { pageCache = memActiveFile->value + memInactiveFile->value; } else { @@ -1234,6 +1270,7 @@ GuestInfoDeriveMemNeeded(GuestInfoCollector *collector) // IN/OUT: pageCache -= MIN(pageCache / 2, lowWaterMarkValue); memAvailable += pageCache; + /* coverity[var_deref_op] */ if (memSlabReclaim->err == 0) { memAvailable += memSlabReclaim->value - MIN(memSlabReclaim->value / 2, lowWaterMarkValue); @@ -1249,6 +1286,13 @@ GuestInfoDeriveMemNeeded(GuestInfoCollector *collector) // IN/OUT: } } + HashTable_Lookup(collector->reportMap, + INT_AS_HASHKEY(GuestStatID_MemPhysUsable), + (void **) &memPhysUsable); + + ASSERT(memPhysUsable != NULL); // Must be in the table + + /* coverity[var_deref_op] */ if (memPhysUsable->err == 0) { /* * Reserve 5% of physical RAM for surges.