]> git.ipfire.org Git - thirdparty/open-vm-tools.git/commitdiff
Fix memory leaks.
authorJohn Wolfe <jwolfe@vmware.com>
Tue, 27 Oct 2020 00:29:54 +0000 (17:29 -0700)
committerJohn Wolfe <jwolfe@vmware.com>
Tue, 27 Oct 2020 00:29:54 +0000 (17:29 -0700)
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.

open-vm-tools/libvmtools/vmtoolsLog.c
open-vm-tools/services/plugins/guestInfo/guestInfoServer.c
open-vm-tools/services/vmtoolsd/pluginMgr.c
open-vm-tools/vgauth/lib/proto.c
open-vm-tools/vgauth/serviceImpl/alias.c

index a991b49ff285de94355bec3286ecf50c161debc8..bea5abd404ccf31682eaeea31125c1ba09bb49da 100644 (file)
@@ -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);
index c1ab69629056017aafe8a7e7aa1b976a8b6dc4a3..ab6725fe2439eec7047a7a1c688a423f90414d74 100644 (file)
@@ -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,
index 53b91f7aafc210b8bdcaa3631f87081e8882cb0e..d5f2c0efcb19a617539194bcbb8d744cce4a8664 100644 (file)
@@ -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;
    }
 
index 1238691893dcbccd6cbceb9c7371ba9c3193ecf8..01df9df71d8e07105e2aed45618acf506bf61fb9 100644 (file)
@@ -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
index f6cde02cf978b8921fac1199241805577096a5a2..0a43811e7418a66d719c0310a5629153cda93e76 100644 (file)
@@ -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);