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 615a810859ba8873bcaf9126fda7bbd77d8dc1bc..4b88f591ed01d134c03c7dc7ad03e70ccf47febf 100644 (file)
--- 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 c32fa6945d1d5e9f610bbc18258204427eb68189..3a496172cbdcf7693ef3a931100297143b7f269e 100644 (file)
--- 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 0b07ab44a580a2a50c6b83c63925688f3db2256c..603c6782f49980bf1aab5560dce8dddeddf0d52c 100644 (file)
--- 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);
 }