Blob Blame History Raw
From 62b5ab4f78de027021bb5557568e607270314f82 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 12 Dec 2017 10:22:59 +0100
Subject: [PATCH 1/7] core: fix uninialized boolean variable in
 reset_autoconnect_all()

It's not critical, because at worst we get a false-positive that
something changed.

Found by coverity.

Fixes: 4e7b05de7981c28eba6db48eb16476372594fed2
(cherry picked from commit fbc6008260f58b625e3a6bd26129c66b51904012)
(cherry picked from commit 387377d8fce937de4ed583fdf631d6dcf35edf93)
---
 src/nm-policy.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/nm-policy.c b/src/nm-policy.c
index 8dfb0ab00..4d0ef91a1 100644
--- a/src/nm-policy.c
+++ b/src/nm-policy.c
@@ -1409,7 +1409,7 @@ reset_autoconnect_all (NMPolicy *self,
 	NMPolicyPrivate *priv = NM_POLICY_GET_PRIVATE (self);
 	NMSettingsConnection *const*connections = NULL;
 	guint i;
-	gboolean changed;
+	gboolean changed = FALSE;
 
 	_LOGD (LOGD_DEVICE, "re-enabling autoconnect for all connections%s%s%s",
 	       device ? " on " : "",
-- 
2.14.3


From 3b3e530fedfceafb694a961482e22e64c0a76348 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 12 Dec 2017 10:27:51 +0100
Subject: [PATCH 2/7] core: avoid dereferencing NULL in
 nm_utils_resolve_conf_parse()

Found by coverity.

Fixes: 8f1ef161f4dd5ac197b622ac681d55d64c176797
(cherry picked from commit a7087b1f0539d43783c67c36ba9ed2165f81cb77)
(cherry picked from commit 1bb6b3a79f1e8cf2790aca1f78ccafad8ef7d56c)
---
 src/nm-core-utils.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c
index 40a0c352a..f6b33a149 100644
--- a/src/nm-core-utils.c
+++ b/src/nm-core-utils.c
@@ -1960,8 +1960,7 @@ nm_utils_resolve_conf_parse (int addr_family,
 				gsize i_tokens;
 
 				tokens = nm_utils_strsplit_set (s, " \t");
-				nm_assert (tokens);
-				for (i_tokens = 0; tokens[i_tokens]; i_tokens++) {
+				for (i_tokens = 0; tokens && tokens[i_tokens]; i_tokens++) {
 					gs_free char *t = g_strstrip (g_strdup (tokens[i_tokens]));
 
 					if (   _nm_utils_dns_option_validate (t, NULL, NULL,
-- 
2.14.3


From f67c30627b7c8369b957909abb84be27011ee2aa Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 12 Dec 2017 10:31:01 +0100
Subject: [PATCH 3/7] libnm: fix checking argument in
 nm_team_link_watcher_new_arp_ping()

Found by coverity.

Fixes: 6c93e32212f9901e882d5a9ca221d037162bc740
(cherry picked from commit fb2da4b26c86a1916ad52cba00d6e6c725e74f4c)
(cherry picked from commit ccdfe9d3e500d0b695de2f528745924583c81c27)
---
 libnm-core/nm-setting-team.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/libnm-core/nm-setting-team.c b/libnm-core/nm-setting-team.c
index d36882783..88008e422 100644
--- a/libnm-core/nm-setting-team.c
+++ b/libnm-core/nm-setting-team.c
@@ -249,7 +249,7 @@ nm_team_link_watcher_new_arp_ping (gint init_wait,
 		return NULL;
 	}
 
-	if (strpbrk (target_host, " \\/\t=\"\'")) {
+	if (strpbrk (source_host, " \\/\t=\"\'")) {
 		g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_FAILED,
 		             _("source-host '%s' contains invalid characters"), source_host);
 		return NULL;
-- 
2.14.3


From fbf92a9d6e73a9b47a149fd451a6d4edfbe00c05 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 12 Dec 2017 10:37:59 +0100
Subject: [PATCH 4/7] platform: fix crash hashing NMPlatformTfilter and
 NMPlatformQdisc

@kind might be NULL. There are 3 forms of the hash-update functions for
string: str(), str0(), and strarr().

- str0() is when the string might be NULL.
- str() does not allow the string to be NULL
- strarr() is like str(), except it adds a G_STATIC_ASSERT()
  that the argument is a C array.

The reason why a difference between str() and str0() exists, is
because str0() hashes NULL different from a "" or any other string.
This has an overhead, because it effectively must hash another bit
of information that tells whether a string was passed or not.

The reason is, that hashing a tupple of two strings should always
yield a different hash value, even for "aa",""; "a","a"; "","aa",
where naive concatentation would yield identical hash values in all
three cases.

Fixes: e75fc8279becce29e688551930d85e59e1280c89
(cherry picked from commit 27e8fffdb833748dfeb6648b8768c4ef48822841)
(cherry picked from commit 99eef7a2ea3e7f79e2f449a04d7a3c78ba05f427)
---
 src/platform/nm-platform.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index 2ca379e4e..70675579f 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -5320,7 +5320,7 @@ nm_platform_qdisc_to_string (const NMPlatformQdisc *qdisc, char *buf, gsize len)
 void
 nm_platform_qdisc_hash_update (const NMPlatformQdisc *obj, NMHashState *h)
 {
-	nm_hash_update_str (h, obj->kind);
+	nm_hash_update_str0 (h, obj->kind);
 	nm_hash_update_vals (h,
 	                     obj->ifindex,
 	                     obj->addr_family,
@@ -5387,17 +5387,17 @@ nm_platform_tfilter_to_string (const NMPlatformTfilter *tfilter, char *buf, gsiz
 void
 nm_platform_tfilter_hash_update (const NMPlatformTfilter *obj, NMHashState *h)
 {
-	nm_hash_update_str (h, obj->kind);
+	nm_hash_update_str0 (h, obj->kind);
 	nm_hash_update_vals (h,
 	                     obj->ifindex,
 	                     obj->addr_family,
 	                     obj->handle,
 	                     obj->parent,
 	                     obj->info);
-	nm_hash_update_str (h, obj->action.kind);
 	if (obj->action.kind) {
+		nm_hash_update_str (h, obj->action.kind);
 		if (nm_streq (obj->action.kind, NM_PLATFORM_ACTION_KIND_SIMPLE))
-			nm_hash_update_str (h, obj->action.simple.sdata);
+			nm_hash_update_strarr (h, obj->action.simple.sdata);
 	}
 }
 
-- 
2.14.3


From 55a7ac49eb196937bc76db4a292832b266df5398 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 12 Dec 2017 10:49:09 +0100
Subject: [PATCH 5/7] platform: assert() for valid item in
 nm_platform_link_get_all()

Coverity thinks that item might be NULL, but actually it
cannot. Unclear how to avoid the false positive.

(cherry picked from commit 62d4dba74bac791c06ba70547f81b78eab5b9dc5)
(cherry picked from commit 490911c6fa368692b4a6dfefdc2a1e253b40f7df)
---
 src/platform/nm-platform.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c
index 70675579f..c7ed90e3f 100644
--- a/src/platform/nm-platform.c
+++ b/src/platform/nm-platform.c
@@ -667,6 +667,7 @@ skip:
 			/* There is a loop, pop the first (remaining) element from the list.
 			 * This can happen for veth pairs where each peer is parent of the other end. */
 			item = NMP_OBJECT_CAST_LINK (links->pdata[first_idx]);
+			nm_assert (item);
 			g_hash_table_remove (unseen, GINT_TO_POINTER (item->ifindex));
 			g_ptr_array_add (result, links->pdata[first_idx]);
 			links->pdata[first_idx] = NULL;
-- 
2.14.3


From 954ae22f4219db60aeee641a3026d37fe7b71614 Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 12 Dec 2017 10:59:26 +0100
Subject: [PATCH 6/7] cli: avoid out-of-bounds-read for show_device_info()

Probably not critical, because it will still include
the terminating NULL, and just continue to fill the
temporary buffer with static addresses.

Found by coverity.

Fixes: bfb9fd0d2f3d602a69537fe7776426ee9202ce9e
(cherry picked from commit c274b565a66e0c2932377554ce4d33a4772602e4)
(cherry picked from commit 7fb865947cd131d34703f0dc7ce153fb2e6676fc)
---
 clients/cli/devices.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/clients/cli/devices.c b/clients/cli/devices.c
index 467ce427d..da969fbe7 100644
--- a/clients/cli/devices.c
+++ b/clients/cli/devices.c
@@ -1116,8 +1116,8 @@ show_device_info (NMDevice *device, NmCli *nmc)
 		                                   (const NMMetaAbstractInfo *const*) nmc_fields_dev_show_general,
 		                                   FALSE, NULL, NULL);
 
-		row = g_new0 (NmcOutputField, _NM_META_SETTING_TYPE_NUM + 1);
-		for (i = 0; i < _NM_META_SETTING_TYPE_NUM; i++)
+		row = g_new0 (NmcOutputField, G_N_ELEMENTS (nmc_fields_dev_show_general));
+		for (i = 0; i < G_N_ELEMENTS (nmc_fields_dev_show_general); i++)
 			row[i].info = (const NMMetaAbstractInfo *) &nmc_fields_dev_show_general[i];
 
 		print_required_fields (&nmc->nmc_config, NMC_OF_FLAG_MAIN_HEADER_ONLY,
-- 
2.14.3


From 187d0038f5e929b338a005804386851d3d5c7e8b Mon Sep 17 00:00:00 2001
From: Thomas Haller <thaller@redhat.com>
Date: Tue, 12 Dec 2017 11:05:52 +0100
Subject: [PATCH 7/7] core: avoid leaks parsing team link-watcher

Found by coverity.

(cherry picked from commit f44f21c87e157c84ab865334f115e9fecd302465)
(cherry picked from commit 7673a3badc475998fc8f4169d0e2831d1625f1f9)
---
 libnm-core/nm-utils.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c
index 6cc92ec8d..9aafdd914 100644
--- a/libnm-core/nm-utils.c
+++ b/libnm-core/nm-utils.c
@@ -5185,13 +5185,16 @@ _nm_utils_team_link_watcher_from_json (json_t *json_element)
 	g_return_val_if_fail (json_element, NULL);
 
 	json_object_foreach (json_element, j_key, j_val) {
-		if (nm_streq (j_key, "name"))
+		if (nm_streq (j_key, "name")) {
+			g_free (name);
 			name = strdup (json_string_value (j_val));
-		else if (nm_streq (j_key, "target_host"))
+		} else if (nm_streq (j_key, "target_host")) {
+			g_free (target_host);
 			target_host = strdup (json_string_value (j_val));
-		else if (nm_streq (j_key, "source_host"))
+		} else if (nm_streq (j_key, "source_host")) {
+			g_free (source_host);
 			source_host = strdup (json_string_value (j_val));
-		else if (NM_IN_STRSET (j_key, "delay_up", "init_wait"))
+		} else if (NM_IN_STRSET (j_key, "delay_up", "init_wait"))
 			val1 = json_integer_value (j_val);
 		else if (NM_IN_STRSET (j_key, "delay_down", "interval"))
 			val2 = json_integer_value (j_val);
-- 
2.14.3