From 95800c144d2ab2af95cdc8f08df0518c496a579a Mon Sep 17 00:00:00 2001 From: Cathy Avery Date: Thu, 12 Nov 2020 09:01:08 -0500 Subject: [PATCH] Fix memory leaks. RH-Author: Cathy Avery (cavery) RH-MergeRequest: 2: Fix memory leaks. RH-Commit: [1/1] 79ac85f5e8c31cc48b7b0834682c6320afcc2288 (cavery/open-vm-tools) RH-Bugzilla: 1896804 Bugzilla: https://bugzilla.redhat.com/show_bug.cgi?id=1896804 Brew: https://brewweb.engineering.redhat.com/brew/taskinfo?taskID=33050983 Tested: By QE Upstream Status: devel branch A Coverity scan of open-vm-tools reported a number of memory leaks on error code paths. Fix seven reported leaks, and modify code to address two false positives in order to make the code clearer and/or keep Coverity from reporting the issues. Also fix additional leaks found in the routine Proto_TextContents during code review. (cherry picked from commit e18e67f727d0354b08a55b685178fd05f542c6da) Signed-off-by: Cathy Avery --- open-vm-tools/libvmtools/vmtoolsLog.c | 6 ++--- .../plugins/guestInfo/guestInfoServer.c | 2 +- open-vm-tools/services/vmtoolsd/pluginMgr.c | 1 + open-vm-tools/vgauth/lib/proto.c | 23 +++++++++++++++---- open-vm-tools/vgauth/serviceImpl/alias.c | 4 ++++ 5 files changed, 28 insertions(+), 8 deletions(-) diff --git a/open-vm-tools/libvmtools/vmtoolsLog.c b/open-vm-tools/libvmtools/vmtoolsLog.c index a991b49f..bea5abd4 100644 --- a/open-vm-tools/libvmtools/vmtoolsLog.c +++ b/open-vm-tools/libvmtools/vmtoolsLog.c @@ -2395,7 +2395,6 @@ VMTools_ChangeLogFilePath(const gchar *delimiter, // IN { gchar key[128]; gchar *path = NULL; - gchar *userLogTemp = NULL; gchar **tokens; gboolean retVal = FALSE; @@ -2412,8 +2411,9 @@ VMTools_ChangeLogFilePath(const gchar *delimiter, // IN tokens = g_strsplit(path, delimiter, 2); if (tokens != NULL && *tokens != NULL){ - userLogTemp = g_strjoin(appendString, *tokens, " ", NULL); - userLogTemp = g_strchomp (userLogTemp); + char *userLogTemp = g_strjoin(appendString, *tokens, " ", NULL); + + g_strchomp(userLogTemp); if (*(tokens+1) != NULL){ gchar *userLog; userLog = g_strjoin(delimiter, userLogTemp, *(tokens+1), NULL); diff --git a/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c b/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c index c1ab6962..ab6725fe 100644 --- a/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c +++ b/open-vm-tools/services/plugins/guestInfo/guestInfoServer.c @@ -1298,12 +1298,12 @@ GuestInfoSendDiskInfoV1(ToolsAppCtx *ctx, // IN b64name, pdi->partitionList[i].freeBytes, pdi->partitionList[i].totalBytes); + g_free(b64name); 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, diff --git a/open-vm-tools/services/vmtoolsd/pluginMgr.c b/open-vm-tools/services/vmtoolsd/pluginMgr.c index 53b91f7a..d5f2c0ef 100644 --- a/open-vm-tools/services/vmtoolsd/pluginMgr.c +++ b/open-vm-tools/services/vmtoolsd/pluginMgr.c @@ -512,6 +512,7 @@ ToolsCoreLoadDirectory(ToolsAppCtx *ctx, dir = g_dir_open(pluginPath, 0, &err); if (dir == NULL) { g_warning("Error opening dir: %s\n", err->message); + g_clear_error(&err); goto exit; } diff --git a/open-vm-tools/vgauth/lib/proto.c b/open-vm-tools/vgauth/lib/proto.c index 12386918..01df9df7 100644 --- a/open-vm-tools/vgauth/lib/proto.c +++ b/open-vm-tools/vgauth/lib/proto.c @@ -830,8 +830,10 @@ Proto_TextContents(GMarkupParseContext *parseContext, g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT, "Found pipeName in reply type %d", reply->expectedReplyType); + g_free(val); + } else { + reply->replyData.sessionReq.pipeName = val; } - reply->replyData.sessionReq.pipeName = val; break; case PARSE_STATE_TICKET: @@ -839,8 +841,10 @@ Proto_TextContents(GMarkupParseContext *parseContext, g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT, "Found ticket in reply type %d", reply->expectedReplyType); + g_free(val); + } else { + reply->replyData.createTicket.ticket = val; } - reply->replyData.createTicket.ticket = val; break; case PARSE_STATE_TOKEN: @@ -853,6 +857,7 @@ Proto_TextContents(GMarkupParseContext *parseContext, g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT, "Found token in reply type %d", reply->expectedReplyType); + g_free(val); } break; @@ -863,6 +868,7 @@ Proto_TextContents(GMarkupParseContext *parseContext, g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT, "Found token in reply type %d", reply->expectedReplyType); + g_free(val); } break; @@ -878,6 +884,7 @@ Proto_TextContents(GMarkupParseContext *parseContext, g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT, "Found username in reply type %d", reply->expectedReplyType); + g_free(val); } break; @@ -890,6 +897,7 @@ Proto_TextContents(GMarkupParseContext *parseContext, g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT, "Found pemCert in reply type %d", reply->expectedReplyType); + g_free(val); } break; case PARSE_STATE_CERTCOMMENT: @@ -899,6 +907,7 @@ Proto_TextContents(GMarkupParseContext *parseContext, g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT, "Found cert comment in reply type %d", reply->expectedReplyType); + g_free(val); } break; @@ -923,6 +932,7 @@ Proto_TextContents(GMarkupParseContext *parseContext, g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT, "Found SAMLSubject in reply type %d", reply->expectedReplyType); + g_free(val); } break; case PARSE_STATE_USERHANDLETYPE: @@ -968,6 +978,7 @@ Proto_TextContents(GMarkupParseContext *parseContext, g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT, "Found NamedSubject in reply type %d", reply->expectedReplyType); + g_free(val); } break; case PARSE_STATE_ANYSUBJECT: @@ -990,6 +1001,7 @@ Proto_TextContents(GMarkupParseContext *parseContext, "Found AnySubject in reply type %d", reply->expectedReplyType); } + g_free(val); break; case PARSE_STATE_COMMENT: if (PROTO_REPLY_QUERYALIASES == reply->expectedReplyType) { @@ -1005,11 +1017,13 @@ Proto_TextContents(GMarkupParseContext *parseContext, g_set_error(error, G_MARKUP_ERROR_PARSE, VGAUTH_E_INVALID_ARGUMENT, "Found comment in reply type %d", reply->expectedReplyType); + g_free(val); } break; default: g_warning("Unexpected value '%s' in unhandled parseState %d in %s\n", val, reply->parseState, __FUNCTION__); + g_free(val); ASSERT(0); } } @@ -1200,7 +1214,6 @@ VGAuth_ReadAndParseResponse(VGAuthContext *ctx, VGAuthError err = VGAUTH_E_OK; GMarkupParseContext *parseContext; gsize len; - gchar *rawReply = NULL; ProtoReply *reply; gboolean bRet; GError *gErr = NULL; @@ -1217,6 +1230,8 @@ VGAuth_ReadAndParseResponse(VGAuthContext *ctx, * transport. */ while (!reply->complete) { + gchar *rawReply = NULL; + err = VGAuth_CommReadData(ctx, &len, &rawReply); if (0 == len) { // EOF -- not expected err = VGAUTH_E_COMM; @@ -1237,6 +1252,7 @@ VGAuth_ReadAndParseResponse(VGAuthContext *ctx, rawReply, len, &gErr); + g_free(rawReply); if (!bRet) { /* * XXX Could drain the wire here, but since this should @@ -1252,7 +1268,6 @@ VGAuth_ReadAndParseResponse(VGAuthContext *ctx, * XXX need some way to break out if packet never completed * yet socket left valid. timer? */ - g_free(rawReply); } #if VGAUTH_PROTO_TRACE diff --git a/open-vm-tools/vgauth/serviceImpl/alias.c b/open-vm-tools/vgauth/serviceImpl/alias.c index f6cde02c..0a43811e 100644 --- a/open-vm-tools/vgauth/serviceImpl/alias.c +++ b/open-vm-tools/vgauth/serviceImpl/alias.c @@ -3158,6 +3158,9 @@ ServiceIDVerifyStoreContents(void) * a blacklist of bad files and keep going. but that's * a lot of risky work that's very hard to test, so punt for now. */ + g_free(badFileName); + g_free(fullFileName); + g_dir_close(dir); return VGAUTH_E_FAIL; } else { Audit_Event(TRUE, @@ -3408,6 +3411,7 @@ ServiceAliasInitAliasStore(void) "Failed to rename suspect Alias store directory '%s' to '%s'"), aliasStoreRootDir, badRootDirName); // XXX making this fatal for now. can we do anything better? + g_free(badRootDirName); return VGAUTH_E_FAIL; } g_free(badRootDirName); -- 2.18.4