From 46a724937bfbfe4fa4e64269f057c71893f25b30 Mon Sep 17 00:00:00 2001 From: Cathy Avery 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 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 RH-Acked-by: Miroslav Rezanina commit 2bbd56da4314856dfc1a8fed2db5b55cd9ef8860 Author: Oliver Kurth 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 Signed-off-by: Miroslav Rezanina --- 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 +#include #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