From: Oliver Kurth <okurth@vmware.com>
Date: Wed, 8 May 2019 22:27:18 +0000 (-0700)
Subject: Fixes for few leaks and improved error handling.
X-Git-Tag: stable-11.0.0~91
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=2bbd56da4314856dfc1a8fed2db5b55cd9ef8860;p=thirdparty%2Fopen-vm-tools.git

Fixes for few leaks and improved error handling.

Fix a memory leak detected by coverity scan. It is not critical,
but it is real in an error case when there is no end mark. While
fixing it, also enhanced code to handle different error cases
properly because we would want valid content to be decoded even
when there are invalid marks in the log file. Invalid log marks
are possible when vmware.log gets rotated in the middle of guest
logging.

While verifying the fix using valgrind, found a couple of more
leaks in panic and warning stubs. Addressed those as well.
---

diff --git a/open-vm-tools/lib/stubs/stub-panic.c b/open-vm-tools/lib/stubs/stub-panic.c
index 615a81085..4b88f591e 100644
--- a/open-vm-tools/lib/stubs/stub-panic.c
+++ b/open-vm-tools/lib/stubs/stub-panic.c
@@ -1,5 +1,5 @@
 /*********************************************************
- * Copyright (C) 2008 VMware, Inc. All rights reserved.
+ * Copyright (C) 2008,2019 VMware, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU Lesser General Public License as published
@@ -43,6 +43,7 @@ Panic(const char *fmt, ...)
 
    if (str != NULL) {
       fputs(str, stderr);
+      free(str);
    }
 
    assert(FALSE);
diff --git a/open-vm-tools/lib/stubs/stub-warning.c b/open-vm-tools/lib/stubs/stub-warning.c
index c32fa6945..3a496172c 100644
--- a/open-vm-tools/lib/stubs/stub-warning.c
+++ b/open-vm-tools/lib/stubs/stub-warning.c
@@ -1,5 +1,5 @@
 /*********************************************************
- * Copyright (C) 2008-2016 VMware, Inc. All rights reserved.
+ * Copyright (C) 2008-2016,2019 VMware, Inc. All rights reserved.
  *
  * This program is free software; you can redistribute it and/or modify it
  * under the terms of the GNU Lesser General Public License as published
@@ -24,6 +24,7 @@
  */
 
 #include <stdio.h>
+#include <stdlib.h>
 #include "str.h"
 
 
@@ -39,6 +40,6 @@ Warning(const char *fmt, ...)
 
    if (str != NULL) {
       fputs(str, stderr);
+      free(str);
    }
 }
-
diff --git a/open-vm-tools/xferlogs/xferlogs.c b/open-vm-tools/xferlogs/xferlogs.c
index 0b07ab44a..603c6782f 100644
--- a/open-vm-tools/xferlogs/xferlogs.c
+++ b/open-vm-tools/xferlogs/xferlogs.c
@@ -187,8 +187,19 @@ extractFile(char *filename) //IN: vmx log filename e.g. vmware.log
             char tstamp[32];
             time_t now;
 
-            ASSERT(outfp == NULL);
-            ASSERT(state == NOT_IN_GUEST_LOGGING);
+            /*
+             * There could be multiple LOG_START_MARK in the log,
+             * close existing one before opening a new file.
+             */
+            if (outfp) {
+               ASSERT(state == IN_GUEST_LOGGING);
+               Warning("Found a new start mark before end mark for "
+                       "previous one\n");
+               fclose(outfp);
+               outfp = NULL;
+            } else {
+               ASSERT(state == NOT_IN_GUEST_LOGGING);
+            }
             DEBUG_ONLY(state = IN_GUEST_LOGGING);
 
             /*
@@ -237,23 +248,32 @@ extractFile(char *filename) //IN: vmx log filename e.g. vmware.log
                ver = ver + sizeof "ver - " - 1;
                version = strtol(ver, NULL, 0);
                if (version != LOG_VERSION) {
-                  Warning("input version %d doesnt match the\
+                  Warning("Input version %d doesn't match the\
                           version of this binary %d", version, LOG_VERSION);
                } else {
-                  printf("reading file %s to %s \n", logInpFilename, fname);
+                  printf("Reading file %s to %s \n", logInpFilename, fname);
                   if (!(outfp = fopen(fname, "wb"))) {
                      Warning("Error opening file %s\n", fname);
                   }
                }
             }
          } else if (strstr(buf, LOG_END_MARK)) { // close the output file.
-            ASSERT(state == IN_GUEST_LOGGING);
+            /*
+             * Need to check outfp, because we might get LOG_END_MARK
+             * before LOG_START_MARK due to log rotation.
+             */
+            if (outfp) {
+               ASSERT(state == IN_GUEST_LOGGING);
+               fclose(outfp);
+               outfp = NULL;
+            } else {
+               ASSERT(state == NOT_IN_GUEST_LOGGING);
+               Warning("Reached file end mark without start mark\n");
+            }
             DEBUG_ONLY(state = NOT_IN_GUEST_LOGGING);
-            fclose(outfp);
-            outfp = NULL;
          } else { // write to the output file
-            ASSERT(state == IN_GUEST_LOGGING);
             if (outfp) {
+               ASSERT(state == IN_GUEST_LOGGING);
                ptrStr = strstr(buf, LOG_GUEST_MARK);
                ptrStr += sizeof LOG_GUEST_MARK - 1;
                if (Base64_Decode(ptrStr, base64Out, BUF_OUT_SIZE, &lenOut)) {
@@ -263,10 +283,21 @@ extractFile(char *filename) //IN: vmx log filename e.g. vmware.log
                } else {
                   Warning("Error decoding output %s\n", ptrStr);
                }
+            } else {
+               ASSERT(state == NOT_IN_GUEST_LOGGING);
+               Warning("Missing file start mark\n");
             }
          }
       }
    }
+
+   /*
+    * We may need to close file in case LOG_END_MARK is missing.
+    */
+   if (outfp) {
+      ASSERT(state == IN_GUEST_LOGGING);
+      fclose(outfp);
+   }
    fclose(fp);
 }