From 8e0f2bf38db6e8cc954c0365a41aa683a1849682 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 5 Oct 2017 10:55:31 +0200 Subject: [PATCH] device: fix delay startup complete for unrealized devices (1:1.8.4-4) And 2 more fixes from upstream. --- ...-startup-complete-for-unrealized-dev.patch | 37 ++ ...n-notify-signals-on-unrealize-error-.patch | 40 +++ 0006-keyfile-route-metric-zero-fix.patch | 325 ++++++++++++++++++ NetworkManager.spec | 12 +- 4 files changed, 413 insertions(+), 1 deletion(-) create mode 100644 0004-device-fix-delay-startup-complete-for-unrealized-dev.patch create mode 100644 0005-device-fix-frozen-notify-signals-on-unrealize-error-.patch create mode 100644 0006-keyfile-route-metric-zero-fix.patch diff --git a/0004-device-fix-delay-startup-complete-for-unrealized-dev.patch b/0004-device-fix-delay-startup-complete-for-unrealized-dev.patch new file mode 100644 index 0000000..6d9bf2d --- /dev/null +++ b/0004-device-fix-delay-startup-complete-for-unrealized-dev.patch @@ -0,0 +1,37 @@ +From 613a88779bf424c9bcae20ccd9c4c04574f37554 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Fri, 29 Sep 2017 17:08:55 +0200 +Subject: [PATCH 4/5] device: fix delay startup complete for unrealized devices + +Since commit 6845b9b80a9fcec9d2c9e7b56a37329f38089f2e ("device: delay +startup complete until device is initialized in platform", we also wait +for devices that are still initializing platform/UDEV. + +Obviously, that only applies to realized devices. + +Otherwise, an unrealized device is going to block startup complete. + +Fixes: 6845b9b80a9fcec9d2c9e7b56a37329f38089f2e +(cherry picked from commit 9ad8010fe0c42291580e4a801ed85947ae660d38) +(cherry picked from commit 0ba498b17dc582dcbd9b7102e03496f391d67812) +--- + src/devices/nm-device.c | 3 ++- + 1 file changed, 2 insertions(+), 1 deletion(-) + +diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c +index bacbfb33e..a085e466d 100644 +--- a/src/devices/nm-device.c ++++ b/src/devices/nm-device.c +@@ -12030,7 +12030,8 @@ nm_device_has_pending_action (NMDevice *self) + if (priv->pending_actions) + return TRUE; + +- if (nm_device_get_unmanaged_flags (self, NM_UNMANAGED_PLATFORM_INIT)) { ++ if ( nm_device_is_real (self) ++ && nm_device_get_unmanaged_flags (self, NM_UNMANAGED_PLATFORM_INIT)) { + /* as long as the platform link is not yet initialized, we have a pending + * action. */ + return TRUE; +-- +2.13.6 + diff --git a/0005-device-fix-frozen-notify-signals-on-unrealize-error-.patch b/0005-device-fix-frozen-notify-signals-on-unrealize-error-.patch new file mode 100644 index 0000000..d8b3c2f --- /dev/null +++ b/0005-device-fix-frozen-notify-signals-on-unrealize-error-.patch @@ -0,0 +1,40 @@ +From 7d99bbea013d9f01d764e3394b77f1f709047129 Mon Sep 17 00:00:00 2001 +From: Beniamino Galvani +Date: Wed, 4 Oct 2017 14:30:57 +0200 +Subject: [PATCH 5/5] device: fix frozen notify signals on unrealize error path + +If unrealize() failed we returned without thawing notify signals. Fix +this by moving g_object_freeze_notify() after the +unrealization/deletion but before the properties are reset in +unrealize_notify(). + +Fixes: a93807c288743f499362f7edfe0674020762811c +(cherry picked from commit 24a7f88bc56b66745c1e6b9444df8a80125de059) +(cherry picked from commit 5bd8269315fc7d41c62e258689a05bf062c6f592) +--- + src/devices/nm-device.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c +index a085e466d..6321d2b49 100644 +--- a/src/devices/nm-device.c ++++ b/src/devices/nm-device.c +@@ -3257,7 +3257,6 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error) + g_return_val_if_fail (priv->iface != NULL, FALSE); + g_return_val_if_fail (priv->real, FALSE); + +- g_object_freeze_notify (G_OBJECT (self)); + + ifindex = nm_device_get_ifindex (self); + +@@ -3274,6 +3273,7 @@ nm_device_unrealize (NMDevice *self, gboolean remove_resources, GError **error) + } + } + ++ g_object_freeze_notify (G_OBJECT (self)); + NM_DEVICE_GET_CLASS (self)->unrealize_notify (self); + + _parent_set_ifindex (self, 0, FALSE); +-- +2.13.6 + diff --git a/0006-keyfile-route-metric-zero-fix.patch b/0006-keyfile-route-metric-zero-fix.patch new file mode 100644 index 0000000..e168be4 --- /dev/null +++ b/0006-keyfile-route-metric-zero-fix.patch @@ -0,0 +1,325 @@ +From 5c5876732c51adcf0e1973021bc26a663b240ec9 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Wed, 4 Oct 2017 11:14:48 +0200 +Subject: [PATCH 6/8] keyfile: minor cleanup in get_one_int() to use + _nm_utils_ascii_str_to_int64() + +(cherry picked from commit 72c28cb6bcc26e6a63083e4d92f8f66ee5c121e4) +(cherry picked from commit 14f0f23e77219364c0ee7ae692aae35551101ed8) +--- + libnm-core/nm-keyfile-reader.c | 11 +++++------ + 1 file changed, 5 insertions(+), 6 deletions(-) + +diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c +index eb257eeb1..15a0e406f 100644 +--- a/libnm-core/nm-keyfile-reader.c ++++ b/libnm-core/nm-keyfile-reader.c +@@ -133,8 +133,7 @@ read_array_of_uint (GKeyFile *file, + static gboolean + get_one_int (KeyfileReaderInfo *info, const char *property_name, const char *str, guint32 max_val, guint32 *out) + { +- long tmp; +- char *endptr; ++ gint64 tmp; + + g_return_val_if_fail (!info == !property_name, FALSE); + +@@ -145,13 +144,13 @@ get_one_int (KeyfileReaderInfo *info, const char *property_name, const char *str + return FALSE; + } + +- errno = 0; +- tmp = strtol (str, &endptr, 10); +- if (errno || (tmp < 0) || (tmp > max_val) || *endptr != 0) { +- if (property_name) ++ tmp = _nm_utils_ascii_str_to_int64 (str, 10, 0, max_val, -1); ++ if (tmp == -1) { ++ if (property_name) { + handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, + _("ignoring invalid number '%s'"), + str); ++ } + return FALSE; + } + +-- +2.13.6 + + +From e843259d6a13e9219cf151432ed3794246c7d067 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Wed, 4 Oct 2017 11:16:36 +0200 +Subject: [PATCH 7/8] keyfile: cleanup error argument for read_field() + +Rename @error to @out_err_str, because @error is usually used for GError +output arguments. + +Also, make the string variables "const char *". + +Use nm_assert() in read_field(), because it is a static function +with only four call sites. It's easily verified that the assertion +holds, so no need for a run-time check in production builds. + +(cherry picked from commit 29e9b567f0938fd202a433e7098092f0a39723ed) +(cherry picked from commit f889aa783d776afa200587b5891e3578a3033518) +--- + libnm-core/nm-keyfile-reader.c | 58 ++++++++++++++++++++++++------------------ + 1 file changed, 33 insertions(+), 25 deletions(-) + +diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c +index 15a0e406f..5934c833b 100644 +--- a/libnm-core/nm-keyfile-reader.c ++++ b/libnm-core/nm-keyfile-reader.c +@@ -249,17 +249,17 @@ build_route (KeyfileReaderInfo *info, + * When @current target is %NULL, gracefully fail returning %NULL while + * leaving the @current target %NULL end setting @error to %NULL; + */ +-static char * +-read_field (char **current, char **error, const char *characters, const char *delimiters) ++static const char * ++read_field (char **current, const char **out_err_str, const char *characters, const char *delimiters) + { +- char *start; ++ const char *start; + +- g_return_val_if_fail (current, NULL); +- g_return_val_if_fail (error, NULL); +- g_return_val_if_fail (characters, NULL); +- g_return_val_if_fail (delimiters, NULL); ++ nm_assert (current); ++ nm_assert (out_err_str); ++ nm_assert (characters); ++ nm_assert (delimiters); + +- *error = NULL; ++ *out_err_str = NULL; + + if (!*current) { + /* graceful failure, leave '*current' NULL */ +@@ -282,8 +282,8 @@ read_field (char **current, char **error, const char *characters, const char *de + return start; + } else { + /* error, bad character */ +- *error = *current; +- *current = start; ++ *out_err_str = *current; ++ *current = (char *) start; + return NULL; + } + else { +@@ -332,42 +332,50 @@ read_one_ip_address_or_route (KeyfileReaderInfo *info, + char **out_gateway, + NMSetting *setting) + { +- guint32 plen = G_MAXUINT32; ++ guint plen; + gpointer result; +- char *address_str, *plen_str, *gateway_str, *metric_str, *current, *error; +- gs_free char *value = NULL, *value_orig = NULL; ++ const char *address_str; ++ const char *plen_str; ++ const char *gateway_str; ++ const char *metric_str; ++ const char *err_str = NULL; ++ char *current; ++ gs_free char *value = NULL; ++ gs_free char *value_orig = NULL; + + #define VALUE_ORIG() (value_orig ? value_orig : (value_orig = nm_keyfile_plugin_kf_get_string (info->keyfile, setting_name, key_name, NULL))) + +- current = value = nm_keyfile_plugin_kf_get_string (info->keyfile, setting_name, key_name, NULL); ++ value = nm_keyfile_plugin_kf_get_string (info->keyfile, setting_name, key_name, NULL); + if (!value) + return NULL; + ++ current = value; ++ + /* get address field */ +- address_str = read_field (¤t, &error, IP_ADDRESS_CHARS, DELIMITERS); +- if (error) { ++ address_str = read_field (¤t, &err_str, IP_ADDRESS_CHARS, DELIMITERS); ++ if (err_str) { + handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, + _("unexpected character '%c' for address %s: '%s' (position %td)"), +- *error, key_name, VALUE_ORIG (), error - current); ++ *err_str, key_name, VALUE_ORIG (), err_str - current); + return NULL; + } + /* get prefix length field (skippable) */ +- plen_str = read_field (¤t, &error, DIGITS, DELIMITERS); ++ plen_str = read_field (¤t, &err_str, DIGITS, DELIMITERS); + /* get gateway field */ +- gateway_str = read_field (¤t, &error, IP_ADDRESS_CHARS, DELIMITERS); +- if (error) { ++ gateway_str = read_field (¤t, &err_str, IP_ADDRESS_CHARS, DELIMITERS); ++ if (err_str) { + handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, + _("unexpected character '%c' for %s: '%s' (position %td)"), +- *error, key_name, VALUE_ORIG (), error - current); ++ *err_str, key_name, VALUE_ORIG (), err_str - current); + return NULL; + } + /* for routes, get metric */ + if (route) { +- metric_str = read_field (¤t, &error, DIGITS, DELIMITERS); +- if (error) { ++ metric_str = read_field (¤t, &err_str, DIGITS, DELIMITERS); ++ if (err_str) { + handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, + _("unexpected character '%c' in prefix length for %s: '%s' (position %td)"), +- *error, key_name, VALUE_ORIG (), error - current); ++ *err_str, key_name, VALUE_ORIG (), err_str - current); + return NULL; + } + } else +@@ -393,7 +401,7 @@ read_one_ip_address_or_route (KeyfileReaderInfo *info, + + /* parse plen, fallback to defaults */ + if (plen_str) { +- if (!get_one_int (info, property_name, plen_str, ipv6 ? 128 : 32, &plen) ++ if ( !get_one_int (info, property_name, plen_str, ipv6 ? 128 : 32, &plen) + || (route && plen == 0)) { + plen = DEFAULT_PREFIX (route, ipv6); + if ( info->error +-- +2.13.6 + + +From 0a76ddaad11baec08ab0826a5d635fa5b158c6e4 Mon Sep 17 00:00:00 2001 +From: Thomas Haller +Date: Wed, 4 Oct 2017 11:28:15 +0200 +Subject: [PATCH 8/8] keyfile: fix reading/writing route metric zero + +Zero is a valid route metric and distinct from -1, which means unspecified. +Fix reader and writer. + +Fixes: e374923bbe4a9f608644756f749b9bae9aa5f349 +(cherry picked from commit 099be8e4db0b00d4ff3ded60a4a3cb65d55bbd40) +(cherry picked from commit 482fcb507e0b7d611701d9537321cdc6d58d3b84) +--- + libnm-core/nm-keyfile-reader.c | 15 +++++++++------ + libnm-core/nm-keyfile-writer.c | 12 +++++++----- + src/settings/plugins/keyfile/tests/test-keyfile.c | 6 +++--- + 3 files changed, 19 insertions(+), 14 deletions(-) + +diff --git a/libnm-core/nm-keyfile-reader.c b/libnm-core/nm-keyfile-reader.c +index 5934c833b..0ac417cdb 100644 +--- a/libnm-core/nm-keyfile-reader.c ++++ b/libnm-core/nm-keyfile-reader.c +@@ -185,7 +185,8 @@ build_route (KeyfileReaderInfo *info, + const char *gateway_str, const char *metric_str) + { + NMIPRoute *route; +- guint32 metric = 0; ++ guint32 u32; ++ gint64 metric = -1; + GError *error = NULL; + + g_return_val_if_fail (plen, NULL); +@@ -204,9 +205,10 @@ build_route (KeyfileReaderInfo *info, + **/ + if ( family == AF_INET6 + && !metric_str +- && get_one_int (NULL, NULL, gateway_str, G_MAXUINT32, &metric)) ++ && get_one_int (NULL, NULL, gateway_str, G_MAXUINT32, &u32)) { ++ metric = u32; + gateway_str = NULL; +- else { ++ } else { + if (!info->error) { + handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, + _("ignoring invalid gateway '%s' for %s route"), +@@ -218,14 +220,15 @@ build_route (KeyfileReaderInfo *info, + } else + gateway_str = NULL; + +- /* parse metric, default to 0 */ ++ /* parse metric, default to -1 */ + if (metric_str) { +- if (!get_one_int (info, property_name, metric_str, G_MAXUINT32, &metric)) ++ if (!get_one_int (info, property_name, metric_str, G_MAXUINT32, &u32)) + return NULL; ++ metric = u32; + } + + route = nm_ip_route_new (family, dest_str, plen, gateway_str, +- metric ? (gint64) metric : -1, ++ metric, + &error); + if (!route) { + handle_warn (info, property_name, NM_KEYFILE_WARN_SEVERITY_WARN, +diff --git a/libnm-core/nm-keyfile-writer.c b/libnm-core/nm-keyfile-writer.c +index 6a3d9a9f4..19b734a05 100644 +--- a/libnm-core/nm-keyfile-writer.c ++++ b/libnm-core/nm-keyfile-writer.c +@@ -137,7 +137,7 @@ write_ip_values (GKeyFile *file, + GString *output; + int family, i; + const char *addr, *gw; +- guint32 plen, metric; ++ guint32 plen; + char key_name[64], *key_name_idx; + + if (!array->len) +@@ -150,25 +150,27 @@ write_ip_values (GKeyFile *file, + + output = g_string_sized_new (2*INET_ADDRSTRLEN + 10); + for (i = 0; i < array->len; i++) { ++ gint64 metric = -1; ++ + if (is_route) { + NMIPRoute *route = array->pdata[i]; + + addr = nm_ip_route_get_dest (route); + plen = nm_ip_route_get_prefix (route); + gw = nm_ip_route_get_next_hop (route); +- metric = MAX (0, nm_ip_route_get_metric (route)); ++ metric = nm_ip_route_get_metric (route); + } else { + NMIPAddress *address = array->pdata[i]; + + addr = nm_ip_address_get_address (address); + plen = nm_ip_address_get_prefix (address); + gw = i == 0 ? gateway : NULL; +- metric = 0; + } + + g_string_set_size (output, 0); + g_string_append_printf (output, "%s/%u", addr, plen); +- if (metric || gw) { ++ if ( metric != -1 ++ || gw) { + /* Older versions of the plugin do not support the form + * "a.b.c.d/plen,,metric", so, we always have to write the + * gateway, even if there isn't one. +@@ -182,7 +184,7 @@ write_ip_values (GKeyFile *file, + } + + g_string_append_printf (output, ",%s", gw); +- if (metric) ++ if (is_route && metric != -1) + g_string_append_printf (output, ",%lu", (unsigned long) metric); + } + +diff --git a/src/settings/plugins/keyfile/tests/test-keyfile.c b/src/settings/plugins/keyfile/tests/test-keyfile.c +index d9da53173..2584a7229 100644 +--- a/src/settings/plugins/keyfile/tests/test-keyfile.c ++++ b/src/settings/plugins/keyfile/tests/test-keyfile.c +@@ -312,11 +312,11 @@ test_read_valid_wired_connection (void) + check_ip_route (s_ip4, 3, "1.1.1.3", 13, NULL, -1); + check_ip_route (s_ip4, 4, "1.1.1.4", 14, "2.2.2.4", -1); + check_ip_route (s_ip4, 5, "1.1.1.5", 15, "2.2.2.5", -1); +- check_ip_route (s_ip4, 6, "1.1.1.6", 16, "2.2.2.6", -1); ++ check_ip_route (s_ip4, 6, "1.1.1.6", 16, "2.2.2.6", 0); + check_ip_route (s_ip4, 7, "1.1.1.7", 17, NULL, -1); + check_ip_route (s_ip4, 8, "1.1.1.8", 18, NULL, -1); +- check_ip_route (s_ip4, 9, "1.1.1.9", 19, NULL, -1); +- check_ip_route (s_ip4, 10, "1.1.1.10", 20, NULL, -1); ++ check_ip_route (s_ip4, 9, "1.1.1.9", 19, NULL, 0); ++ check_ip_route (s_ip4, 10, "1.1.1.10", 20, NULL, 0); + check_ip_route (s_ip4, 11, "1.1.1.11", 21, NULL, 21); + + /* Route attributes */ +-- +2.13.6 + diff --git a/NetworkManager.spec b/NetworkManager.spec index 52c63c9..4e48d30 100644 --- a/NetworkManager.spec +++ b/NetworkManager.spec @@ -9,7 +9,7 @@ %global epoch_version 1 %global rpm_version 1.8.4 %global real_version 1.8.4 -%global release_version 3 +%global release_version 4 %global snapshot %{nil} %global git_sha %{nil} @@ -88,6 +88,9 @@ Source3: 20-connectivity-fedora.conf Patch1: 0001-manager-Disconnect-from-signals-on-the-proxy-when-we.patch Patch2: 0002-vpn-remote-connection-disconnect-signal-handlers-whe.patch Patch3: 0003-cli-fix-crash-in-interactive-mode-for-describe.patch +Patch4: 0004-device-fix-delay-startup-complete-for-unrealized-dev.patch +Patch5: 0005-device-fix-frozen-notify-signals-on-unrealize-error-.patch +Patch6: 0006-keyfile-route-metric-zero-fix.patch Requires(post): systemd Requires(preun): systemd @@ -346,6 +349,9 @@ by nm-connection-editor and nm-applet in a non-graphical environment. %patch1 -p1 %patch2 -p1 %patch3 -p1 +%patch4 -p1 +%patch5 -p1 +%patch6 -p1 %build %if %{with regen_docs} @@ -674,6 +680,10 @@ fi %endif %changelog +* Thu Oct 5 2017 Thomas Haller - 1:1.8.4-4 +- device: fix frozen notify signals on unrealize error path +- device: fix delay startup complete for unrealized devices +- keyfile: fix handling routes with metric zero * Fri Sep 29 2017 Thomas Haller - 1:1.8.4-3 - cli: fix crash in interactive mode for "describe ."