Note: This bug is displayed in read-only format because the product is no longer active in Red Hat Bugzilla.

RHEL Engineering is moving the tracking of its product development work on RHEL 6 through RHEL 9 to Red Hat Jira (issues.redhat.com). If you're a Red Hat customer, please continue to file support cases via the Red Hat customer portal. If you're not, please head to the "RHEL project" in Red Hat Jira and file new tickets here. Individual Bugzilla bugs in the statuses "NEW", "ASSIGNED", and "POST" are being migrated throughout September 2023. Bugs of Red Hat partners with an assigned Engineering Partner Manager (EPM) are migrated in late September as per pre-agreed dates. Bugs against components "kernel", "kernel-rt", and "kpatch" are only migrated if still in "NEW" or "ASSIGNED". If you cannot log in to RH Jira, please consult article #7032570. That failing, please send an e-mail to the RH Jira admins at rh-issues@redhat.com to troubleshoot your issue as a user management inquiry. The email creates a ServiceNow ticket with Red Hat. Individual Bugzilla bugs that are migrated will be moved to status "CLOSED", resolution "MIGRATED", and set with "MigratedToJIRA" in "Keywords". The link to the successor Jira issue will be found under "Links", have a little "two-footprint" icon next to it, and direct you to the "RHEL project" in Red Hat Jira (issue links are of type "https://issues.redhat.com/browse/RHEL-XXXX", where "X" is a digit). This same link will be available in a blue banner at the top of the page informing you that that bug has been migrated.

Bug 1886866

Summary:	[ESXi][open-vm-tools] Coverity detected important defects in open-vm-tools-11.1.5 rebase
Product:	Red Hat Enterprise Linux 8	Reporter:	Cathy Avery <cavery>
Component:	open-vm-tools	Assignee:	Cathy Avery <cavery>
Status:	CLOSED WONTFIX	QA Contact:	ldu <ldu>
Severity:	unspecified	Docs Contact:
Priority:	high
Version:	8.4	CC:	boyang, cavery, jen, jjarvis, jsaks, jsavanyo, ldu, leiwang, ravindrakumar, vmware-gos-qa, yacao
Target Milestone:	rc	Keywords:	Triaged
Target Release:	8.0	Flags:	pm-rhel: mirror+
Hardware:	Unspecified
OS:	Unspecified
Whiteboard:
Fixed In Version:		Doc Type:	If docs needed, set a value
Doc Text:		Story Points:	---
Clone Of:
Clones:	1896804 (view as bug list)		Environment:
Last Closed:	2020-12-06 18:41:35 UTC	Type:	Bug
Regression:	---	Mount Type:	---
Documentation:	---	CRM:
Verified Versions:		Category:	---
oVirt Team:	---	RHEL 7.3 requirements from Atomic Host:
Cloudforms Team:	---	Target Upstream Version:
Embargoed:
Bug Depends On:
Bug Blocks:	1896804

Description Cathy Avery 2020-10-09 14:51:29 UTC

In running Coverity version 2020.06 on the open-vm-tools-11.1.5 rebase for RHEL 8.4 the following defects classified as important were reported:

> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/lib/slashProc/net.c:441: alloc_arg: "g_io_channel_read_line" allocates memory that is stored into "myInputLine".
> open-vm-tools-11.1.5-16724464/lib/slashProc/net.c:489: leaked_storage: Variable "myInputLine" going out of scope leaks the storage it points to.
> #  487|      g_io_channel_unref(myChannel);
> #  488|   
> #  489|->    return myHashTable;
> #  490|   }
> #  491|   
>  
> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/libvmtools/vmtoolsLog.c:2450: alloc_fn: Storage is returned from allocation function "g_strjoin".
> open-vm-tools-11.1.5-16724464/libvmtools/vmtoolsLog.c:2450: var_assign: Assigning: "userLogTemp" = storage returned from "g_strjoin(appendString, *tokens, " ", NULL)".
> open-vm-tools-11.1.5-16724464/libvmtools/vmtoolsLog.c:2451: noescape: Resource "userLogTemp" is not freed or pointed-to in "g_strchomp".
> open-vm-tools-11.1.5-16724464/libvmtools/vmtoolsLog.c:2451: overwrite_var: Overwriting "userLogTemp" in "userLogTemp = g_strchomp(userLogTemp)" leaks the storage that "userLogTemp" points to.
> # 2449|      if (tokens != NULL && *tokens != NULL){
> # 2450|         userLogTemp = g_strjoin(appendString, *tokens, " ", NULL);
> # 2451|->       userLogTemp = g_strchomp (userLogTemp);
> # 2452|         if (*(tokens+1) != NULL){
> # 2453|            gchar *userLog;
>  
> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/services/plugins/guestInfo/guestInfoServer.c:1293: alloc_fn: Storage is returned from allocation function "g_base64_encode".
> open-vm-tools-11.1.5-16724464/services/plugins/guestInfo/guestInfoServer.c:1293: var_assign: Assigning: "b64name" = storage returned from "g_base64_encode(pdi->partitionList[i].name, strlen(pdi->partitionList[i].name))".
> open-vm-tools-11.1.5-16724464/services/plugins/guestInfo/guestInfoServer.c:1296: noescape: Resource "b64name" is not freed or pointed-to in "Str_Snprintf".
> open-vm-tools-11.1.5-16724464/services/plugins/guestInfo/guestInfoServer.c:1301: leaked_storage: Variable "b64name" going out of scope leaks the storage it points to.
> # 1299|                            pdi->partitionList[i].totalBytes);
> # 1300|         if (len <= 0) {
> # 1301|->          goto exit;
> # 1302|         }
> # 1303|   
>  
> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/services/vmtoolsd/pluginMgr.c:512: alloc_arg: "g_dir_open" allocates memory that is stored into "err".
> open-vm-tools-11.1.5-16724464/services/vmtoolsd/pluginMgr.c:601: leaked_storage: Variable "err" going out of scope leaks the storage it points to.
> #  599|   
> #  600|   exit:
> #  601|->    return ret;
> #  602|   }
> #  603|   
>  
> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/vgauth/common/i18n.c:613: alloc_arg: "DictLL_UnmarshalLine" allocates memory that is stored into "value".
> open-vm-tools-11.1.5-16724464/vgauth/common/i18n.c:648: leaked_storage: Variable "value" going out of scope leaks the storage it points to.
> #  646|            g_hash_table_insert(dict, name, val);
> #  647|         }
> #  648|->    }
> #  649|   
> #  650|      g_io_channel_unref(stream);
>  
> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:803: alloc_fn: Storage is returned from allocation function "g_strndup".
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:803: var_assign: Assigning: "val" = storage returned from "g_strndup(text, textSize)".
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:1011: noescape: Assuming resource "val" is not freed or pointed-to as ellipsis argument to "g_log".
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:1011: noescape: Resource "val" is not freed or pointed-to in "g_log".
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:1015: leaked_storage: Variable "val" going out of scope leaks the storage it points to.
> # 1013|         ASSERT(0);
> # 1014|      }
> # 1015|-> }
> # 1016|   
> # 1017|   
>  
> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:1220: alloc_arg: "VGAuth_CommReadData" allocates memory that is stored into "rawReply".
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:1283: leaked_storage: Variable "rawReply" going out of scope leaks the storage it points to.
> # 1281|      *wireReply = reply;
> # 1282|      g_markup_parse_context_free(parseContext);
> # 1283|->    return err;
> # 1284|   }
> # 1285|   
>  
> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3149: alloc_fn: Storage is returned from allocation function "g_strdup_printf".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3149: var_assign: Assigning: "badFileName" = storage returned from "g_strdup_printf("%s.bad", fullFileName)".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3151: noescape: Resource "badFileName" is not freed or pointed-to in "ServiceFileRenameFile".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3153: noescape: Assuming resource "badFileName" is not freed or pointed-to as ellipsis argument to "Audit_Event".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3162: leaked_storage: Variable "badFileName" going out of scope leaks the storage it points to.
> # 3160|                * a lot of risky work that's very hard to test, so punt for now.
> # 3161|                */
> # 3162|->             return VGAUTH_E_FAIL;
> # 3163|            } else {
> # 3164|               Audit_Event(TRUE,
>  
> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3107: alloc_fn: Storage is returned from allocation function "g_dir_open".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3107: var_assign: Assigning: "dir" = storage returned from "g_dir_open(aliasStoreRootDir, 0U, &gErr)".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3115: noescape: Resource "dir" is not freed or pointed-to in "g_dir_read_name".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3162: leaked_storage: Variable "dir" going out of scope leaks the storage it points to.
> # 3160|                * a lot of risky work that's very hard to test, so punt for now.
> # 3161|                */
> # 3162|->             return VGAUTH_E_FAIL;
> # 3163|            } else {
> # 3164|               Audit_Event(TRUE,
>  
> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3116: alloc_fn: Storage is returned from allocation function "g_strdup_printf".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3116: var_assign: Assigning: "fullFileName" = storage returned from "g_strdup_printf("%s/%s", aliasStoreRootDir, fileName)".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3121: noescape: Resource "fullFileName" is not freed or pointed-to in "AliasCheckMapFilePerms".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3149: noescape: Assuming resource "fullFileName" is not freed or pointed-to as ellipsis argument to "g_strdup_printf".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3149: noescape: Resource "fullFileName" is not freed or pointed-to in "g_strdup_printf".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3151: noescape: Resource "fullFileName" is not freed or pointed-to in "ServiceFileRenameFile".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3153: noescape: Assuming resource "fullFileName" is not freed or pointed-to as ellipsis argument to "Audit_Event".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3162: leaked_storage: Variable "fullFileName" going out of scope leaks the storage it points to.
> # 3160|                * a lot of risky work that's very hard to test, so punt for now.
> # 3161|                */
> # 3162|->             return VGAUTH_E_FAIL;
> # 3163|            } else {
> # 3164|               Audit_Event(TRUE,
>  
> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3404: alloc_fn: Storage is returned from allocation function "g_strdup_printf".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3404: var_assign: Assigning: "badRootDirName" = storage returned from "g_strdup_printf("%s.bad", aliasStoreRootDir)".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3405: noescape: Resource "badRootDirName" is not freed or pointed-to in "ServiceFileRenameFile".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3407: noescape: Assuming resource "badRootDirName" is not freed or pointed-to as ellipsis argument to "Audit_Event".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/alias.c:3412: leaked_storage: Variable "badRootDirName" going out of scope leaks the storage it points to.
> # 3410|                        aliasStoreRootDir, badRootDirName);
> # 3411|            // XXX making this fatal for now.  can we do anything better?
> # 3412|->          return VGAUTH_E_FAIL;
> # 3413|         }
> # 3414|         g_free(badRootDirName);
>  
> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/proto.c:1115: alloc_arg: "ServiceNetworkReadData" allocates memory that is stored into "data".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/proto.c:1178: leaked_storage: Variable "data" going out of scope leaks the storage it points to.
> # 1176|      }
> # 1177|   
> # 1178|->    return err;
> # 1179|   }
> # 1180|

Comment 1 jsaks 2020-10-27 02:20:37 UTC

Hi Cathy,

Fixes for the memory leaks are now available on https://github.com/vmware/open-vm-tools/tree/devel, commit e18e67f727d0354b08a55b685178fd05f542c6da.

Of the issues reported above, the following three are false positives with no fix needed:

> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/lib/slashProc/net.c:441: alloc_arg: "g_io_channel_read_line" allocates memory that is stored into "myInputLine".
> open-vm-tools-11.1.5-16724464/lib/slashProc/net.c:489: leaked_storage: Variable "myInputLine" going out of scope leaks the storage it points to.
> #  487|      g_io_channel_unref(myChannel);
> #  488|   
> #  489|->    return myHashTable;
> #  490|   }
> #  491|   

myInputLine is freed and set to NULL at lines 471-472:

      g_free(myInputLine);
      myInputLine = NULL;


> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/vgauth/common/i18n.c:613: alloc_arg: "DictLL_UnmarshalLine" allocates memory that is stored into "value".
> open-vm-tools-11.1.5-16724464/vgauth/common/i18n.c:648: leaked_storage: Variable "value" going out of scope leaks the storage it points to.
> #  646|            g_hash_table_insert(dict, name, val);
> #  647|         }
> #  648|->    }
> #  649|   
> #  650|      g_io_channel_unref(stream);

See comment on lines 621-624 as to why this isn't a problem.


> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/proto.c:1115: alloc_arg: "ServiceNetworkReadData" allocates memory that is stored into "data".
> open-vm-tools-11.1.5-16724464/vgauth/serviceImpl/proto.c:1178: leaked_storage: Variable "data" going out of scope leaks the storage it points to.
> # 1176|      }
> # 1177|   
> # 1178|->    return err;
> # 1179|   }
> # 1180|

If conn->eof (line 1117) or err != OK (line 1123), then data is NULL; all other cases free data at line 1137.


The following two are false positives, but the fixes provided includes changes that should cause Coverity to stop reporting the problem.

> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/libvmtools/vmtoolsLog.c:2450: alloc_fn: Storage is returned from allocation function "g_strjoin".
> open-vm-tools-11.1.5-16724464/libvmtools/vmtoolsLog.c:2450: var_assign: Assigning: "userLogTemp" = storage returned from "g_strjoin(appendString, *tokens, " ", NULL)".
> open-vm-tools-11.1.5-16724464/libvmtools/vmtoolsLog.c:2451: noescape: Resource "userLogTemp" is not freed or pointed-to in "g_strchomp".
> open-vm-tools-11.1.5-16724464/libvmtools/vmtoolsLog.c:2451: overwrite_var: Overwriting "userLogTemp" in "userLogTemp = g_strchomp(userLogTemp)" leaks the storage that "userLogTemp" points to.
> # 2449|      if (tokens != NULL && *tokens != NULL){
> # 2450|         userLogTemp = g_strjoin(appendString, *tokens, " ", NULL);
> # 2451|->       userLogTemp = g_strchomp (userLogTemp);
> # 2452|         if (*(tokens+1) != NULL){
> # 2453|            gchar *userLog;
>  

g_strchomp just returns its argument, so the value of userLogTemp is not changed by line 2451.  However, the code has been modified to discard the return value from g_strchomp, which should stop the issue from being reported  and also makes this use of g_strchomp consistent with other uses in this file.


> Error: RESOURCE_LEAK (CWE-772):
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:803: alloc_fn: Storage is returned from allocation function "g_strndup".
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:803: var_assign: Assigning: "val" = storage returned from "g_strndup(text, textSize)".
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:1011: noescape: Assuming resource "val" is not freed or pointed-to as ellipsis argument to "g_log".
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:1011: noescape: Resource "val" is not freed or pointed-to in "g_log".
> open-vm-tools-11.1.5-16724464/vgauth/lib/proto.c:1015: leaked_storage: Variable "val" going out of scope leaks the storage it points to.
> # 1013|         ASSERT(0);
> # 1014|      }
> # 1015|-> }
> # 1016|   
> # 1017|   

ASSERT(0) is an indicator this code path is never executed.  However a call to g_free has been inserted before the ASSERT, which should stop the issue from being reported and also prevent any memory leak if for some reason this path is executed.


The other seven issues have been fixed.

Let me know how your new scan turns out and whether you run into further problems.

Regards,

Jonathan

Comment 2 Cathy Avery 2020-10-28 12:36:25 UTC

Great! So commit e18e67f727d0354b08a55b685178fd05f542c6da contains all the fixes.

Thanks,

Cathy

Comment 3 Cathy Avery 2020-12-06 18:41:35 UTC

We have decided to use 11.2.0 for RHEL8.4 and not 11.1.5 so closing.