From 5b36f215f4592a528e345ce7e2b8299ee9ca4282 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 6 May 2021 16:54:35 +0200 Subject: [PATCH] ifcfg-rh: fix code that looks like a leak in write_bridge_vlans() "string" is leaked in the error case. But in practice, this cannot happen because nm_bridge_vlan_to_str() cannot fail. While at it, replace GString by NMStrBuf. Thanks Coverity: Error: RESOURCE_LEAK (CWE-772): NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:1565: alloc_fn: Storage is returned from allocation function "g_string_new". NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:1565: var_assign: Assigning: "string" = storage returned from "g_string_new("")". NetworkManager-1.31.3/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c:1572: leaked_storage: Variable "string" going out of scope leaks the storage it points to. # 1570| vlan_str = nm_bridge_vlan_to_str(vlan, error); # 1571| if (!vlan_str) # 1572|-> return FALSE; # 1573| if (string->len > 0) # 1574| g_string_append(string, ","); --- .../settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c index d5ceab3217..4338c30593 100644 --- a/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c +++ b/src/core/settings/plugins/ifcfg-rh/nms-ifcfg-rh-writer.c @@ -16,6 +16,7 @@ #include #include "libnm-glib-aux/nm-enum-utils.h" +#include "libnm-glib-aux/nm-str-buf.h" #include "libnm-glib-aux/nm-io-utils.h" #include "nm-manager.h" #include "nm-setting-connection.h" @@ -1558,7 +1559,7 @@ write_bridge_vlans(NMSetting * setting, { gs_unref_ptrarray GPtrArray *vlans = NULL; NMBridgeVlan * vlan; - GString * string; + nm_auto_str_buf NMStrBuf strbuf = NM_STR_BUF_INIT(0, FALSE); guint i; g_object_get(setting, property_name, &vlans, NULL); @@ -1566,7 +1567,6 @@ write_bridge_vlans(NMSetting * setting, if (!vlans || !vlans->len) return TRUE; - string = g_string_new(""); for (i = 0; i < vlans->len; i++) { gs_free char *vlan_str = NULL; @@ -1574,13 +1574,12 @@ write_bridge_vlans(NMSetting * setting, vlan_str = nm_bridge_vlan_to_str(vlan, error); if (!vlan_str) return FALSE; - if (string->len > 0) - g_string_append(string, ","); - nm_utils_escaped_tokens_escape_gstr_assert(vlan_str, ",", string); + if (strbuf.len > 0) + nm_str_buf_append_c(&strbuf, ','); + nm_str_buf_append(&strbuf, nm_utils_escaped_tokens_escape_unnecessary(vlan_str, ",")); } - svSetValueStr(ifcfg, key, string->str); - g_string_free(string, TRUE); + svSetValueStr(ifcfg, key, nm_str_buf_get_str(&strbuf)); return TRUE; }