From d477b6e21915d5099018f4fc4b60f257bb593d72 Mon Sep 17 00:00:00 2001
From: Cathy Avery <cavery@redhat.com>
Date: Thu, 25 Jul 2019 12:32:33 +0200
Subject: [PATCH 10/16] Fix Coverity-reported double memory free errors.
RH-Author: Cathy Avery <cavery@redhat.com>
Message-id: <20190725123239.18274-11-cavery@redhat.com>
Patchwork-id: 89725
O-Subject: [RHEL8.1 open-vm-tools PATCH 10/16] Fix Coverity-reported double memory free errors.
Bugzilla: 1602648
RH-Acked-by: Vitaly Kuznetsov <vkuznets@redhat.com>
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
commit 801df14f0e2b32aea17771bbd33d65140ff2361c
Author: Oliver Kurth <okurth@vmware.com>
Date: Wed May 8 15:27:19 2019 -0700
Fix Coverity-reported double memory free errors.
Similar double memory free errors were reported in each of two
functions, VixToolsListAuthAliases and VixToolsListMappedAliases.
The fixes for each function are similar: be consistent in using
tmpBuf2 (renamed tmpBuf) as the pointer to the overall buffer being
computed and tmpBuf (renamed nextBuf) as the "next" version of the
buffer. Specifically, in the computation of recordBuf following exit
from the for loop, use the variable formerly known as tmpBuf2 rather
than the one formerly known as tmpBuf.
The variables were renamed in an attempt to distinguish more clearly
between them and how they are used. Also, with these changes in
place, it's evident that there's no need to free nextBuf in the abort
case and as a result its scope can be limited.
Signed-off-by: Cathy Avery <cavery@redhat.com>
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
---
open-vm-tools/services/plugins/vix/vixTools.c | 88 ++++++++++++++-------------
1 file changed, 45 insertions(+), 43 deletions(-)
diff --git a/services/plugins/vix/vixTools.c b/services/plugins/vix/vixTools.c
index 2355beb..ef26742 100644
--- a/services/plugins/vix/vixTools.c
+++ b/services/plugins/vix/vixTools.c
@@ -9616,7 +9616,6 @@ VixToolsListAuthAliases(VixCommandRequestHeader *requestMsg, // IN
char *destPtr;
char *endDestPtr;
char *tmpBuf = NULL;
- char *tmpBuf2 = NULL;
char *recordBuf;
size_t recordSize;
char *escapedStr = NULL;
@@ -9681,15 +9680,17 @@ VixToolsListAuthAliases(VixCommandRequestHeader *requestMsg, // IN
err = VIX_E_OUT_OF_MEMORY;
goto abort;
}
- tmpBuf2 = Str_Asprintf(NULL, "<record><pemCert>%s</pemCert>",
- escapedStr);
+ tmpBuf = Str_Asprintf(NULL, "<record><pemCert>%s</pemCert>",
+ escapedStr);
free(escapedStr);
escapedStr = NULL;
- if (tmpBuf2 == NULL) {
+ if (tmpBuf == NULL) {
err = VIX_E_OUT_OF_MEMORY;
goto abort;
}
for (j = 0; j < uaList[i].numInfos; j++) {
+ char *nextBuf;
+
if (uaList[i].infos[j].comment) {
escapedStr = VixToolsEscapeXMLString(uaList[i].infos[j].comment);
if (escapedStr == NULL) {
@@ -9704,25 +9705,26 @@ VixToolsListAuthAliases(VixCommandRequestHeader *requestMsg, // IN
goto abort;
}
}
- tmpBuf = Str_Asprintf(NULL,
- "%s"
- "<alias>"
- "<type>%d</type>"
- "<name>%s</name>"
- "<comment>%s</comment>"
- "</alias>",
- tmpBuf2,
- (uaList[i].infos[j].subject.type == VGAUTH_SUBJECT_NAMED)
- ? VIX_GUEST_AUTH_SUBJECT_TYPE_NAMED :
- VIX_GUEST_AUTH_SUBJECT_TYPE_ANY,
- escapedStr2 ? escapedStr2 : "",
- escapedStr ? escapedStr : "");
- if (tmpBuf == NULL) {
+ nextBuf = Str_Asprintf(NULL,
+ "%s"
+ "<alias>"
+ "<type>%d</type>"
+ "<name>%s</name>"
+ "<comment>%s</comment>"
+ "</alias>",
+ tmpBuf,
+ (uaList[i].infos[j].subject.type ==
+ VGAUTH_SUBJECT_NAMED) ?
+ VIX_GUEST_AUTH_SUBJECT_TYPE_NAMED :
+ VIX_GUEST_AUTH_SUBJECT_TYPE_ANY,
+ escapedStr2 ? escapedStr2 : "",
+ escapedStr ? escapedStr : "");
+ if (nextBuf == NULL) {
err = VIX_E_OUT_OF_MEMORY;
goto abort;
}
- free(tmpBuf2);
- tmpBuf2 = tmpBuf;
+ free(tmpBuf);
+ tmpBuf = nextBuf;
free(escapedStr);
escapedStr = NULL;
free(escapedStr2);
@@ -9732,7 +9734,7 @@ VixToolsListAuthAliases(VixCommandRequestHeader *requestMsg, // IN
"%s</record>",
tmpBuf);
free(tmpBuf);
- tmpBuf = tmpBuf2 = NULL;
+ tmpBuf = NULL;
if (recordBuf == NULL) {
err = VIX_E_OUT_OF_MEMORY;
goto abort;
@@ -9752,7 +9754,6 @@ VixToolsListAuthAliases(VixCommandRequestHeader *requestMsg, // IN
abort:
free(tmpBuf);
- free(tmpBuf2);
free(escapedStr);
free(escapedStr2);
VGAuth_FreeUserAliasList(num, uaList);
@@ -9812,7 +9813,6 @@ VixToolsListMappedAliases(VixCommandRequestHeader *requestMsg, // IN
char *destPtr;
char *endDestPtr;
char *tmpBuf = NULL;
- char *tmpBuf2 = NULL;
char *recordBuf;
char *escapedStr = NULL;
char *escapedStr2 = NULL;
@@ -9876,19 +9876,21 @@ VixToolsListMappedAliases(VixCommandRequestHeader *requestMsg, // IN
err = VIX_E_OUT_OF_MEMORY;
goto abort;
}
- tmpBuf2 = Str_Asprintf(NULL, "<record><pemCert>%s</pemCert>"
- "<userName>%s</userName>",
- escapedStr,
- escapedStr2);
+ tmpBuf = Str_Asprintf(NULL, "<record><pemCert>%s</pemCert>"
+ "<userName>%s</userName>",
+ escapedStr,
+ escapedStr2);
g_free(escapedStr2);
g_free(escapedStr);
escapedStr = NULL;
escapedStr2 = NULL;
- if (tmpBuf2 == NULL) {
+ if (tmpBuf == NULL) {
err = VIX_E_OUT_OF_MEMORY;
goto abort;
}
for (j = 0; j < maList[i].numSubjects; j++) {
+ char *nextBuf;
+
if (maList[i].subjects[j].type == VGAUTH_SUBJECT_NAMED) {
escapedStr = VixToolsEscapeXMLString(maList[i].subjects[j].val.name);
if (escapedStr == NULL) {
@@ -9896,23 +9898,24 @@ VixToolsListMappedAliases(VixCommandRequestHeader *requestMsg, // IN
goto abort;
}
}
- tmpBuf = Str_Asprintf(NULL,
- "%s"
- "<alias>"
- "<type>%d</type>"
- "<name>%s</name>"
- "</alias>",
- tmpBuf2,
- (maList[i].subjects[j].type == VGAUTH_SUBJECT_NAMED)
- ? VIX_GUEST_AUTH_SUBJECT_TYPE_NAMED :
- VIX_GUEST_AUTH_SUBJECT_TYPE_ANY,
+ nextBuf = Str_Asprintf(NULL,
+ "%s"
+ "<alias>"
+ "<type>%d</type>"
+ "<name>%s</name>"
+ "</alias>",
+ tmpBuf,
+ (maList[i].subjects[j].type ==
+ VGAUTH_SUBJECT_NAMED) ?
+ VIX_GUEST_AUTH_SUBJECT_TYPE_NAMED :
+ VIX_GUEST_AUTH_SUBJECT_TYPE_ANY,
escapedStr ? escapedStr : "");
- if (tmpBuf == NULL) {
+ if (nextBuf == NULL) {
err = VIX_E_OUT_OF_MEMORY;
goto abort;
}
- free(tmpBuf2);
- tmpBuf2 = tmpBuf;
+ free(tmpBuf);
+ tmpBuf = nextBuf;
free(escapedStr);
escapedStr = NULL;
}
@@ -9920,7 +9923,7 @@ VixToolsListMappedAliases(VixCommandRequestHeader *requestMsg, // IN
"%s</record>",
tmpBuf);
free(tmpBuf);
- tmpBuf = tmpBuf2 = NULL;
+ tmpBuf = NULL;
if (recordBuf == NULL) {
err = VIX_E_OUT_OF_MEMORY;
goto abort;
@@ -9940,7 +9943,6 @@ VixToolsListMappedAliases(VixCommandRequestHeader *requestMsg, // IN
abort:
free(tmpBuf);
- free(tmpBuf2);
free(escapedStr);
free(escapedStr2);
VGAuth_FreeMappedAliasList(num, maList);
--
1.8.3.1