From 83dadca08ef9606a8541b26d3a2b93a66d828055 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Jul 2015 11:40:35 +0200 Subject: [PATCH 1/9] device/trivial: rename private field in NMDevicePrivate Rename "default_route.v4_configure_first_time" to "v4_commit_first_time". For one, the name "commit" matches better to the @commit variable in ip4_config_merge_and_apply() and ip6_config_merge_and_apply(). Then, we don't need this information only for default-routes, so move the variable out of the @default_route struct. (cherry picked from commit ad03cdbc73dad81aa8934afa2060ccbd9e776f7f) --- src/devices/nm-device.c | 27 +++++++++++++++------------ 1 file changed, 15 insertions(+), 12 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index bf4ca8e6e0..867f3846f9 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -271,14 +271,15 @@ typedef struct { struct { gboolean v4_has; gboolean v4_is_assumed; - gboolean v4_configure_first_time; NMPlatformIP4Route v4; gboolean v6_has; gboolean v6_is_assumed; - gboolean v6_configure_first_time; NMPlatformIP6Route v6; } default_route; + gboolean v4_commit_first_time; + gboolean v6_commit_first_time; + /* DHCPv4 tracking */ NMDhcpClient * dhcp4_client; gulong dhcp4_state_sigid; @@ -3302,7 +3303,7 @@ ip4_config_merge_and_apply (NMDevice *self, priv->default_route.v4_is_assumed = TRUE; routes_full_sync = commit - && priv->default_route.v4_configure_first_time + && priv->v4_commit_first_time && !nm_device_uses_assumed_connection (self); if (!commit) { @@ -3314,7 +3315,7 @@ ip4_config_merge_and_apply (NMDevice *self, = nm_default_route_manager_ip4_connection_has_default_route (nm_default_route_manager_get (), connection, &connection_is_never_default); - if ( !priv->default_route.v4_configure_first_time + if ( !priv->v4_commit_first_time && !nm_device_uses_assumed_connection (self) && connection_is_never_default) { /* If the connection is explicitly configured as never-default, we enforce the (absense of the) @@ -3330,7 +3331,7 @@ ip4_config_merge_and_apply (NMDevice *self, /* we are about to commit (for a non-assumed connection). Enforce whatever we have * configured. */ - priv->default_route.v4_configure_first_time = FALSE; + priv->v4_commit_first_time = FALSE; priv->default_route.v4_is_assumed = FALSE; if (!connection_has_default_route) @@ -3920,7 +3921,7 @@ ip6_config_merge_and_apply (NMDevice *self, priv->default_route.v6_is_assumed = TRUE; routes_full_sync = commit - && priv->default_route.v6_configure_first_time + && priv->v6_commit_first_time && !nm_device_uses_assumed_connection (self); if (!commit) { @@ -3932,7 +3933,7 @@ ip6_config_merge_and_apply (NMDevice *self, = nm_default_route_manager_ip6_connection_has_default_route (nm_default_route_manager_get (), connection, &connection_is_never_default); - if ( !priv->default_route.v6_configure_first_time + if ( !priv->v6_commit_first_time && !nm_device_uses_assumed_connection (self) && connection_is_never_default) { /* If the connection is explicitly configured as never-default, we enforce the (absence of the) @@ -3948,7 +3949,7 @@ ip6_config_merge_and_apply (NMDevice *self, /* we are about to commit (for a non-assumed connection). Enforce whatever we have * configured. */ - priv->default_route.v6_configure_first_time = FALSE; + priv->v6_commit_first_time = FALSE; priv->default_route.v6_is_assumed = FALSE; if (!connection_has_default_route) @@ -7930,10 +7931,11 @@ _cleanup_generic_post (NMDevice *self, CleanupType cleanup_type) priv->default_route.v4_has = FALSE; priv->default_route.v4_is_assumed = TRUE; - priv->default_route.v4_configure_first_time = TRUE; priv->default_route.v6_has = FALSE; priv->default_route.v6_is_assumed = TRUE; - priv->default_route.v6_configure_first_time = TRUE; + + priv->v4_commit_first_time = TRUE; + priv->v6_commit_first_time = TRUE; nm_default_route_manager_ip4_update_default_route (nm_default_route_manager_get (), self); nm_default_route_manager_ip6_update_default_route (nm_default_route_manager_get (), self); @@ -8875,9 +8877,10 @@ nm_device_init (NMDevice *self) priv->ip6_saved_properties = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, g_free); priv->default_route.v4_is_assumed = TRUE; - priv->default_route.v4_configure_first_time = TRUE; priv->default_route.v6_is_assumed = TRUE; - priv->default_route.v6_configure_first_time = TRUE; + + priv->v4_commit_first_time = TRUE; + priv->v6_commit_first_time = TRUE; } static GObject* From 351a645ad63a9f84bef0fa8abaa0340dc1acbf21 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Jul 2015 11:56:05 +0200 Subject: [PATCH 2/9] device: move setting v4_commit_first_time/v6_commit_first_time to the end of merge_and_apply() (cherry picked from commit cbd246c9b04868ada07b4853753fdba26bca54f3) --- src/devices/nm-device.c | 23 +++++++++++++---------- 1 file changed, 13 insertions(+), 10 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 867f3846f9..5eeccbe61c 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3302,10 +3302,6 @@ ip4_config_merge_and_apply (NMDevice *self, priv->default_route.v4_has = FALSE; priv->default_route.v4_is_assumed = TRUE; - routes_full_sync = commit - && priv->v4_commit_first_time - && !nm_device_uses_assumed_connection (self); - if (!commit) { /* during a non-commit event, we always pickup whatever is configured. */ goto END_ADD_DEFAULT_ROUTE; @@ -3331,7 +3327,6 @@ ip4_config_merge_and_apply (NMDevice *self, /* we are about to commit (for a non-assumed connection). Enforce whatever we have * configured. */ - priv->v4_commit_first_time = FALSE; priv->default_route.v4_is_assumed = FALSE; if (!connection_has_default_route) @@ -3385,8 +3380,15 @@ END_ADD_DEFAULT_ROUTE: NM_DEVICE_GET_CLASS (self)->ip4_config_pre_commit (self, composite); } + routes_full_sync = commit + && priv->v4_commit_first_time + && !nm_device_uses_assumed_connection (self); + success = nm_device_set_ip4_config (self, composite, default_route_metric, commit, routes_full_sync, out_reason); g_object_unref (composite); + + if (commit) + priv->v4_commit_first_time = FALSE; return success; } @@ -3920,10 +3922,6 @@ ip6_config_merge_and_apply (NMDevice *self, priv->default_route.v6_has = FALSE; priv->default_route.v6_is_assumed = TRUE; - routes_full_sync = commit - && priv->v6_commit_first_time - && !nm_device_uses_assumed_connection (self); - if (!commit) { /* during a non-commit event, we always pickup whatever is configured. */ goto END_ADD_DEFAULT_ROUTE; @@ -3949,7 +3947,6 @@ ip6_config_merge_and_apply (NMDevice *self, /* we are about to commit (for a non-assumed connection). Enforce whatever we have * configured. */ - priv->v6_commit_first_time = FALSE; priv->default_route.v6_is_assumed = FALSE; if (!connection_has_default_route) @@ -4006,8 +4003,14 @@ END_ADD_DEFAULT_ROUTE: NM_DEVICE_GET_CLASS (self)->ip6_config_pre_commit (self, composite); } + routes_full_sync = commit + && priv->v6_commit_first_time + && !nm_device_uses_assumed_connection (self); + success = nm_device_set_ip6_config (self, composite, commit, routes_full_sync, out_reason); g_object_unref (composite); + if (commit) + priv->v6_commit_first_time = FALSE; return success; } From 0ab389e15cde106e0e66f25f5f3a74a8c0607f39 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Jul 2015 10:56:53 +0200 Subject: [PATCH 3/9] device: always assume default-route for generate-assumed-connections Commit d51975e changed, that we treat assumed and non-assumed connections the same with respect to the default route. This is certainly wrong, if we have an nm-generated-assumed connection at hand. In this case, NM just generated a connection based on what was configured on the system. Looking at that result and re-enforcing the default-route is wrong. We want to manage the default-route for assumed, persistent connections. If the connection was assumed and generated, we do not. This commit reverts d51975ed for nm-generated-assumed connection and restores the previous behavior. https://bugzilla.redhat.com/show_bug.cgi?id=1244483 Fixes: d51975ed921a5876b76e081b8f3df4e2ca1f1ca9 (cherry picked from commit bebeff69e89de04fdd53e21a0edb5d1fbfbfcf0b) --- src/devices/nm-device.c | 48 ++++++++++++++++++++++++----------------- 1 file changed, 28 insertions(+), 20 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 5eeccbe61c..a0aaf03158 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3307,16 +3307,8 @@ ip4_config_merge_and_apply (NMDevice *self, goto END_ADD_DEFAULT_ROUTE; } - connection_has_default_route - = nm_default_route_manager_ip4_connection_has_default_route (nm_default_route_manager_get (), - connection, &connection_is_never_default); - - if ( !priv->v4_commit_first_time - && !nm_device_uses_assumed_connection (self) - && connection_is_never_default) { - /* If the connection is explicitly configured as never-default, we enforce the (absense of the) - * default-route only once. That allows the user to configure a connection as never-default, - * but he can add default routes externally (via a dispatcher script) and NM will not interfere. */ + if (nm_device_uses_generated_assumed_connection (self)) { + /* a generate-assumed-connection always detects the default route from platform */ goto END_ADD_DEFAULT_ROUTE; } @@ -3325,6 +3317,18 @@ ip4_config_merge_and_apply (NMDevice *self, * leases for them, so we must extend/update the default route on commits. */ + connection_has_default_route + = nm_default_route_manager_ip4_connection_has_default_route (nm_default_route_manager_get (), + connection, &connection_is_never_default); + + if ( !priv->v4_commit_first_time + && connection_is_never_default) { + /* If the connection is explicitly configured as never-default, we enforce the (absense of the) + * default-route only once. That allows the user to configure a connection as never-default, + * but he can add default routes externally (via a dispatcher script) and NM will not interfere. */ + goto END_ADD_DEFAULT_ROUTE; + } + /* we are about to commit (for a non-assumed connection). Enforce whatever we have * configured. */ priv->default_route.v4_is_assumed = FALSE; @@ -3927,16 +3931,8 @@ ip6_config_merge_and_apply (NMDevice *self, goto END_ADD_DEFAULT_ROUTE; } - connection_has_default_route - = nm_default_route_manager_ip6_connection_has_default_route (nm_default_route_manager_get (), - connection, &connection_is_never_default); - - if ( !priv->v6_commit_first_time - && !nm_device_uses_assumed_connection (self) - && connection_is_never_default) { - /* If the connection is explicitly configured as never-default, we enforce the (absence of the) - * default-route only once. That allows the user to configure a connection as never-default, - * but he can add default routes externally (via a dispatcher script) and NM will not interfere. */ + if (nm_device_uses_generated_assumed_connection (self)) { + /* a generate-assumed-connection always detects the default route from platform */ goto END_ADD_DEFAULT_ROUTE; } @@ -3945,6 +3941,18 @@ ip6_config_merge_and_apply (NMDevice *self, * leases for them, so we must extend/update the default route on commits. */ + connection_has_default_route + = nm_default_route_manager_ip6_connection_has_default_route (nm_default_route_manager_get (), + connection, &connection_is_never_default); + + if ( !priv->v6_commit_first_time + && connection_is_never_default) { + /* If the connection is explicitly configured as never-default, we enforce the (absence of the) + * default-route only once. That allows the user to configure a connection as never-default, + * but he can add default routes externally (via a dispatcher script) and NM will not interfere. */ + goto END_ADD_DEFAULT_ROUTE; + } + /* we are about to commit (for a non-assumed connection). Enforce whatever we have * configured. */ priv->default_route.v6_is_assumed = FALSE; From 5e9dd4b267b7dacc96da6beca1df0ec9781c362b Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Wed, 22 Jul 2015 10:33:49 +0200 Subject: [PATCH 4/9] default-route-manager: pick up platform changes after NMDevice If a default route is configured externally, we want the device to pick the change and register it with the default-route-manager first. https://bugzilla.redhat.com/show_bug.cgi?id=1244483 (cherry picked from commit e67b52ed16afebce538f2f1844cf6e854e27c64d) --- src/nm-default-route-manager.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/nm-default-route-manager.c b/src/nm-default-route-manager.c index fbb07ce3fc..b36dd44f3a 100644 --- a/src/nm-default-route-manager.c +++ b/src/nm-default-route-manager.c @@ -1290,7 +1290,9 @@ _resync_idle_reschedule (NMDefaultRouteManager *self) g_source_remove (priv->resync.idle_handle); else _LOGD (0, "resync: schedule on idle"); - priv->resync.idle_handle = g_idle_add ((GSourceFunc) _resync_idle_now, self); + /* Schedule this at low priority so that on an external change to platform + * a NMDevice has a chance to picks up the changes first. */ + priv->resync.idle_handle = g_idle_add_full (G_PRIORITY_LOW, (GSourceFunc) _resync_idle_now, self, NULL); } else if (!priv->resync.idle_handle) { priv->resync.idle_handle = g_timeout_add (priv->resync.backoff_wait_time_ms, (GSourceFunc) _resync_idle_now, self); _LOGD (0, "resync: schedule in %u.%03u seconds (%u)", priv->resync.backoff_wait_time_ms/1000, From f11e4c31ee9014304f05b4ccfc6e2b2c934e9ee4 Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 21 Jul 2015 23:07:34 +0200 Subject: [PATCH 5/9] ip4-config: 0.0.0.0 is a valid gateway too It makes sense for point-to point links. https://bugzilla.redhat.com/show_bug.cgi?id=1244483 (cherry picked from commit 063677101ab7d43a9aa94c70eb1ca3a201269043) --- src/devices/nm-device.c | 2 +- src/nm-ip4-config.c | 67 ++++++++++++++----- src/nm-ip4-config.h | 2 + .../plugins/ifcfg-rh/tests/test-ifcfg-rh.c | 2 +- 4 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index a0aaf03158..9f8c488a8f 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -3342,7 +3342,7 @@ ip4_config_merge_and_apply (NMDevice *self, } gateway = nm_ip4_config_get_gateway (composite); - if ( !gateway + if ( !nm_ip4_config_has_gateway (composite) && nm_device_get_device_type (self) != NM_DEVICE_TYPE_MODEM) goto END_ADD_DEFAULT_ROUTE; diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index 1b48517347..035849ee62 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -45,6 +45,7 @@ typedef struct { gboolean never_default; guint32 gateway; + gboolean has_gateway; GArray *addresses; GArray *routes; GArray *nameservers; @@ -187,7 +188,7 @@ nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf) guint i; guint32 lowest_metric = G_MAXUINT32; guint32 old_gateway = 0; - gboolean has_gateway = FALSE; + gboolean old_has_gateway = FALSE; /* Slaves have no IP configuration */ if (nm_platform_link_get_master (NM_PLATFORM_GET, ifindex) > 0) @@ -204,6 +205,7 @@ nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf) /* Extract gateway from default route */ old_gateway = priv->gateway; + old_has_gateway = priv->has_gateway; for (i = 0; i < priv->routes->len; ) { const NMPlatformIP4Route *route = &g_array_index (priv->routes, NMPlatformIP4Route, i); @@ -212,7 +214,7 @@ nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf) priv->gateway = route->gateway; lowest_metric = route->metric; } - has_gateway = TRUE; + priv->has_gateway = TRUE; /* Remove the default route from the list */ g_array_remove_index_fast (priv->routes, i); continue; @@ -222,12 +224,12 @@ nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf) /* we detect the route metric based on the default route. All non-default * routes have their route metrics explicitly set. */ - priv->route_metric = has_gateway ? (gint64) lowest_metric : (gint64) -1; + priv->route_metric = priv->has_gateway ? (gint64) lowest_metric : (gint64) -1; /* If there is a host route to the gateway, ignore that route. It is * automatically added by NetworkManager when needed. */ - if (has_gateway) { + if (priv->has_gateway) { for (i = 0; i < priv->routes->len; i++) { const NMPlatformIP4Route *route = &g_array_index (priv->routes, NMPlatformIP4Route, i); @@ -243,7 +245,7 @@ nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf) /* If the interface has the default route, and has IPv4 addresses, capture * nameservers from /etc/resolv.conf. */ - if (priv->addresses->len && has_gateway && capture_resolv_conf) { + if (priv->addresses->len && priv->has_gateway && capture_resolv_conf) { if (nm_ip4_config_capture_resolv_conf (priv->nameservers, NULL)) _NOTIFY (config, PROP_NAMESERVERS); } @@ -253,7 +255,8 @@ nm_ip4_config_capture (int ifindex, gboolean capture_resolv_conf) _NOTIFY (config, PROP_ROUTE_DATA); _NOTIFY (config, PROP_ADDRESSES); _NOTIFY (config, PROP_ROUTES); - if (priv->gateway != old_gateway) + if ( priv->gateway != old_gateway + || priv->has_gateway != old_has_gateway) _NOTIFY (config, PROP_GATEWAY); return config; @@ -484,7 +487,7 @@ nm_ip4_config_create_setting (const NMIP4Config *config) } /* Gateway */ - if ( gateway + if ( nm_ip4_config_has_gateway (config) && nm_setting_ip_config_get_num_addresses (s_ip4) > 0) { g_object_set (s_ip4, NM_SETTING_IP_CONFIG_GATEWAY, nm_utils_inet4_ntop (gateway, NULL), @@ -561,7 +564,7 @@ nm_ip4_config_merge (NMIP4Config *dst, const NMIP4Config *src) nm_ip4_config_add_nameserver (dst, nm_ip4_config_get_nameserver (src, i)); /* default gateway */ - if (nm_ip4_config_get_gateway (src)) + if (nm_ip4_config_has_gateway (src)) nm_ip4_config_set_gateway (dst, nm_ip4_config_get_gateway (src)); /* routes */ @@ -752,11 +755,12 @@ nm_ip4_config_subtract (NMIP4Config *dst, const NMIP4Config *src) } /* default gateway */ - if (nm_ip4_config_get_gateway (src) == nm_ip4_config_get_gateway (dst)) - nm_ip4_config_set_gateway (dst, 0); + if ( (nm_ip4_config_has_gateway (src) == nm_ip4_config_has_gateway (dst)) + && (nm_ip4_config_get_gateway (src) == nm_ip4_config_get_gateway (dst))) + nm_ip4_config_unset_gateway (dst); if (!nm_ip4_config_get_num_addresses (dst)) - nm_ip4_config_set_gateway (dst, 0); + nm_ip4_config_unset_gateway (dst); /* ignore route_metric */ @@ -834,8 +838,10 @@ nm_ip4_config_intersect (NMIP4Config *dst, const NMIP4Config *src) /* default gateway */ if ( !nm_ip4_config_get_num_addresses (dst) - || (nm_ip4_config_get_gateway (src) != nm_ip4_config_get_gateway (dst))) - nm_ip4_config_set_gateway (dst, 0); + || (nm_ip4_config_has_gateway (src) != nm_ip4_config_has_gateway (dst)) + || (nm_ip4_config_get_gateway (src) != nm_ip4_config_get_gateway (dst))) { + nm_ip4_config_unset_gateway (dst); + } /* routes */ for (i = 0; i < nm_ip4_config_get_num_routes (dst); ) { @@ -901,7 +907,8 @@ nm_ip4_config_replace (NMIP4Config *dst, const NMIP4Config *src, gboolean *relev } /* default gateway */ - if (src_priv->gateway != dst_priv->gateway) { + if ( src_priv->gateway != dst_priv->gateway + || src_priv->has_gateway != dst_priv->has_gateway) { nm_ip4_config_set_gateway (dst, src_priv->gateway); has_relevant_changes = TRUE; } @@ -1104,8 +1111,10 @@ nm_ip4_config_dump (const NMIP4Config *config, const char *detail) g_message (" a: %s", nm_platform_ip4_address_to_string (nm_ip4_config_get_address (config, i))); /* default gateway */ - tmp = nm_ip4_config_get_gateway (config); - g_message (" gw: %s", nm_utils_inet4_ntop (tmp, NULL)); + if (nm_ip4_config_has_gateway (config)) { + tmp = nm_ip4_config_get_gateway (config); + g_message (" gw: %s", nm_utils_inet4_ntop (tmp, NULL)); + } /* nameservers */ for (i = 0; i < nm_ip4_config_get_num_nameservers (config); i++) { @@ -1185,12 +1194,33 @@ nm_ip4_config_set_gateway (NMIP4Config *config, guint32 gateway) { NMIP4ConfigPrivate *priv = NM_IP4_CONFIG_GET_PRIVATE (config); - if (priv->gateway != gateway) { + if (priv->gateway != gateway || !priv->has_gateway) { priv->gateway = gateway; + priv->has_gateway = TRUE; _NOTIFY (config, PROP_GATEWAY); } } +void +nm_ip4_config_unset_gateway (NMIP4Config *config) +{ + NMIP4ConfigPrivate *priv = NM_IP4_CONFIG_GET_PRIVATE (config); + + if (priv->has_gateway) { + priv->gateway = 0; + priv->has_gateway = FALSE; + _NOTIFY (config, PROP_GATEWAY); + } +} + +gboolean +nm_ip4_config_has_gateway (const NMIP4Config *config) +{ + NMIP4ConfigPrivate *priv = NM_IP4_CONFIG_GET_PRIVATE (config); + + return priv->has_gateway; +} + guint32 nm_ip4_config_get_gateway (const NMIP4Config *config) { @@ -1832,6 +1862,7 @@ nm_ip4_config_hash (const NMIP4Config *config, GChecksum *sum, gboolean dns_only g_return_if_fail (sum); if (!dns_only) { + hash_u32 (sum, nm_ip4_config_has_gateway (config)); hash_u32 (sum, nm_ip4_config_get_gateway (config)); for (i = 0; i < nm_ip4_config_get_num_addresses (config); i++) { @@ -2093,7 +2124,7 @@ get_property (GObject *object, guint prop_id, } break; case PROP_GATEWAY: - if (priv->gateway) + if (priv->has_gateway) g_value_set_string (value, nm_utils_inet4_ntop (priv->gateway, NULL)); else g_value_set_string (value, NULL); diff --git a/src/nm-ip4-config.h b/src/nm-ip4-config.h index fb072404f0..2bb766ab5a 100644 --- a/src/nm-ip4-config.h +++ b/src/nm-ip4-config.h @@ -80,6 +80,8 @@ void nm_ip4_config_dump (const NMIP4Config *config, const char *detail); void nm_ip4_config_set_never_default (NMIP4Config *config, gboolean never_default); gboolean nm_ip4_config_get_never_default (const NMIP4Config *config); void nm_ip4_config_set_gateway (NMIP4Config *config, guint32 gateway); +void nm_ip4_config_unset_gateway (NMIP4Config *config); +gboolean nm_ip4_config_has_gateway (const NMIP4Config *config); guint32 nm_ip4_config_get_gateway (const NMIP4Config *config); gint64 nm_ip4_config_get_route_metric (const NMIP4Config *config); diff --git a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c index d2fb6867c4..0edbe9b209 100644 --- a/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c +++ b/src/settings/plugins/ifcfg-rh/tests/test-ifcfg-rh.c @@ -6405,7 +6405,7 @@ test_write_wired_static_ip6_only_gw (gconstpointer user_data) g_assert (addr6); /* assert that the gateway was written and reloaded as expected */ - if (!gateway6 || !strcmp (gateway6, "::")) { + if (!gateway6) { g_assert (nm_setting_ip_config_get_gateway (s_ip6) == NULL); g_assert (written_ifcfg_gateway == NULL); } else { From fa2b8fdbdfabaee292cf63adcc53ae026e6afbe3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Jul 2015 13:06:29 +0200 Subject: [PATCH 6/9] libnm-core: properly handle %NULL @ip in nm_utils_ipaddr_valid() A is_valid() function should just accept NULL as input and return "invalid". It certainly should not crash. Fixes: 21c8a6b20effbe1e689505a0cbb23594be06068c (cherry picked from commit 2b55de856027657e567914361f501bbfbca050b4) --- libnm-core/nm-utils.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 6a7ed61f5c..3fb8676db1 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -3353,6 +3353,9 @@ nm_utils_ipaddr_valid (int family, const char *ip) g_return_val_if_fail (family == AF_INET || family == AF_INET6 || family == AF_UNSPEC, FALSE); + if (!ip) + return FALSE; + if (family == AF_UNSPEC) family = strchr (ip, ':') ? AF_INET6 : AF_INET; From 89b630ff32f9111b3ed518c413b576adbf746e78 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Jul 2015 13:05:31 +0200 Subject: [PATCH 7/9] libnm-core: fix nm-setting-ip-config.c:valid_ip() to handle %NULL argument We call valid_ip() from nm_ip_route_new() to check whether an untrusted string is a valid ip address. Properly handle %NULL argument. Fixes: 21c8a6b20effbe1e689505a0cbb23594be06068c (cherry picked from commit 93425686947127e5bfc2d00521b35c4b75340ee5) --- libnm-core/nm-setting-ip-config.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c index e6f0401a01..2e8d0c8ea4 100644 --- a/libnm-core/nm-setting-ip-config.c +++ b/libnm-core/nm-setting-ip-config.c @@ -74,6 +74,11 @@ canonicalize_ip (int family, const char *ip, gboolean null_any) static gboolean valid_ip (int family, const char *ip, GError **error) { + if (!ip) { + g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_FAILED, + family == AF_INET ? _("Missing IPv4 address") : _("Missing IPv6 address'")); + return FALSE; + } if (!nm_utils_ipaddr_valid (family, ip)) { g_set_error (error, NM_CONNECTION_ERROR, NM_CONNECTION_ERROR_FAILED, family == AF_INET ? _("Invalid IPv4 address '%s'") : _("Invalid IPv6 address '%s"), From 2958b3d1afc6c23757893837b0b4353df889794d Mon Sep 17 00:00:00 2001 From: Lubomir Rintel Date: Tue, 21 Jul 2015 23:07:34 +0200 Subject: [PATCH 8/9] libnm-core: 0.0.0.0 is a valid gateway too It makes sense for point-to point links. https://bugzilla.redhat.com/show_bug.cgi?id=1244483 (cherry picked from commit f14fd048ff84794f72892a1fe0209d56552b0e6e) --- libnm-core/nm-setting-ip-config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c index 2e8d0c8ea4..d637b5ea2f 100644 --- a/libnm-core/nm-setting-ip-config.c +++ b/libnm-core/nm-setting-ip-config.c @@ -2017,7 +2017,7 @@ set_property (GObject *object, guint prop_id, gateway = g_value_get_string (value); g_return_if_fail (!gateway || nm_utils_ipaddr_valid (NM_SETTING_IP_CONFIG_GET_FAMILY (setting), gateway)); g_free (priv->gateway); - priv->gateway = canonicalize_ip (NM_SETTING_IP_CONFIG_GET_FAMILY (setting), gateway, TRUE); + priv->gateway = canonicalize_ip (NM_SETTING_IP_CONFIG_GET_FAMILY (setting), gateway, gateway == NULL); break; case PROP_ROUTES: g_ptr_array_unref (priv->routes); From 9cfdbf6a0655ce93c1aaf25ed8c571636bc4236e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 22 Jul 2015 13:37:45 +0200 Subject: [PATCH 9/9] libnm-core: don't assert against non-NULL @ip argument to canonicalize_ip() Remove an assertion in canonicalize_ip() to assert that either a non-NULL @ip is given, or @null_any is TRUE. The condition of the assert is not easy to understand without context. Instead the caller should already handle %NULL properly. All callers that pass @null_any=FALSE to canonicalize_ip(), already assert that the argument is not %NULL. With the exception of nm_ip_route_new() which however checks for a valid @dest early on. (cherry picked from commit 7f129b976cf175ef7d3d75227761d14afad69dd3) --- libnm-core/nm-setting-ip-config.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/libnm-core/nm-setting-ip-config.c b/libnm-core/nm-setting-ip-config.c index d637b5ea2f..4b73d5fd0e 100644 --- a/libnm-core/nm-setting-ip-config.c +++ b/libnm-core/nm-setting-ip-config.c @@ -53,10 +53,8 @@ canonicalize_ip (int family, const char *ip, gboolean null_any) char addr_str[NM_UTILS_INET_ADDRSTRLEN]; int ret; - if (!ip) { - g_return_val_if_fail (null_any == TRUE, NULL); + if (!ip) return NULL; - } ret = inet_pton (family, ip, addr_bytes); g_return_val_if_fail (ret == 1, NULL); @@ -2017,7 +2015,7 @@ set_property (GObject *object, guint prop_id, gateway = g_value_get_string (value); g_return_if_fail (!gateway || nm_utils_ipaddr_valid (NM_SETTING_IP_CONFIG_GET_FAMILY (setting), gateway)); g_free (priv->gateway); - priv->gateway = canonicalize_ip (NM_SETTING_IP_CONFIG_GET_FAMILY (setting), gateway, gateway == NULL); + priv->gateway = canonicalize_ip (NM_SETTING_IP_CONFIG_GET_FAMILY (setting), gateway, FALSE); break; case PROP_ROUTES: g_ptr_array_unref (priv->routes);