From 46a724937bfbfe4fa4e64269f057c71893f25b30 Mon Sep 17 00:00:00 2001

From: Cathy Avery <cavery@redhat.com>

Date: Thu, 25 Jul 2019 12:32:32 +0200

Subject: [PATCH 09/16] Fixes for few leaks and improved error handling.

RH-Author: Cathy Avery <cavery@redhat.com>

Message-id: <20190725123239.18274-10-cavery@redhat.com>

Patchwork-id: 89723

O-Subject: [RHEL8.1 open-vm-tools PATCH 09/16] Fixes for few leaks and improved error handling.

Bugzilla: 1602648

RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>

RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>

commit 2bbd56da4314856dfc1a8fed2db5b55cd9ef8860

Author: Oliver Kurth <okurth@vmware.com>

Date:   Wed May 8 15:27:18 2019 -0700

    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.

Signed-off-by: Cathy Avery <cavery@redhat.com>

Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>

---

 open-vm-tools/lib/stubs/stub-panic.c   |  3 ++-

 open-vm-tools/lib/stubs/stub-warning.c |  5 ++--

 open-vm-tools/xferlogs/xferlogs.c      | 47 ++++++++++++++++++++++++++++------

 3 files changed, 44 insertions(+), 11 deletions(-)

diff --git a/lib/stubs/stub-panic.c b/lib/stubs/stub-panic.c

index 615a810..4b88f59 100644

--- a/lib/stubs/stub-panic.c

+++ b/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/lib/stubs/stub-warning.c b/lib/stubs/stub-warning.c

index c32fa69..3a49617 100644

--- a/lib/stubs/stub-warning.c

+++ b/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/xferlogs/xferlogs.c b/xferlogs/xferlogs.c

index 9aa9b3a..d4a600f 100644

--- a/xferlogs/xferlogs.c

+++ b/xferlogs/xferlogs.c

@@ -184,8 +184,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);

             /*

@@ -234,23 +245,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)) {

@@ -260,10 +280,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);

 }

-- 

1.8.3.1