From e442e3881efad8b0b484763f87544facdc40cd7b Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 14 Dec 2018 16:45:53 +0100 Subject: [PATCH 01/16] core: implement nm_utils_ip4_netmask_to_prefix() via __builtin_ctz() Taken from systemd's in4_addr_netmask_to_prefixlen(). Yes, this adds the requirement that "int" is 32 bits. But systemd already has the same requirement in u32ctz(), hence we anyway cannot build on other architectures. If that is ever necessary, it's easy to adjust. --- libnm-core/nm-utils.c | 29 +++++------------------ libnm-core/tests/test-general.c | 41 ++++++++++++++++++++++++++++++--- 2 files changed, 44 insertions(+), 26 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 547c141850..b9d4b3d543 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1389,30 +1389,13 @@ nm_utils_ip4_routes_from_variant (GVariant *value) guint32 nm_utils_ip4_netmask_to_prefix (guint32 netmask) { - guint32 prefix; - guint8 v; - const guint8 *p = (guint8 *) &netmask; + G_STATIC_ASSERT_EXPR (__SIZEOF_INT__ == 4); + G_STATIC_ASSERT_EXPR (sizeof (int) == 4); + G_STATIC_ASSERT_EXPR (sizeof (netmask) == 4); - if (p[3]) { - prefix = 24; - v = p[3]; - } else if (p[2]) { - prefix = 16; - v = p[2]; - } else if (p[1]) { - prefix = 8; - v = p[1]; - } else { - prefix = 0; - v = p[0]; - } - - while (v) { - prefix++; - v <<= 1; - } - - return prefix; + return ( (netmask != 0) + ? (32 - __builtin_ctz (ntohl (netmask))) + : 0); } /** diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 3ea7bbffa3..03c923bef1 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -3468,6 +3468,41 @@ test_connection_changed_cb (NMConnection *connection, gboolean *data) *data = TRUE; } +static guint32 +_netmask_to_prefix (guint32 netmask) +{ + guint32 prefix; + guint8 v; + const guint8 *p = (guint8 *) &netmask; + + if (p[3]) { + prefix = 24; + v = p[3]; + } else if (p[2]) { + prefix = 16; + v = p[2]; + } else if (p[1]) { + prefix = 8; + v = p[1]; + } else { + prefix = 0; + v = p[0]; + } + + while (v) { + prefix++; + v <<= 1; + } + + g_assert_cmpint (prefix, <=, 32); + + /* we re-implemented the netmask-to-prefix code differently. Check + * that they agree. */ + g_assert_cmpint (prefix, ==, nm_utils_ip4_netmask_to_prefix (netmask)); + + return prefix; +} + static void test_ip4_prefix_to_netmask (void) { @@ -3475,7 +3510,7 @@ test_ip4_prefix_to_netmask (void) for (i = 0; i<=32; i++) { guint32 netmask = _nm_utils_ip4_prefix_to_netmask (i); - int plen = nm_utils_ip4_netmask_to_prefix (netmask); + int plen = _netmask_to_prefix (netmask); g_assert_cmpint (i, ==, plen); { @@ -3505,7 +3540,7 @@ test_ip4_netmask_to_prefix (void) guint32 netmask = _nm_utils_ip4_prefix_to_netmask (i); guint32 netmask_lowest_bit = netmask & ~_nm_utils_ip4_prefix_to_netmask (i-1); - g_assert_cmpint (i, ==, nm_utils_ip4_netmask_to_prefix (netmask)); + g_assert_cmpint (i, ==, _netmask_to_prefix (netmask)); for (j = 0; j < 2*i; j++) { guint32 r = g_rand_int (rand); @@ -3519,7 +3554,7 @@ test_ip4_netmask_to_prefix (void) /* create an invalid netmask with holes and check that the function * returns the longest prefix. */ - prefix_holey = nm_utils_ip4_netmask_to_prefix (netmask_holey); + prefix_holey = _netmask_to_prefix (netmask_holey); g_assert_cmpint (i, ==, prefix_holey); } From 1e4c0dd6805c75694e86342fe6a0c01bae5198fc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 27 Nov 2018 10:20:25 +0100 Subject: [PATCH 02/16] shared/tests: add helper functions to convert IP address to string We have nm_utils_inet*_ntop(), however: - that is partly private API libnm-core, and thus only available in components that have access to that. Partly it's public API of libnm, but still only available in components that use libnm. - relying on the static buffers is discouraged for nm_utils_inet*_ntop(). For testing, that is fine as we are in a more controlled envionment. So, add a test variant that explicitly relies on static buffers. That way, it's more convenient to use from tests. - these functions can assert more and are more convenient to use from tests. --- shared/nm-utils/nm-test-utils.h | 42 +++++++++++++++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/shared/nm-utils/nm-test-utils.h b/shared/nm-utils/nm-test-utils.h index 18b42888e8..64a024ca4e 100644 --- a/shared/nm-utils/nm-test-utils.h +++ b/shared/nm-utils/nm-test-utils.h @@ -1166,6 +1166,48 @@ nmtst_inet6_from_string (const char *str) return &addr; } +static inline gconstpointer +nmtst_inet_from_string (int addr_family, const char *str) +{ + if (addr_family == AF_INET) { + static in_addr_t a; + + a = nmtst_inet4_from_string (str); + return &a; + } + if (addr_family == AF_INET6) + return nmtst_inet6_from_string (str); + + g_assert_not_reached (); + return NULL; +} + +static inline const char * +nmtst_inet_to_string (int addr_family, gconstpointer addr) +{ + static char buf[MAX (INET6_ADDRSTRLEN, INET_ADDRSTRLEN)]; + + g_assert (NM_IN_SET (addr_family, AF_INET, AF_INET6)); + g_assert (addr); + + if (inet_ntop (addr_family, addr, buf, sizeof (buf)) != buf) + g_assert_not_reached (); + + return buf; +} + +static inline const char * +nmtst_inet4_to_string (in_addr_t addr) +{ + return nmtst_inet_to_string (AF_INET, &addr); +} + +static inline const char * +nmtst_inet6_to_string (const struct in6_addr *addr) +{ + return nmtst_inet_to_string (AF_INET6, addr); +} + static inline void _nmtst_assert_ip4_address (const char *file, int line, in_addr_t addr, const char *str_expected) { From 898f7a56655de1edf0bcb8036d3a8ca71b19c536 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 27 Nov 2018 10:24:03 +0100 Subject: [PATCH 03/16] libnm: add internal API nm_utils_inet*_ntop_dup() In quite some cases we need the string representation on the heap. While nm_utils_inet*_ntop() accepts NULL as output buffer to fallback to a static buffer, such usage of a static buffer is discouraged. So, we actually should always allocate a temporaray buffer on the stack. But that is cumbersome to write. Add simple wrappers that makes calling this more convenient. --- libnm-core/nm-core-internal.h | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/libnm-core/nm-core-internal.h b/libnm-core/nm-core-internal.h index 6c85cc6be4..ff2444673f 100644 --- a/libnm-core/nm-core-internal.h +++ b/libnm-core/nm-core-internal.h @@ -496,6 +496,30 @@ NMSettingBluetooth *_nm_connection_get_setting_bluetooth_for_nap (NMConnection * const char *nm_utils_inet_ntop (int addr_family, gconstpointer addr, char *dst); +static inline char * +nm_utils_inet4_ntop_dup (in_addr_t addr) +{ + char buf[NM_UTILS_INET_ADDRSTRLEN]; + + return g_strdup (nm_utils_inet4_ntop (addr, buf)); +} + +static inline char * +nm_utils_inet6_ntop_dup (const struct in6_addr *addr) +{ + char buf[NM_UTILS_INET_ADDRSTRLEN]; + + return g_strdup (nm_utils_inet6_ntop (addr, buf)); +} + +static inline char * +nm_utils_inet_ntop_dup (int addr_family, const struct in6_addr *addr) +{ + char buf[NM_UTILS_INET_ADDRSTRLEN]; + + return g_strdup (nm_utils_inet_ntop (addr_family, addr, buf)); +} + gboolean _nm_utils_inet6_is_token (const struct in6_addr *in6addr); /*****************************************************************************/ From a51c09dc12e2f1ba53a2239a0239fe98f66d628e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 26 Nov 2018 16:49:51 +0100 Subject: [PATCH 04/16] all: don't use static buffer for nm_utils_inet*_ntop() While nm_utils_inet*_ntop() accepts a %NULL buffer to fallback to a static buffer, don't do that. I find the possibility of using a static buffer here error prone and something that should be avoided. There is of course the downside, that in some cases it requires an additional line of code to allocate the buffer on the stack as auto-variable. --- libnm-core/nm-utils.c | 8 +-- libnm-core/tests/test-general.c | 6 +- src/devices/nm-acd-manager.c | 11 +-- src/devices/nm-device-ip-tunnel.c | 8 +-- src/devices/nm-device-vxlan.c | 18 ++--- src/devices/nm-device.c | 20 ++++-- src/devices/tests/test-acd.c | 3 +- src/dhcp/nm-dhcp-systemd.c | 37 +++++----- src/dhcp/nm-dhcp-utils.c | 11 +-- src/ndisc/nm-ndisc.c | 7 +- src/nm-core-utils.c | 8 +-- src/nm-ip4-config.c | 26 +++---- src/nm-ip6-config.c | 19 +++--- src/nm-pacrunner-manager.c | 10 +-- src/nm-policy.c | 11 +-- src/platform/nm-fake-platform.c | 6 +- src/platform/nm-linux-platform.c | 3 +- src/platform/nm-platform.c | 41 ++++++++---- src/platform/nmp-object.c | 6 +- src/platform/tests/test-common.c | 67 ++++++++++++------- .../plugins/ifcfg-rh/nms-ifcfg-rh-reader.c | 5 +- src/vpn/nm-vpn-connection.c | 31 ++++----- 22 files changed, 211 insertions(+), 151 deletions(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index b9d4b3d543..740892c85a 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -1173,7 +1173,7 @@ nm_utils_ip4_dns_from_variant (GVariant *value) dns = g_new (char *, length + 1); for (i = 0; i < length; i++) - dns[i] = g_strdup (nm_utils_inet4_ntop (array[i], NULL)); + dns[i] = nm_utils_inet4_ntop_dup (array[i]); dns[i] = NULL; return dns; @@ -1271,7 +1271,7 @@ nm_utils_ip4_addresses_from_variant (GVariant *value, char **out_gateway) g_ptr_array_add (addresses, addr); if (addr_array[2] && out_gateway && !*out_gateway) - *out_gateway = g_strdup (nm_utils_inet4_ntop (addr_array[2], NULL)); + *out_gateway = nm_utils_inet4_ntop_dup (addr_array[2]); } else { g_warning ("Ignoring invalid IP4 address: %s", error->message); g_clear_error (&error); @@ -1493,7 +1493,7 @@ nm_utils_ip6_dns_from_variant (GVariant *value) continue; } - dns[i++] = g_strdup (nm_utils_inet6_ntop (ip, NULL)); + dns[i++] = nm_utils_inet6_ntop_dup (ip); g_variant_unref (ip_var); } dns[i] = NULL; @@ -1612,7 +1612,7 @@ nm_utils_ip6_addresses_from_variant (GVariant *value, char **out_gateway) goto next; } if (!IN6_IS_ADDR_UNSPECIFIED (gateway_bytes)) - *out_gateway = g_strdup (nm_utils_inet6_ntop (gateway_bytes, NULL)); + *out_gateway = nm_utils_inet6_ntop_dup (gateway_bytes); } } else { g_warning ("Ignoring invalid IP6 address: %s", error->message); diff --git a/libnm-core/tests/test-general.c b/libnm-core/tests/test-general.c index 03c923bef1..91f116de43 100644 --- a/libnm-core/tests/test-general.c +++ b/libnm-core/tests/test-general.c @@ -5084,7 +5084,7 @@ test_setting_ip4_gateway (void) GVariantBuilder addrs_builder; GError *error = NULL; - g_assert_cmpstr (nm_utils_inet4_ntop (addr_vals_0[0], NULL), ==, "192.168.1.10"); + nmtst_assert_ip4_address (addr_vals_0[0], "192.168.1.10"); /* When serializing on the daemon side, ipv4.gateway is copied to the first * entry of ipv4.addresses @@ -5126,7 +5126,7 @@ test_setting_ip4_gateway (void) addr_array = g_variant_get_fixed_array (addr_var, &length, sizeof (guint32)); g_assert_cmpint (length, ==, 3); - g_assert_cmpstr (nm_utils_inet4_ntop (addr_array[2], NULL), ==, "192.168.1.1"); + nmtst_assert_ip4_address (addr_array[2], "192.168.1.1"); g_variant_unref (addr_var); } g_variant_unref (value); @@ -5233,7 +5233,7 @@ test_setting_ip6_gateway (void) gateway_bytes = g_variant_get_fixed_array (gateway_var, &length, 1); g_assert_cmpint (length, ==, 16); - g_assert_cmpstr (nm_utils_inet6_ntop ((struct in6_addr *) gateway_bytes, NULL), ==, "abcd::1"); + nmtst_assert_ip6_address ((struct in6_addr *) gateway_bytes, "abcd::1"); g_variant_unref (gateway_var); } g_variant_unref (value); diff --git a/src/devices/nm-acd-manager.c b/src/devices/nm-acd-manager.c index 7be0374baa..3c6128f060 100644 --- a/src/devices/nm-acd-manager.c +++ b/src/devices/nm-acd-manager.c @@ -230,11 +230,12 @@ acd_probe_add (NMAcdManager *self, { NAcdProbeConfig *probe_config; int r; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; r = n_acd_probe_config_new (&probe_config); if (r) { _LOGW ("could not create probe config for %s on interface '%s': %s", - nm_utils_inet4_ntop (info->address, NULL), + nm_utils_inet4_ntop (info->address, sbuf), nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex), acd_error_to_string (r)); return FALSE; @@ -246,7 +247,7 @@ acd_probe_add (NMAcdManager *self, r = n_acd_probe (self->acd, &info->probe, probe_config); if (r) { _LOGW ("could not start probe for %s on interface '%s': %s", - nm_utils_inet4_ntop (info->address, NULL), + nm_utils_inet4_ntop (info->address, sbuf), nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex), acd_error_to_string (r)); n_acd_probe_config_free (probe_config); @@ -381,6 +382,8 @@ nm_acd_manager_announce_addresses (NMAcdManager *self) acd_probe_add (self, info, 0); self->state = STATE_ANNOUNCING; } else if (self->state == STATE_ANNOUNCING) { + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; + g_hash_table_iter_init (&iter, self->addresses); while (g_hash_table_iter_next (&iter, NULL, (gpointer *) &info)) { if (info->duplicate) @@ -388,11 +391,11 @@ nm_acd_manager_announce_addresses (NMAcdManager *self) r = n_acd_probe_announce (info->probe, N_ACD_DEFEND_ONCE); if (r) { _LOGW ("couldn't announce address %s on interface '%s': %s", - nm_utils_inet4_ntop (info->address, NULL), + nm_utils_inet4_ntop (info->address, sbuf), nm_platform_link_get_name (NM_PLATFORM_GET, self->ifindex), acd_error_to_string (r)); } else - _LOGD ("announcing address %s", nm_utils_inet4_ntop (info->address, NULL)); + _LOGD ("announcing address %s", nm_utils_inet4_ntop (info->address, sbuf)); } } } diff --git a/src/devices/nm-device-ip-tunnel.c b/src/devices/nm-device-ip-tunnel.c index 1c7e6d51e5..79d2720b4c 100644 --- a/src/devices/nm-device-ip-tunnel.c +++ b/src/devices/nm-device-ip-tunnel.c @@ -329,28 +329,28 @@ clear: if (!address_equal_pn (AF_INET, priv->local, &local4)) { g_clear_pointer (&priv->local, g_free); if (local4) - priv->local = g_strdup (nm_utils_inet4_ntop (local4, NULL)); + priv->local = nm_utils_inet4_ntop_dup (local4); _notify (self, PROP_LOCAL); } if (!address_equal_pn (AF_INET, priv->remote, &remote4)) { g_clear_pointer (&priv->remote, g_free); if (remote4) - priv->remote = g_strdup (nm_utils_inet4_ntop (remote4, NULL)); + priv->remote = nm_utils_inet4_ntop_dup (remote4); _notify (self, PROP_REMOTE); } } else { if (!address_equal_pn (AF_INET6, priv->local, &local6)) { g_clear_pointer (&priv->local, g_free); if (memcmp (&local6, &in6addr_any, sizeof (in6addr_any))) - priv->local = g_strdup (nm_utils_inet6_ntop (&local6, NULL)); + priv->local = nm_utils_inet6_ntop_dup (&local6); _notify (self, PROP_LOCAL); } if (!address_equal_pn (AF_INET6, priv->remote, &remote6)) { g_clear_pointer (&priv->remote, g_free); if (memcmp (&remote6, &in6addr_any, sizeof (in6addr_any))) - priv->remote = g_strdup (nm_utils_inet6_ntop (&remote6, NULL)); + priv->remote = nm_utils_inet6_ntop_dup (&remote6); _notify (self, PROP_REMOTE); } } diff --git a/src/devices/nm-device-vxlan.c b/src/devices/nm-device-vxlan.c index c34f4142b2..229fbda4c9 100644 --- a/src/devices/nm-device-vxlan.c +++ b/src/devices/nm-device-vxlan.c @@ -34,6 +34,7 @@ #include "settings/nm-settings.h" #include "nm-act-request.h" #include "nm-ip4-config.h" +#include "nm-core-internal.h" #include "nm-device-logging.h" _LOG_DECLARE_SELF(NMDeviceVxlan); @@ -386,6 +387,7 @@ update_connection (NMDevice *device, NMConnection *connection) { NMDeviceVxlanPrivate *priv = NM_DEVICE_VXLAN_GET_PRIVATE ((NMDeviceVxlan *) device); NMSettingVxlan *s_vxlan = nm_connection_get_setting_vxlan (connection); + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; if (!s_vxlan) { s_vxlan = (NMSettingVxlan *) nm_setting_vxlan_new (); @@ -404,11 +406,11 @@ update_connection (NMDevice *device, NMConnection *connection) if (!address_matches (nm_setting_vxlan_get_remote (s_vxlan), priv->props.group, &priv->props.group6)) { if (priv->props.group) { g_object_set (s_vxlan, NM_SETTING_VXLAN_REMOTE, - nm_utils_inet4_ntop (priv->props.group, NULL), + nm_utils_inet4_ntop (priv->props.group, sbuf), NULL); } else { g_object_set (s_vxlan, NM_SETTING_VXLAN_REMOTE, - nm_utils_inet6_ntop (&priv->props.group6, NULL), + nm_utils_inet6_ntop (&priv->props.group6, sbuf), NULL); } } @@ -416,11 +418,11 @@ update_connection (NMDevice *device, NMConnection *connection) if (!address_matches (nm_setting_vxlan_get_local (s_vxlan), priv->props.local, &priv->props.local6)) { if (priv->props.local) { g_object_set (s_vxlan, NM_SETTING_VXLAN_LOCAL, - nm_utils_inet4_ntop (priv->props.local, NULL), + nm_utils_inet4_ntop (priv->props.local, sbuf), NULL); } else if (memcmp (&priv->props.local6, &in6addr_any, sizeof (in6addr_any))) { g_object_set (s_vxlan, NM_SETTING_VXLAN_LOCAL, - nm_utils_inet6_ntop (&priv->props.local6, NULL), + nm_utils_inet6_ntop (&priv->props.local6, sbuf), NULL); } } @@ -510,15 +512,15 @@ get_property (GObject *object, guint prop_id, break; case PROP_GROUP: if (priv->props.group) - g_value_set_string (value, nm_utils_inet4_ntop (priv->props.group, NULL)); + g_value_take_string (value, nm_utils_inet4_ntop_dup (priv->props.group)); else if (!IN6_IS_ADDR_UNSPECIFIED (&priv->props.group6)) - g_value_set_string (value, nm_utils_inet6_ntop (&priv->props.group6, NULL)); + g_value_take_string (value, nm_utils_inet6_ntop_dup (&priv->props.group6)); break; case PROP_LOCAL: if (priv->props.local) - g_value_set_string (value, nm_utils_inet4_ntop (priv->props.local, NULL)); + g_value_take_string (value, nm_utils_inet4_ntop_dup (priv->props.local)); else if (!IN6_IS_ADDR_UNSPECIFIED (&priv->props.local6)) - g_value_set_string (value, nm_utils_inet6_ntop (&priv->props.local6, NULL)); + g_value_take_string (value, nm_utils_inet6_ntop_dup (&priv->props.local6)); break; case PROP_TOS: g_value_set_uchar (value, priv->props.tos); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 3dbe5839ee..f33ede7b90 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -5475,10 +5475,13 @@ nm_device_generate_connection (NMDevice *self, nm_connection_add_setting (connection, nm_setting_proxy_new ()); pllink = nm_platform_link_get (nm_device_get_platform (self), priv->ifindex); - if (pllink && pllink->inet6_token.id) { + if ( pllink + && pllink->inet6_token.id) { + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; + g_object_set (s_ip6, NM_SETTING_IP6_CONFIG_ADDR_GEN_MODE, NM_IN6_ADDR_GEN_MODE_EUI64, - NM_SETTING_IP6_CONFIG_TOKEN, nm_utils_inet6_interface_identifier_to_token (pllink->inet6_token, NULL), + NM_SETTING_IP6_CONFIG_TOKEN, nm_utils_inet6_interface_identifier_to_token (pllink->inet6_token, sbuf), NULL); } } @@ -6641,13 +6644,15 @@ acd_manager_probe_terminated (NMAcdManager *acd_manager, gpointer user_data) for (i = 0; data->configs && data->configs[i]; i++) { nm_ip_config_iter_ip4_address_for_each (&ipconf_iter, data->configs[i], &address) { + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; + result = nm_acd_manager_check_address (acd_manager, address->address); success &= result; _NMLOG (result ? LOGL_DEBUG : LOGL_WARN, LOGD_DEVICE, "IPv4 DAD result: address %s is %s", - nm_utils_inet4_ntop (address->address, NULL), + nm_utils_inet4_ntop (address->address, sbuf), result ? "unique" : "duplicate"); } } @@ -8678,6 +8683,7 @@ nm_device_use_ip6_subnet (NMDevice *self, const NMPlatformIP6Address *subnet) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); NMPlatformIP6Address address = *subnet; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; if (!applied_config_get_current (&priv->ac_ip6_config)) applied_config_init_new (&priv->ac_ip6_config, self, AF_INET6); @@ -8687,7 +8693,7 @@ nm_device_use_ip6_subnet (NMDevice *self, const NMPlatformIP6Address *subnet) applied_config_add_address (&priv->ac_ip6_config, NM_PLATFORM_IP_ADDRESS_CAST (&address)); _LOGD (LOGD_IP6, "ipv6-pd: using %s address (preferred for %u seconds)", - nm_utils_inet6_ntop (&address.address, NULL), + nm_utils_inet6_ntop (&address.address, sbuf), subnet->preferred); /* This also updates the ndisc if there are actual changes. */ @@ -8806,6 +8812,7 @@ check_and_add_ipv6ll_addr (NMDevice *self) NMSettingIP6Config *s_ip6 = NULL; GError *error = NULL; const char *addr_type; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; if (!priv->ipv6ll_handle) return; @@ -8866,7 +8873,8 @@ check_and_add_ipv6ll_addr (NMDevice *self) addr_type = "EUI-64"; } - _LOGD (LOGD_IP6, "linklocal6: generated %s IPv6LL address %s", addr_type, nm_utils_inet6_ntop (&lladdr, NULL)); + _LOGD (LOGD_IP6, "linklocal6: generated %s IPv6LL address %s", + addr_type, nm_utils_inet6_ntop (&lladdr, sbuf)); priv->ipv6ll_has = TRUE; priv->ipv6ll_addr = lladdr; ip_config_merge_and_apply (self, AF_INET6, TRUE); @@ -14435,7 +14443,7 @@ find_dhcp4_address (NMDevice *self) nm_ip_config_iter_ip4_address_for_each (&ipconf_iter, priv->ip_config_4, &a) { if (a->addr_source == NM_IP_CONFIG_SOURCE_DHCP) - return g_strdup (nm_utils_inet4_ntop (a->address, NULL)); + return nm_utils_inet4_ntop_dup (a->address); } return NULL; } diff --git a/src/devices/tests/test-acd.c b/src/devices/tests/test-acd.c index 28910338ea..aff71825e8 100644 --- a/src/devices/tests/test-acd.c +++ b/src/devices/tests/test-acd.c @@ -154,6 +154,7 @@ again: for (i = 0; info->addresses[i]; i++) { gboolean val; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; val = nm_acd_manager_check_address (manager, info->addresses[i]); if (val == info->expected_result[i]) @@ -168,7 +169,7 @@ again: } g_error ("expected check for address #%d (%s) to %s, but it didn't", - i, nm_utils_inet4_ntop (info->addresses[i], NULL), + i, nm_utils_inet4_ntop (info->addresses[i], sbuf), info->expected_result[i] ? "detect no duplicated" : "detect a duplicate"); } diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index c836822df1..4e4b9b0940 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -232,7 +232,8 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, NMIP4Config *ip4_config = NULL; struct in_addr tmp_addr; const struct in_addr *addr_list; - char buf[INET_ADDRSTRLEN]; + char addr_str[NM_UTILS_INET_ADDRSTRLEN]; + char addr_str2[NM_UTILS_INET_ADDRSTRLEN]; const char *s; guint32 lifetime = 0, i; NMPlatformIP4Address address; @@ -258,9 +259,9 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, memset (&address, 0, sizeof (address)); address.address = tmp_addr.s_addr; address.peer_address = tmp_addr.s_addr; - s = nm_utils_inet4_ntop (tmp_addr.s_addr, NULL); - LOG_LEASE (LOGD_DHCP4, "address %s", s); - add_option (options, dhcp4_requests, DHCP_OPTION_IP_ADDRESS, s); + nm_utils_inet4_ntop (tmp_addr.s_addr, addr_str); + LOG_LEASE (LOGD_DHCP4, "address %s", addr_str); + add_option (options, dhcp4_requests, DHCP_OPTION_IP_ADDRESS, addr_str); /* Prefix/netmask */ sd_dhcp_lease_get_netmask (lease, &tmp_addr); @@ -269,7 +270,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, add_option (options, dhcp4_requests, SD_DHCP_OPTION_SUBNET_MASK, - nm_utils_inet4_ntop (tmp_addr.s_addr, NULL)); + nm_utils_inet4_ntop (tmp_addr.s_addr, addr_str)); /* Lease time */ sd_dhcp_lease_get_lifetime (lease, &lifetime); @@ -292,7 +293,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, for (i = 0; i < num; i++) { if (addr_list[i].s_addr) { nm_ip4_config_add_nameserver (ip4_config, addr_list[i].s_addr); - s = nm_utils_inet4_ntop (addr_list[i].s_addr, NULL); + s = nm_utils_inet4_ntop (addr_list[i].s_addr, addr_str); LOG_LEASE (LOGD_DHCP4, "nameserver '%s'", s); g_string_append_printf (str, "%s%s", str->len ? " " : "", s); } @@ -366,8 +367,8 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, route.table_coerced = nm_platform_route_table_coerce (route_table); nm_ip4_config_add_route (ip4_config, &route, NULL); - s = nm_utils_inet4_ntop (route.network, buf); - gw_str = nm_utils_inet4_ntop (route.gateway, NULL); + s = nm_utils_inet4_ntop (route.network, addr_str); + gw_str = nm_utils_inet4_ntop (route.gateway, addr_str2); LOG_LEASE (LOGD_DHCP4, "static route %s/%d gw %s", s, route.plen, gw_str); g_string_append_printf (str, "%s%s/%d %s", str->len ? " " : "", s, route.plen, gw_str); @@ -377,7 +378,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, gateway_has = TRUE; gateway = route.gateway; - s = nm_utils_inet4_ntop (route.gateway, NULL); + s = nm_utils_inet4_ntop (route.gateway, addr_str); LOG_LEASE (LOGD_DHCP4, "gateway %s", s); add_option (options, dhcp4_requests, SD_DHCP_OPTION_ROUTER, s); } @@ -398,7 +399,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, if (r == 0) { gateway_has = TRUE; gateway = tmp_addr.s_addr; - s = nm_utils_inet4_ntop (tmp_addr.s_addr, NULL); + s = nm_utils_inet4_ntop (tmp_addr.s_addr, addr_str); LOG_LEASE (LOGD_DHCP4, "gateway %s", s); add_option (options, dhcp4_requests, SD_DHCP_OPTION_ROUTER, s); } @@ -428,7 +429,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, if (num > 0) { nm_gstring_prepare (&str); for (i = 0; i < num; i++) { - s = nm_utils_inet4_ntop (addr_list[i].s_addr, buf); + s = nm_utils_inet4_ntop (addr_list[i].s_addr, addr_str); LOG_LEASE (LOGD_DHCP4, "ntp server '%s'", s); g_string_append_printf (str, "%s%s", str->len ? " " : "", s); } @@ -726,7 +727,7 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, struct in6_addr tmp_addr, *dns; uint32_t lft_pref, lft_valid; NMIP6Config *ip6_config; - const char *addr_str; + char addr_str[NM_UTILS_INET_ADDRSTRLEN]; char **domains; nm_auto_free_gstring GString *str = NULL; int num, i; @@ -751,8 +752,10 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, nm_ip6_config_add_address (ip6_config, &address); - addr_str = nm_utils_inet6_ntop (&tmp_addr, NULL); - g_string_append_printf (str, "%s%s", str->len ? " " : "", addr_str); + nm_utils_inet6_ntop (&tmp_addr, addr_str); + if (str->len) + g_string_append_c (str, ' '); + g_string_append (str, addr_str); LOG_LEASE (LOGD_DHCP6, "address %s", @@ -777,8 +780,10 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, nm_gstring_prepare (&str); for (i = 0; i < num; i++) { nm_ip6_config_add_nameserver (ip6_config, &dns[i]); - addr_str = nm_utils_inet6_ntop (&dns[i], NULL); - g_string_append_printf (str, "%s%s", str->len ? " " : "", addr_str); + nm_utils_inet6_ntop (&dns[i], addr_str); + if (str->len) + g_string_append_c (str, ' '); + g_string_append (str, addr_str); LOG_LEASE (LOGD_DHCP6, "nameserver %s", addr_str); } add_option (options, dhcp6_requests, SD_DHCP6_OPTION_DNS_SERVERS, str->str); diff --git a/src/dhcp/nm-dhcp-utils.c b/src/dhcp/nm-dhcp-utils.c index 9b1653b8f9..768f9fd790 100644 --- a/src/dhcp/nm-dhcp-utils.c +++ b/src/dhcp/nm-dhcp-utils.c @@ -197,7 +197,8 @@ ip4_process_dhclient_rfc3442_routes (const char *iface, /* gateway passed as classless static route */ *gwaddr = route.gateway; } else { - char addr[INET_ADDRSTRLEN]; + char b1[INET_ADDRSTRLEN]; + char b2[INET_ADDRSTRLEN]; /* normal route */ route.rt_source = NM_IP_CONFIG_SOURCE_DHCP; @@ -206,8 +207,9 @@ ip4_process_dhclient_rfc3442_routes (const char *iface, nm_ip4_config_add_route (ip4_config, &route, NULL); _LOG2I (LOGD_DHCP4, iface, " classless static route %s/%d gw %s", - nm_utils_inet4_ntop (route.network, addr), route.plen, - nm_utils_inet4_ntop (route.gateway, NULL)); + nm_utils_inet4_ntop (route.network, b1), + route.plen, + nm_utils_inet4_ntop (route.gateway, b2)); } } @@ -408,6 +410,7 @@ nm_dhcp_utils_ip4_config_from_options (NMDedupMultiIndex *multi_idx, gboolean gateway_has = FALSE; guint32 gateway = 0; guint8 plen = 0; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; g_return_val_if_fail (options != NULL, NULL); @@ -439,7 +442,7 @@ nm_dhcp_utils_ip4_config_from_options (NMDedupMultiIndex *multi_idx, process_classful_routes (iface, options, route_table, route_metric, ip4_config); if (gateway) { - _LOG2I (LOGD_DHCP4, iface, " gateway %s", nm_utils_inet4_ntop (gateway, NULL)); + _LOG2I (LOGD_DHCP4, iface, " gateway %s", nm_utils_inet4_ntop (gateway, sbuf)); gateway_has = TRUE; } else { /* If the gateway wasn't provided as a classless static route with a diff --git a/src/ndisc/nm-ndisc.c b/src/ndisc/nm-ndisc.c index 1dd8398c75..2da08c5fc2 100644 --- a/src/ndisc/nm-ndisc.c +++ b/src/ndisc/nm-ndisc.c @@ -965,7 +965,9 @@ nm_ndisc_dad_failed (NMNDisc *ndisc, const struct in6_addr *address, gboolean em NMNDiscAddress *item = &g_array_index (rdata->addresses, NMNDiscAddress, i); if (IN6_ARE_ADDR_EQUAL (&item->address, address)) { - _LOGD ("DAD failed for discovered address %s", nm_utils_inet6_ntop (address, NULL)); + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; + + _LOGD ("DAD failed for discovered address %s", nm_utils_inet6_ntop (address, sbuf)); changed = TRUE; if (!complete_address (ndisc, item)) { g_array_remove_index (rdata->addresses, i); @@ -1056,10 +1058,11 @@ _config_changed_log (NMNDisc *ndisc, NMNDiscConfigMap changed) } for (i = 0; i < rdata->routes->len; i++) { NMNDiscRoute *route = &g_array_index (rdata->routes, NMNDiscRoute, i); + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; inet_ntop (AF_INET6, &route->network, addrstr, sizeof (addrstr)); _LOGD (" route %s/%u via %s pref %s exp %s", addrstr, (guint) route->plen, - nm_utils_inet6_ntop (&route->gateway, NULL), + nm_utils_inet6_ntop (&route->gateway, sbuf), nm_icmpv6_router_pref_to_string (route->preference, str_pref, sizeof (str_pref)), get_exp (str_exp, now_ns, route)); } diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index e10298b47c..0ba3a5cb94 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -3045,14 +3045,12 @@ nm_utils_ipv6_interface_identifier_get_from_token (NMUtilsIPv6IfaceId *iid, /** * nm_utils_inet6_interface_identifier_to_token: * @iid: %NMUtilsIPv6IfaceId interface identifier - * @buf: the destination buffer or %NULL + * @buf: the destination buffer of at least %NM_UTILS_INET_ADDRSTRLEN + * bytes. * * Converts the interface identifier to a string token. - * If the destination buffer it set, set it is used to store the - * resulting token, otherwise an internal static buffer is used. - * The buffer needs to be %NM_UTILS_INET_ADDRSTRLEN characters long. * - * Returns: a statically allocated array. Do not g_free(). + * Returns: the input buffer filled with the id as string. */ const char * nm_utils_inet6_interface_identifier_to_token (NMUtilsIPv6IfaceId iid, char *buf) diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index ce7f7fc4c2..a6b6f83fec 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -1051,6 +1051,7 @@ nm_ip4_config_create_setting (const NMIP4Config *self) NMDedupMultiIter ipconf_iter; const NMPlatformIP4Address *address; const NMPlatformIP4Route *route; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; s_ip4 = NM_SETTING_IP_CONFIG (nm_setting_ip4_config_new ()); @@ -1095,7 +1096,7 @@ nm_ip4_config_create_setting (const NMIP4Config *self) g_object_set (s_ip4, NM_SETTING_IP_CONFIG_GATEWAY, nm_utils_inet4_ntop (NMP_OBJECT_CAST_IP4_ROUTE (priv->best_default_route)->gateway, - NULL), + sbuf), NULL); } @@ -1130,7 +1131,7 @@ nm_ip4_config_create_setting (const NMIP4Config *self) for (i = 0; i < nnameservers; i++) { guint32 nameserver = nm_ip4_config_get_nameserver (self, i); - nm_setting_ip_config_add_dns (s_ip4, nm_utils_inet4_ntop (nameserver, NULL)); + nm_setting_ip_config_add_dns (s_ip4, nm_utils_inet4_ntop (nameserver, sbuf)); } for (i = 0; i < nsearches; i++) { const char *search = nm_ip4_config_get_search (self, i); @@ -2039,9 +2040,11 @@ nm_ip_config_dump (const NMIPConfig *self, } for (i = 0; i < nm_ip_config_get_num_nameservers (self); i++) { + char buf[NM_UTILS_INET_ADDRSTRLEN]; + ptr = nm_ip_config_get_nameserver (self, i); nm_log (level, domain, NULL, NULL, " dns : %s", - nm_utils_inet_ntop (addr_family, ptr, NULL)); + nm_utils_inet_ntop (addr_family, ptr, buf)); } for (i = 0; i < nm_ip_config_get_num_domains (self); i++) @@ -3024,6 +3027,7 @@ get_property (GObject *object, guint prop_id, const NMPlatformIP4Route *route; GVariantBuilder builder_data, builder_legacy; guint i; + char addr_str[NM_UTILS_INET_ADDRSTRLEN]; switch (prop_id) { case PROP_IFINDEX: @@ -3061,14 +3065,14 @@ get_property (GObject *object, guint prop_id, g_variant_builder_init (&addr_builder, G_VARIANT_TYPE ("a{sv}")); g_variant_builder_add (&addr_builder, "{sv}", "address", - g_variant_new_string (nm_utils_inet4_ntop (address->address, NULL))); + g_variant_new_string (nm_utils_inet4_ntop (address->address, addr_str))); g_variant_builder_add (&addr_builder, "{sv}", "prefix", g_variant_new_uint32 (address->plen)); if (address->peer_address != address->address) { g_variant_builder_add (&addr_builder, "{sv}", "peer", - g_variant_new_string (nm_utils_inet4_ntop (address->peer_address, NULL))); + g_variant_new_string (nm_utils_inet4_ntop (address->peer_address, addr_str))); } if (*address->label) { @@ -3123,14 +3127,14 @@ out_addresses_cached: g_variant_builder_init (&route_builder, G_VARIANT_TYPE ("a{sv}")); g_variant_builder_add (&route_builder, "{sv}", "dest", - g_variant_new_string (nm_utils_inet4_ntop (route->network, NULL))); + g_variant_new_string (nm_utils_inet4_ntop (route->network, addr_str))); g_variant_builder_add (&route_builder, "{sv}", "prefix", g_variant_new_uint32 (route->plen)); if (route->gateway) { g_variant_builder_add (&route_builder, "{sv}", "next-hop", - g_variant_new_string (nm_utils_inet4_ntop (route->gateway, NULL))); + g_variant_new_string (nm_utils_inet4_ntop (route->gateway, addr_str))); } g_variant_builder_add (&route_builder, "{sv}", "metric", @@ -3172,9 +3176,8 @@ out_routes_cached: break; case PROP_GATEWAY: if (priv->best_default_route) { - g_value_set_string (value, - nm_utils_inet4_ntop (NMP_OBJECT_CAST_IP4_ROUTE (priv->best_default_route)->gateway, - NULL)); + g_value_take_string (value, + nm_utils_inet4_ntop_dup (NMP_OBJECT_CAST_IP4_ROUTE (priv->best_default_route)->gateway)); } else g_value_set_string (value, NULL); break; @@ -3183,7 +3186,6 @@ out_routes_cached: for (i = 0; i < priv->nameservers->len; i++) { GVariantBuilder nested_builder; - char addr_str[NM_UTILS_INET_ADDRSTRLEN]; nm_utils_inet4_ntop (g_array_index (priv->nameservers, in_addr_t, i), addr_str); @@ -3220,8 +3222,6 @@ out_routes_cached: case PROP_WINS_SERVER_DATA: g_variant_builder_init (&builder_data, G_VARIANT_TYPE ("as")); for (i = 0; i < priv->wins->len; i++) { - char addr_str[NM_UTILS_INET_ADDRSTRLEN]; - g_variant_builder_add (&builder_data, "s", nm_utils_inet4_ntop (g_array_index (priv->wins, in_addr_t, i), diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c index 42240e698f..c1864d338a 100644 --- a/src/nm-ip6-config.c +++ b/src/nm-ip6-config.c @@ -713,6 +713,7 @@ nm_ip6_config_create_setting (const NMIP6Config *self) NMSettingIPConfig *s_ip6; guint nnameservers, nsearches, noptions; const char *method = NULL; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; int i; NMDedupMultiIter ipconf_iter; const NMPlatformIP6Address *address; @@ -765,7 +766,7 @@ nm_ip6_config_create_setting (const NMIP6Config *self) g_object_set (s_ip6, NM_SETTING_IP_CONFIG_GATEWAY, nm_utils_inet6_ntop (&NMP_OBJECT_CAST_IP6_ROUTE (priv->best_default_route)->gateway, - NULL), + sbuf), NULL); } @@ -804,7 +805,7 @@ nm_ip6_config_create_setting (const NMIP6Config *self) for (i = 0; i < nnameservers; i++) { const struct in6_addr *nameserver = nm_ip6_config_get_nameserver (self, i); - nm_setting_ip_config_add_dns (s_ip6, nm_utils_inet6_ntop (nameserver, NULL)); + nm_setting_ip_config_add_dns (s_ip6, nm_utils_inet6_ntop (nameserver, sbuf)); } for (i = 0; i < nsearches; i++) { const char *search = nm_ip6_config_get_search (self, i); @@ -2473,6 +2474,7 @@ get_property (GObject *object, guint prop_id, NMDedupMultiIter ipconf_iter; const NMPlatformIP6Route *route; GVariantBuilder builder_data, builder_legacy; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; switch (prop_id) { case PROP_IFINDEX: @@ -2509,7 +2511,7 @@ get_property (GObject *object, guint prop_id, g_variant_builder_init (&addr_builder, G_VARIANT_TYPE ("a{sv}")); g_variant_builder_add (&addr_builder, "{sv}", "address", - g_variant_new_string (nm_utils_inet6_ntop (&address->address, NULL))); + g_variant_new_string (nm_utils_inet6_ntop (&address->address, sbuf))); g_variant_builder_add (&addr_builder, "{sv}", "prefix", g_variant_new_uint32 (address->plen)); @@ -2517,7 +2519,7 @@ get_property (GObject *object, guint prop_id, && !IN6_ARE_ADDR_EQUAL (&address->peer_address, &address->address)) { g_variant_builder_add (&addr_builder, "{sv}", "peer", - g_variant_new_string (nm_utils_inet6_ntop (&address->peer_address, NULL))); + g_variant_new_string (nm_utils_inet6_ntop (&address->peer_address, sbuf))); } g_variant_builder_add (&builder_data, "a{sv}", &addr_builder); @@ -2562,14 +2564,14 @@ out_addresses_cached: g_variant_builder_init (&route_builder, G_VARIANT_TYPE ("a{sv}")); g_variant_builder_add (&route_builder, "{sv}", "dest", - g_variant_new_string (nm_utils_inet6_ntop (&route->network, NULL))); + g_variant_new_string (nm_utils_inet6_ntop (&route->network, sbuf))); g_variant_builder_add (&route_builder, "{sv}", "prefix", g_variant_new_uint32 (route->plen)); if (!IN6_IS_ADDR_UNSPECIFIED (&route->gateway)) { g_variant_builder_add (&route_builder, "{sv}", "next-hop", - g_variant_new_string (nm_utils_inet6_ntop (&route->gateway, NULL))); + g_variant_new_string (nm_utils_inet6_ntop (&route->gateway, sbuf))); } g_variant_builder_add (&route_builder, "{sv}", @@ -2607,9 +2609,8 @@ out_routes_cached: break; case PROP_GATEWAY: if (priv->best_default_route) { - g_value_set_string (value, - nm_utils_inet6_ntop (&NMP_OBJECT_CAST_IP6_ROUTE (priv->best_default_route)->gateway, - NULL)); + g_value_take_string (value, + nm_utils_inet6_ntop_dup (&NMP_OBJECT_CAST_IP6_ROUTE (priv->best_default_route)->gateway)); } else g_value_set_string (value, NULL); break; diff --git a/src/nm-pacrunner-manager.c b/src/nm-pacrunner-manager.c index 677e56e393..b9881f76f2 100644 --- a/src/nm-pacrunner-manager.c +++ b/src/nm-pacrunner-manager.c @@ -173,6 +173,7 @@ get_ip4_domains (GPtrArray *domains, NMIP4Config *ip4) const NMPlatformIP4Address *address; const NMPlatformIP4Route *routes; guint i; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; /* Extract searches */ for (i = 0; i < nm_ip4_config_get_num_searches (ip4); i++) @@ -186,7 +187,7 @@ get_ip4_domains (GPtrArray *domains, NMIP4Config *ip4) nm_ip_config_iter_ip4_address_for_each (&ipconf_iter, ip4, &address) { cidr = g_strdup_printf ("%s/%u", - nm_utils_inet4_ntop (address->address, NULL), + nm_utils_inet4_ntop (address->address, sbuf), address->plen); g_ptr_array_add (domains, cidr); } @@ -195,7 +196,7 @@ get_ip4_domains (GPtrArray *domains, NMIP4Config *ip4) if (NM_PLATFORM_IP_ROUTE_IS_DEFAULT (routes)) continue; cidr = g_strdup_printf ("%s/%u", - nm_utils_inet4_ntop (routes->network, NULL), + nm_utils_inet4_ntop (routes->network, sbuf), routes->plen); g_ptr_array_add (domains, cidr); } @@ -209,6 +210,7 @@ get_ip6_domains (GPtrArray *domains, NMIP6Config *ip6) const NMPlatformIP6Address *address; const NMPlatformIP6Route *routes; guint i; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; /* Extract searches */ for (i = 0; i < nm_ip6_config_get_num_searches (ip6); i++) @@ -221,7 +223,7 @@ get_ip6_domains (GPtrArray *domains, NMIP6Config *ip6) /* Add addresses and routes in CIDR form */ nm_ip_config_iter_ip6_address_for_each (&ipconf_iter, ip6, &address) { cidr = g_strdup_printf ("%s/%u", - nm_utils_inet6_ntop (&address->address, NULL), + nm_utils_inet6_ntop (&address->address, sbuf), address->plen); g_ptr_array_add (domains, cidr); } @@ -230,7 +232,7 @@ get_ip6_domains (GPtrArray *domains, NMIP6Config *ip6) if (NM_PLATFORM_IP_ROUTE_IS_DEFAULT (routes)) continue; cidr = g_strdup_printf ("%s/%u", - nm_utils_inet6_ntop (&routes->network, NULL), + nm_utils_inet6_ntop (&routes->network, sbuf), routes->plen); g_ptr_array_add (domains, cidr); } diff --git a/src/nm-policy.c b/src/nm-policy.c index be9c706212..4120227757 100644 --- a/src/nm-policy.c +++ b/src/nm-policy.c @@ -179,9 +179,10 @@ static void clear_ip6_prefix_delegation (gpointer data) { IP6PrefixDelegation *delegation = data; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; _LOGD (LOGD_IP6, "ipv6-pd: undelegating prefix %s/%d", - nm_utils_inet6_ntop (&delegation->prefix.address, NULL), + nm_utils_inet6_ntop (&delegation->prefix.address, sbuf), delegation->prefix.plen); g_hash_table_foreach (delegation->subnets, _clear_ip6_subnet, NULL); @@ -215,13 +216,14 @@ ip6_subnet_from_delegation (IP6PrefixDelegation *delegation, NMDevice *device) { NMPlatformIP6Address *subnet; int ifindex = nm_device_get_ifindex (device); + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; subnet = g_hash_table_lookup (delegation->subnets, GINT_TO_POINTER (ifindex)); if (!subnet) { /* Check for out-of-prefixes condition. */ if (delegation->next_subnet >= (1 << (64 - delegation->prefix.plen))) { _LOGD (LOGD_IP6, "ipv6-pd: no more prefixes in %s/%d", - nm_utils_inet6_ntop (&delegation->prefix.address, NULL), + nm_utils_inet6_ntop (&delegation->prefix.address, sbuf), delegation->prefix.plen); return FALSE; } @@ -249,7 +251,7 @@ ip6_subnet_from_delegation (IP6PrefixDelegation *delegation, NMDevice *device) subnet->preferred = delegation->prefix.preferred; _LOGD (LOGD_IP6, "ipv6-pd: %s allocated from a /%d prefix on %s", - nm_utils_inet6_ntop (&subnet->address, NULL), + nm_utils_inet6_ntop (&subnet->address, sbuf), delegation->prefix.plen, nm_device_get_iface (device)); @@ -320,9 +322,10 @@ device_ip6_prefix_delegated (NMDevice *device, guint i; const CList *tmp_list; NMActiveConnection *ac; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; _LOGI (LOGD_IP6, "ipv6-pd: received a prefix %s/%d from %s", - nm_utils_inet6_ntop (&prefix->address, NULL), + nm_utils_inet6_ntop (&prefix->address, sbuf), prefix->plen, nm_device_get_iface (device)); diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index 1814eeb967..f03668a7d8 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -1267,12 +1267,14 @@ ip_route_add (NMPlatform *platform, } } if (!has_route_to_gw) { + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; + if (addr_family == AF_INET) { nm_log_warn (LOGD_PLATFORM, "Fake platform: failure adding ip4-route '%d: %s/%d %d': Network Unreachable", - r->ifindex, nm_utils_inet4_ntop (r4->network, NULL), r->plen, r->metric); + r->ifindex, nm_utils_inet4_ntop (r4->network, sbuf), r->plen, r->metric); } else { nm_log_warn (LOGD_PLATFORM, "Fake platform: failure adding ip6-route '%d: %s/%d %d': Network Unreachable", - r->ifindex, nm_utils_inet6_ntop (&r6->network, NULL), r->plen, r->metric); + r->ifindex, nm_utils_inet6_ntop (&r6->network, sbuf), r->plen, r->metric); } return NM_PLATFORM_ERROR_UNSPECIFIED; } diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 008a3bc973..9a385d504c 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5580,9 +5580,10 @@ static gboolean link_set_token (NMPlatform *platform, int ifindex, NMUtilsIPv6IfaceId iid) { nm_auto_nlmsg struct nl_msg *nlmsg = NULL; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; _LOGD ("link: change %d: token: set IPv6 address generation token to %s", - ifindex, nm_utils_inet6_interface_identifier_to_token (iid, NULL)); + ifindex, nm_utils_inet6_interface_identifier_to_token (iid, sbuf)); nlmsg = _nl_msg_new_link (RTM_NEWLINK, 0, ifindex, NULL, 0, 0); if (!nlmsg || !_nl_msg_new_link_set_afspec (nlmsg, -1, &iid)) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index dd119b9970..fc90c452f3 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -3466,8 +3466,9 @@ gboolean nm_platform_ip4_address_delete (NMPlatform *self, int ifindex, in_addr_t address, guint8 plen, in_addr_t peer_address) { char str_dev[TO_STRING_DEV_BUF_SIZE]; - char str_peer2[NM_UTILS_INET_ADDRSTRLEN]; - char str_peer[100]; + char b1[NM_UTILS_INET_ADDRSTRLEN]; + char b2[NM_UTILS_INET_ADDRSTRLEN]; + char str_peer[INET_ADDRSTRLEN + 50]; _CHECK_SELF (self, klass, FALSE); @@ -3475,9 +3476,13 @@ nm_platform_ip4_address_delete (NMPlatform *self, int ifindex, in_addr_t address g_return_val_if_fail (plen <= 32, FALSE); _LOG3D ("address: deleting IPv4 address %s/%d, %s%s", - nm_utils_inet4_ntop (address, NULL), plen, + nm_utils_inet4_ntop (address, b1), + plen, peer_address != address - ? nm_sprintf_buf (str_peer, "peer %s, ", nm_utils_inet4_ntop (peer_address, str_peer2)) : "", + ? nm_sprintf_buf (str_peer, + "peer %s, ", + nm_utils_inet4_ntop (peer_address, b2)) + : "", _to_string_dev (self, ifindex, str_dev, sizeof (str_dev))); return klass->ip4_address_delete (self, ifindex, address, plen, peer_address); } @@ -3486,6 +3491,7 @@ gboolean nm_platform_ip6_address_delete (NMPlatform *self, int ifindex, struct in6_addr address, guint8 plen) { char str_dev[TO_STRING_DEV_BUF_SIZE]; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; _CHECK_SELF (self, klass, FALSE); @@ -3493,7 +3499,7 @@ nm_platform_ip6_address_delete (NMPlatform *self, int ifindex, struct in6_addr a g_return_val_if_fail (plen <= 128, FALSE); _LOG3D ("address: deleting IPv6 address %s/%d, %s", - nm_utils_inet6_ntop (&address, NULL), plen, + nm_utils_inet6_ntop (&address, sbuf), plen, _to_string_dev (self, ifindex, str_dev, sizeof (str_dev))); return klass->ip6_address_delete (self, ifindex, address, plen); } @@ -5517,6 +5523,7 @@ nm_platform_lnk_vxlan_to_string (const NMPlatformLnkVxlan *lnk, char *buf, gsize char str_dst_port[25]; char str_tos[25]; char str_ttl[25]; + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; if (!nm_utils_to_string_buffer_init_null (lnk, &buf, &len)) return buf; @@ -5527,7 +5534,7 @@ nm_platform_lnk_vxlan_to_string (const NMPlatformLnkVxlan *lnk, char *buf, gsize g_snprintf (str_group, sizeof (str_group), " %s %s", IN_MULTICAST (ntohl (lnk->group)) ? "group" : "remote", - nm_utils_inet4_ntop (lnk->group, NULL)); + nm_utils_inet4_ntop (lnk->group, sbuf)); } if (IN6_IS_ADDR_UNSPECIFIED (&lnk->group6)) str_group6[0] = '\0'; @@ -5536,7 +5543,7 @@ nm_platform_lnk_vxlan_to_string (const NMPlatformLnkVxlan *lnk, char *buf, gsize " %s%s %s", IN6_IS_ADDR_MULTICAST (&lnk->group6) ? "group" : "remote", str_group[0] ? "6" : "", /* usually, a vxlan has either v4 or v6 only. */ - nm_utils_inet6_ntop (&lnk->group6, NULL)); + nm_utils_inet6_ntop (&lnk->group6, sbuf)); } if (lnk->local == 0) @@ -5544,7 +5551,7 @@ nm_platform_lnk_vxlan_to_string (const NMPlatformLnkVxlan *lnk, char *buf, gsize else { g_snprintf (str_local, sizeof (str_local), " local %s", - nm_utils_inet4_ntop (lnk->local, NULL)); + nm_utils_inet4_ntop (lnk->local, sbuf)); } if (IN6_IS_ADDR_UNSPECIFIED (&lnk->local6)) str_local6[0] = '\0'; @@ -5552,7 +5559,7 @@ nm_platform_lnk_vxlan_to_string (const NMPlatformLnkVxlan *lnk, char *buf, gsize g_snprintf (str_local6, sizeof (str_local6), " local%s %s", str_local[0] ? "6" : "", /* usually, a vxlan has either v4 or v6 only. */ - nm_utils_inet6_ntop (&lnk->local6, NULL)); + nm_utils_inet6_ntop (&lnk->local6, sbuf)); } g_snprintf (buf, len, @@ -5966,13 +5973,21 @@ nm_platform_ip4_route_to_string (const NMPlatformIP4Route *route, char *buf, gsi const char * nm_platform_ip6_route_to_string (const NMPlatformIP6Route *route, char *buf, gsize len) { - char s_network[INET6_ADDRSTRLEN], s_gateway[INET6_ADDRSTRLEN], s_pref_src[INET6_ADDRSTRLEN]; - char s_src_all[INET6_ADDRSTRLEN + 40], s_src[INET6_ADDRSTRLEN]; + char s_network[INET6_ADDRSTRLEN]; + char s_gateway[INET6_ADDRSTRLEN]; + char s_pref_src[INET6_ADDRSTRLEN]; + char s_src_all[INET6_ADDRSTRLEN + 40]; + char s_src[INET6_ADDRSTRLEN]; char str_table[30]; char str_pref[40]; char str_pref2[30]; - char str_dev[TO_STRING_DEV_BUF_SIZE], s_source[50]; - char str_window[32], str_cwnd[32], str_initcwnd[32], str_initrwnd[32], str_mtu[32]; + char str_dev[TO_STRING_DEV_BUF_SIZE]; + char s_source[50]; + char str_window[32]; + char str_cwnd[32]; + char str_initcwnd[32]; + char str_initrwnd[32]; + char str_mtu[32]; char str_rtm_flags[_RTM_FLAGS_TO_STRING_MAXLEN]; if (!nm_utils_to_string_buffer_init_null (route, &buf, &len)) diff --git a/src/platform/nmp-object.c b/src/platform/nmp-object.c index 1218b5a224..683d5cdcfc 100644 --- a/src/platform/nmp-object.c +++ b/src/platform/nmp-object.c @@ -903,11 +903,9 @@ static const char * \ _vt_cmd_plobj_to_string_id_##type (const NMPlatformObject *_obj, char *buf, gsize buf_len) \ { \ plat_type *const obj = (plat_type *) _obj; \ - char buf1[NM_UTILS_INET_ADDRSTRLEN]; \ - char buf2[NM_UTILS_INET_ADDRSTRLEN]; \ + _nm_unused char buf1[NM_UTILS_INET_ADDRSTRLEN]; \ + _nm_unused char buf2[NM_UTILS_INET_ADDRSTRLEN]; \ \ - (void) buf1; \ - (void) buf2; \ g_snprintf (buf, buf_len, \ __VA_ARGS__); \ return buf; \ diff --git a/src/platform/tests/test-common.c b/src/platform/tests/test-common.c index 8aea3e7d95..5e935b6d77 100644 --- a/src/platform/tests/test-common.c +++ b/src/platform/tests/test-common.c @@ -366,9 +366,12 @@ _nmtstp_assert_ip4_route_exists (const char *file, &c); if (c != c_exists && c_exists != -1) { + char sbuf[NM_UTILS_INET_ADDRSTRLEN]; + g_error ("[%s:%u] %s(): The ip4 route %s/%d metric %u tos %u shall exist %u times, but platform has it %u times", file, line, func, - nm_utils_inet4_ntop (network, NULL), plen, + nm_utils_inet4_ntop (network, sbuf), + plen, metric, tos, c_exists, @@ -821,7 +824,8 @@ _ip_address_add (NMPlatform *platform, gs_free char *s_valid = NULL; gs_free char *s_preferred = NULL; gs_free char *s_label = NULL; - char b1[NM_UTILS_INET_ADDRSTRLEN], b2[NM_UTILS_INET_ADDRSTRLEN]; + char b1[NM_UTILS_INET_ADDRSTRLEN]; + char b2[NM_UTILS_INET_ADDRSTRLEN]; ifname = nm_platform_link_get_name (platform, ifindex); g_assert (ifname); @@ -834,14 +838,14 @@ _ip_address_add (NMPlatform *platform, s_label = g_strdup_printf ("%s:%s", ifname, label); if (is_v4) { - char s_peer[100]; + char s_peer[NM_UTILS_INET_ADDRSTRLEN + 50]; g_assert (flags == 0); if ( peer_address->addr4 != address->addr4 || nmtst_get_rand_int () % 2) { /* If the peer is the same as the local address, we can omit it. The result should be identical */ - g_snprintf (s_peer, sizeof (s_peer), " peer %s", nm_utils_inet4_ntop (peer_address->addr4, b2)); + nm_sprintf_buf (s_peer, " peer %s", nm_utils_inet4_ntop (peer_address->addr4, b2)); } else s_peer[0] = '\0'; @@ -1052,7 +1056,8 @@ _ip_address_del (NMPlatform *platform, if (external_command) { const char *ifname; - char b1[NM_UTILS_INET_ADDRSTRLEN], b2[NM_UTILS_INET_ADDRSTRLEN]; + char b1[NM_UTILS_INET_ADDRSTRLEN]; + char b2[NM_UTILS_INET_ADDRSTRLEN]; int success; gboolean had_address; @@ -1240,7 +1245,8 @@ nmtstp_link_gre_add (NMPlatform *platform, { const NMPlatformLink *pllink = NULL; gboolean success; - char buffer[INET_ADDRSTRLEN]; + char b1[INET_ADDRSTRLEN]; + char b2[INET_ADDRSTRLEN]; NMLinkType link_type; g_assert (nm_utils_is_valid_iface_name (name, NULL)); @@ -1265,8 +1271,8 @@ nmtstp_link_gre_add (NMPlatform *platform, name, type, dev ?: "", - nm_utils_inet4_ntop (lnk->local, NULL), - nm_utils_inet4_ntop (lnk->remote, buffer), + nm_utils_inet4_ntop (lnk->local, b1), + nm_utils_inet4_ntop (lnk->remote, b2), lnk->ttl, lnk->tos, lnk->path_mtu_discovery ? "pmtudisc" : "nopmtudisc"); @@ -1288,7 +1294,8 @@ nmtstp_link_ip6tnl_add (NMPlatform *platform, { const NMPlatformLink *pllink = NULL; gboolean success; - char buffer[INET6_ADDRSTRLEN]; + char b1[NM_UTILS_INET_ADDRSTRLEN]; + char b2[NM_UTILS_INET_ADDRSTRLEN]; char encap[20]; char tclass[20]; gboolean encap_ignore; @@ -1326,8 +1333,8 @@ nmtstp_link_ip6tnl_add (NMPlatform *platform, name, mode, dev, - nm_utils_inet6_ntop (&lnk->local, NULL), - nm_utils_inet6_ntop (&lnk->remote, buffer), + nm_utils_inet6_ntop (&lnk->local, b1), + nm_utils_inet6_ntop (&lnk->remote, b2), lnk->ttl, tclass_inherit ? "inherit" : nm_sprintf_buf (tclass, "%02x", lnk->tclass), encap_ignore ? "none" : nm_sprintf_buf (encap, "%u", lnk->encap_limit), @@ -1350,7 +1357,8 @@ nmtstp_link_ip6gre_add (NMPlatform *platform, { const NMPlatformLink *pllink = NULL; gboolean success; - char buffer[INET6_ADDRSTRLEN]; + char b1[NM_UTILS_INET_ADDRSTRLEN]; + char b2[NM_UTILS_INET_ADDRSTRLEN]; char tclass[20]; gboolean tclass_inherit; @@ -1373,8 +1381,8 @@ nmtstp_link_ip6gre_add (NMPlatform *platform, name, lnk->is_tap ? "ip6gretap" : "ip6gre", dev, - nm_utils_inet6_ntop (&lnk->local, NULL), - nm_utils_inet6_ntop (&lnk->remote, buffer), + nm_utils_inet6_ntop (&lnk->local, b1), + nm_utils_inet6_ntop (&lnk->remote, b2), lnk->ttl, tclass_inherit ? "inherit" : nm_sprintf_buf (tclass, "%02x", lnk->tclass), lnk->flow_label); @@ -1400,7 +1408,8 @@ nmtstp_link_ipip_add (NMPlatform *platform, { const NMPlatformLink *pllink = NULL; gboolean success; - char buffer[INET_ADDRSTRLEN]; + char b1[INET_ADDRSTRLEN]; + char b2[INET_ADDRSTRLEN]; g_assert (nm_utils_is_valid_iface_name (name, NULL)); @@ -1417,8 +1426,8 @@ nmtstp_link_ipip_add (NMPlatform *platform, success = !nmtstp_run_command ("ip tunnel add %s mode ipip %s local %s remote %s ttl %u tos %02x %s", name, dev, - nm_utils_inet4_ntop (lnk->local, NULL), - nm_utils_inet4_ntop (lnk->remote, buffer), + nm_utils_inet4_ntop (lnk->local, b1), + nm_utils_inet4_ntop (lnk->remote, b2), lnk->ttl, lnk->tos, lnk->path_mtu_discovery ? "pmtudisc" : "nopmtudisc"); @@ -1488,7 +1497,8 @@ nmtstp_link_sit_add (NMPlatform *platform, { const NMPlatformLink *pllink = NULL; gboolean success; - char buffer[INET_ADDRSTRLEN]; + char b1[INET_ADDRSTRLEN]; + char b2[INET_ADDRSTRLEN]; g_assert (nm_utils_is_valid_iface_name (name, NULL)); @@ -1510,8 +1520,8 @@ nmtstp_link_sit_add (NMPlatform *platform, success = !nmtstp_run_command ("ip tunnel add %s mode sit%s local %s remote %s ttl %u tos %02x %s", name, dev, - nm_utils_inet4_ntop (lnk->local, NULL), - nm_utils_inet4_ntop (lnk->remote, buffer), + nm_utils_inet4_ntop (lnk->local, b1), + nm_utils_inet4_ntop (lnk->remote, b2), lnk->ttl, lnk->tos, lnk->path_mtu_discovery ? "pmtudisc" : "nopmtudisc"); @@ -1607,27 +1617,32 @@ nmtstp_link_vxlan_add (NMPlatform *platform, if (external_command) { gs_free char *dev = NULL; - gs_free char *local = NULL, *remote = NULL; + char local[NM_UTILS_INET_ADDRSTRLEN]; + char group[NM_UTILS_INET_ADDRSTRLEN]; if (lnk->parent_ifindex) dev = g_strdup_printf ("dev %s", nm_platform_link_get_name (platform, lnk->parent_ifindex)); if (lnk->local) - local = g_strdup_printf ("%s", nm_utils_inet4_ntop (lnk->local, NULL)); + nm_utils_inet4_ntop (lnk->local, local); else if (memcmp (&lnk->local6, &in6addr_any, sizeof (in6addr_any))) - local = g_strdup_printf ("%s", nm_utils_inet6_ntop (&lnk->local6, NULL)); + nm_utils_inet6_ntop (&lnk->local6, local); + else + local[0] = '\0'; if (lnk->group) - remote = g_strdup_printf ("%s", nm_utils_inet4_ntop (lnk->group, NULL)); + nm_utils_inet4_ntop (lnk->group, group); else if (memcmp (&lnk->group6, &in6addr_any, sizeof (in6addr_any))) - remote = g_strdup_printf ("%s", nm_utils_inet6_ntop (&lnk->group6, NULL)); + nm_utils_inet6_ntop (&lnk->group6, group); + else + group[0] = '\0'; err = nmtstp_run_command ("ip link add %s type vxlan id %u %s local %s group %s ttl %u tos %02x dstport %u srcport %u %u ageing %u", name, lnk->id, dev ?: "", local, - remote, + group, lnk->ttl, lnk->tos, lnk->dst_port, diff --git a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c index c6e79cdb92..6eb99d3b69 100644 --- a/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c +++ b/src/settings/plugins/ifcfg-rh/nms-ifcfg-rh-reader.c @@ -670,7 +670,7 @@ read_full_ip4_address (shvarFile *ifcfg, &has_key, &a, error)) return FALSE; if (has_key) - *out_gateway = g_strdup (nm_utils_inet4_ntop (a, inet_buf)); + *out_gateway = nm_utils_inet4_ntop_dup (a); } /* Prefix */ @@ -1529,7 +1529,6 @@ make_ip4_setting (shvarFile *ifcfg, gboolean never_default; gint64 timeout; int priority; - char inet_buf[NM_UTILS_INET_ADDRSTRLEN]; const char *const *item; guint32 route_table; @@ -1681,7 +1680,7 @@ make_ip4_setting (shvarFile *ifcfg, PARSE_WARNING ("ignoring GATEWAY (/etc/sysconfig/network) for %s " "because the connection has no static addresses", f); } else - gateway = g_strdup (nm_utils_inet4_ntop (a, inet_buf)); + gateway = nm_utils_inet4_ntop_dup (a); } } } diff --git a/src/vpn/nm-vpn-connection.c b/src/vpn/nm-vpn-connection.c index d4f9a5d54b..a4a38f45fe 100644 --- a/src/vpn/nm-vpn-connection.c +++ b/src/vpn/nm-vpn-connection.c @@ -981,15 +981,16 @@ print_vpn_config (NMVpnConnection *self) const NMPlatformIP6Address *address6; char *dns_domain = NULL; guint32 num, i; - char buf[NM_UTILS_INET_ADDRSTRLEN]; + char b1[NM_UTILS_INET_ADDRSTRLEN]; + char b2[NM_UTILS_INET_ADDRSTRLEN]; NMDedupMultiIter ipconf_iter; if (priv->ip4_external_gw) { _LOGI ("Data: VPN Gateway: %s", - nm_utils_inet4_ntop (priv->ip4_external_gw, NULL)); + nm_utils_inet4_ntop (priv->ip4_external_gw, b1)); } else if (priv->ip6_external_gw) { _LOGI ("Data: VPN Gateway: %s", - nm_utils_inet6_ntop (priv->ip6_external_gw, NULL)); + nm_utils_inet6_ntop (priv->ip6_external_gw, b1)); } _LOGI ("Data: Tunnel Device: %s%s%s", NM_PRINT_FMT_QUOTE_STRING (priv->ip_iface)); @@ -1003,22 +1004,22 @@ print_vpn_config (NMVpnConnection *self) nm_assert (address4); if (priv->ip4_internal_gw) - _LOGI ("Data: Internal Gateway: %s", nm_utils_inet4_ntop (priv->ip4_internal_gw, NULL)); - _LOGI ("Data: Internal Address: %s", address4 ? nm_utils_inet4_ntop (address4->address, NULL) : "??"); + _LOGI ("Data: Internal Gateway: %s", nm_utils_inet4_ntop (priv->ip4_internal_gw, b1)); + _LOGI ("Data: Internal Address: %s", address4 ? nm_utils_inet4_ntop (address4->address, b1) : "??"); _LOGI ("Data: Internal Prefix: %d", address4 ? (int) address4->plen : -1); - _LOGI ("Data: Internal Point-to-Point Address: %s", nm_utils_inet4_ntop (address4->peer_address, NULL)); + _LOGI ("Data: Internal Point-to-Point Address: %s", nm_utils_inet4_ntop (address4->peer_address, b1)); nm_ip_config_iter_ip4_route_for_each (&ipconf_iter, priv->ip4_config, &route) { _LOGI ("Data: Static Route: %s/%d Next Hop: %s", - nm_utils_inet4_ntop (route->network, NULL), + nm_utils_inet4_ntop (route->network, b1), route->plen, - nm_utils_inet4_ntop (route->gateway, buf)); + nm_utils_inet4_ntop (route->gateway, b2)); } num = nm_ip4_config_get_num_nameservers (priv->ip4_config); for (i = 0; i < num; i++) { _LOGI ("Data: Internal DNS: %s", - nm_utils_inet4_ntop (nm_ip4_config_get_nameserver (priv->ip4_config, i), NULL)); + nm_utils_inet4_ntop (nm_ip4_config_get_nameserver (priv->ip4_config, i), b1)); } if (nm_ip4_config_get_num_domains (priv->ip4_config) > 0) @@ -1037,22 +1038,22 @@ print_vpn_config (NMVpnConnection *self) nm_assert (address6); if (priv->ip6_internal_gw) - _LOGI ("Data: Internal Gateway: %s", nm_utils_inet6_ntop (priv->ip6_internal_gw, NULL)); - _LOGI ("Data: Internal Address: %s", nm_utils_inet6_ntop (&address6->address, NULL)); + _LOGI ("Data: Internal Gateway: %s", nm_utils_inet6_ntop (priv->ip6_internal_gw, b1)); + _LOGI ("Data: Internal Address: %s", nm_utils_inet6_ntop (&address6->address, b1)); _LOGI ("Data: Internal Prefix: %d", address6->plen); - _LOGI ("Data: Internal Point-to-Point Address: %s", nm_utils_inet6_ntop (&address6->peer_address, NULL)); + _LOGI ("Data: Internal Point-to-Point Address: %s", nm_utils_inet6_ntop (&address6->peer_address, b1)); nm_ip_config_iter_ip6_route_for_each (&ipconf_iter, priv->ip6_config, &route) { _LOGI ("Data: Static Route: %s/%d Next Hop: %s", - nm_utils_inet6_ntop (&route->network, NULL), + nm_utils_inet6_ntop (&route->network, b1), route->plen, - nm_utils_inet6_ntop (&route->gateway, buf)); + nm_utils_inet6_ntop (&route->gateway, b2)); } num = nm_ip6_config_get_num_nameservers (priv->ip6_config); for (i = 0; i < num; i++) { _LOGI ("Data: Internal DNS: %s", - nm_utils_inet6_ntop (nm_ip6_config_get_nameserver (priv->ip6_config, i), NULL)); + nm_utils_inet6_ntop (nm_ip6_config_get_nameserver (priv->ip6_config, i), b1)); } if (nm_ip6_config_get_num_domains (priv->ip6_config) > 0) From b9ebaf3a2b3d61ee9976cb332c2cc0a11aead736 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 26 Nov 2018 17:14:19 +0100 Subject: [PATCH 05/16] libnm: discourage static buffer in nm_utils_inet*_ntop() API nm_utils_inet[46]_ntop() are public API of libnm, so we cannot change it. However, discourage the use of the static buffer. --- libnm-core/nm-utils.c | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/libnm-core/nm-utils.c b/libnm-core/nm-utils.c index 740892c85a..69657c9dce 100644 --- a/libnm-core/nm-utils.c +++ b/libnm-core/nm-utils.c @@ -4500,10 +4500,11 @@ nm_utils_inet_ntop (int addr_family, gconstpointer addr, char *dst) nm_assert_addr_family (addr_family); nm_assert (addr); + nm_assert (dst); s = inet_ntop (addr_family, addr, - dst ?: _nm_utils_inet_ntop_buffer, + dst, addr_family == AF_INET6 ? INET6_ADDRSTRLEN : INET_ADDRSTRLEN); nm_assert (s); return s; @@ -4529,6 +4530,11 @@ nm_utils_inet_ntop (int addr_family, gconstpointer addr, char *dst) const char * nm_utils_inet4_ntop (in_addr_t inaddr, char *dst) { + /* relying on the static buffer (by leaving @dst as %NULL) is discouraged. + * Don't do that! + * + * However, still support it to be lenient against mistakes and because + * this is public API of libnm. */ return inet_ntop (AF_INET, &inaddr, dst ?: _nm_utils_inet_ntop_buffer, INET_ADDRSTRLEN); } @@ -4554,6 +4560,11 @@ nm_utils_inet4_ntop (in_addr_t inaddr, char *dst) const char * nm_utils_inet6_ntop (const struct in6_addr *in6addr, char *dst) { + /* relying on the static buffer (by leaving @dst as %NULL) is discouraged. + * Don't do that! + * + * However, still support it to be lenient against mistakes and because + * this is public API of libnm. */ g_return_val_if_fail (in6addr, NULL); return inet_ntop (AF_INET6, in6addr, dst ?: _nm_utils_inet_ntop_buffer, INET6_ADDRSTRLEN); From 22e276a06b2e9a26031a13d02755e6871c6bff22 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 26 Nov 2018 17:04:10 +0100 Subject: [PATCH 06/16] dhcp: cleanup error paths in bound4_handle() and bound6_handle() - return-early on error - use cleanup attribute --- src/dhcp/nm-dhcp-systemd.c | 41 +++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 20 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index 4e4b9b0940..33c46d81e0 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -490,8 +490,8 @@ bound4_handle (NMDhcpSystemd *self) NMDhcpSystemdPrivate *priv = NM_DHCP_SYSTEMD_GET_PRIVATE (self); const char *iface = nm_dhcp_client_get_iface (NM_DHCP_CLIENT (self)); sd_dhcp_lease *lease; - NMIP4Config *ip4_config; - GHashTable *options; + gs_unref_object NMIP4Config *ip4_config = NULL; + gs_unref_hashtable GHashTable *options = NULL; GError *error = NULL; int r; @@ -505,6 +505,7 @@ bound4_handle (NMDhcpSystemd *self) _LOGD ("lease available"); options = g_hash_table_new_full (nm_str_hash, g_str_equal, NULL, g_free); + ip4_config = lease_to_ip4_config (nm_dhcp_client_get_multi_idx (NM_DHCP_CLIENT (self)), iface, nm_dhcp_client_get_ifindex (NM_DHCP_CLIENT (self)), @@ -514,22 +515,20 @@ bound4_handle (NMDhcpSystemd *self) nm_dhcp_client_get_route_metric (NM_DHCP_CLIENT (self)), TRUE, &error); - if (ip4_config) { - add_requests_to_options (options, dhcp4_requests); - dhcp_lease_save (lease, priv->lease_file); - - nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), - NM_DHCP_STATE_BOUND, - NM_IP_CONFIG_CAST (ip4_config), - options); - } else { + if (!ip4_config) { _LOGW ("%s", error->message); - nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), NM_DHCP_STATE_FAIL, NULL, NULL); g_clear_error (&error); + nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), NM_DHCP_STATE_FAIL, NULL, NULL); + return; } - g_hash_table_destroy (options); - g_clear_object (&ip4_config); + add_requests_to_options (options, dhcp4_requests); + dhcp_lease_save (lease, priv->lease_file); + + nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), + NM_DHCP_STATE_BOUND, + NM_IP_CONFIG_CAST (ip4_config), + options); } static void @@ -825,6 +824,7 @@ bound6_handle (NMDhcpSystemd *self) _LOGD ("lease available"); options = g_hash_table_new_full (nm_str_hash, g_str_equal, NULL, g_free); + ip6_config = lease_to_ip6_config (nm_dhcp_client_get_multi_idx (NM_DHCP_CLIENT (self)), iface, nm_dhcp_client_get_ifindex (NM_DHCP_CLIENT (self)), @@ -834,15 +834,16 @@ bound6_handle (NMDhcpSystemd *self) nm_dhcp_client_get_info_only (NM_DHCP_CLIENT (self)), &error); - if (ip6_config) { - nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), - NM_DHCP_STATE_BOUND, - NM_IP_CONFIG_CAST (ip6_config), - options); - } else { + if (!ip6_config) { _LOGW ("%s", error->message); nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), NM_DHCP_STATE_FAIL, NULL, NULL); + return; } + + nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), + NM_DHCP_STATE_BOUND, + NM_IP_CONFIG_CAST (ip6_config), + options); } static void From fed16ff1cbf692dc00f4a08343b3b9689751a010 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 14 Dec 2018 16:06:20 +0100 Subject: [PATCH 07/16] dhcp: don't request DHCP6 client-id option with internal client sd_dhcp6_client_set_request_option() only accepts a white-listed set of options. Unexpected options are rejected with -EINVAL. Currently supported are only: - SD_DHCP6_OPTION_DNS_SERVERS - SD_DHCP6_OPTION_DOMAIN_LIST - SD_DHCP6_OPTION_SNTP_SERVERS - SD_DHCP6_OPTION_NTP_SERVER - SD_DHCP6_OPTION_RAPID_COMMIT As such, SD_DHCP6_OPTION_CLIENTID is not accepted and requesting it was silently ignored. Fixes: d2dd3b2c90221fdfa40ca81a9fcffe6a777d95de --- src/dhcp/nm-dhcp-dhclient-utils.c | 2 ++ src/dhcp/nm-dhcp-systemd.c | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/src/dhcp/nm-dhcp-dhclient-utils.c b/src/dhcp/nm-dhcp-dhclient-utils.c index be8d06d988..d6da3f5c1e 100644 --- a/src/dhcp/nm-dhcp-dhclient-utils.c +++ b/src/dhcp/nm-dhcp-dhclient-utils.c @@ -427,6 +427,8 @@ nm_dhcp_dhclient_create_config (const char *interface, add_hostname6 (new_contents, hostname); add_request (reqs, "dhcp6.name-servers"); add_request (reqs, "dhcp6.domain-search"); + + /* FIXME: internal client does not support requesting client-id option. Does this even work? */ add_request (reqs, "dhcp6.client-id"); } diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index 33c46d81e0..92152bba99 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -133,7 +133,7 @@ static const ReqOption dhcp4_requests[] = { }; static const ReqOption dhcp6_requests[] = { - { SD_DHCP6_OPTION_CLIENTID, REQPREFIX "dhcp6_client_id", TRUE }, + { SD_DHCP6_OPTION_CLIENTID, REQPREFIX "dhcp6_client_id", FALSE }, /* Don't request server ID by default; some servers don't reply to * Information Requests that request the Server ID. From a057d8c3fa04f812ebbb1d1e24c1218e2c4c0627 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 26 Nov 2018 16:49:13 +0100 Subject: [PATCH 08/16] dhcp: cleanup static option list for internal client - use proper data types "guint16" and "bool" in static option list. It saves a few bytes, but also it's the appropriate type. Well, at least, it's the appropriate type for DHCPv6, not for DHCPv4 (which is guint8). - assert against failure of sd_dhcp_client_set_request_option() and sd_dhcp6_client_set_request_option(). --- src/dhcp/nm-dhcp-systemd.c | 126 ++++++++++++++++++++----------------- 1 file changed, 70 insertions(+), 56 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index 92152bba99..e31a526366 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -98,63 +98,72 @@ G_DEFINE_TYPE (NMDhcpSystemd, nm_dhcp_systemd, NM_TYPE_DHCP_CLIENT) #define DHCP6_OPTION_IAID 1034 typedef struct { - guint num; const char *name; - gboolean include; + uint16_t option_num; + bool include; } ReqOption; #define REQPREFIX "requested_" +#define REQ(_num, _name, _include) \ + { \ + .name = REQPREFIX""_name, \ + .option_num = _num, \ + .include = _include, \ + } + static const ReqOption dhcp4_requests[] = { - { SD_DHCP_OPTION_SUBNET_MASK, REQPREFIX "subnet_mask", TRUE }, - { SD_DHCP_OPTION_TIME_OFFSET, REQPREFIX "time_offset", TRUE }, - { SD_DHCP_OPTION_ROUTER, REQPREFIX "routers", TRUE }, - { SD_DHCP_OPTION_DOMAIN_NAME_SERVER, REQPREFIX "domain_name_servers", TRUE }, - { SD_DHCP_OPTION_HOST_NAME, REQPREFIX "host_name", TRUE }, - { SD_DHCP_OPTION_DOMAIN_NAME, REQPREFIX "domain_name", TRUE }, - { SD_DHCP_OPTION_INTERFACE_MTU, REQPREFIX "interface_mtu", TRUE }, - { SD_DHCP_OPTION_BROADCAST, REQPREFIX "broadcast_address", TRUE }, - { SD_DHCP_OPTION_STATIC_ROUTE, REQPREFIX "static_routes", TRUE }, - { DHCP_OPTION_NIS_DOMAIN, REQPREFIX "nis_domain", TRUE }, - { DHCP_OPTION_NIS_SERVERS, REQPREFIX "nis_servers", TRUE }, - { SD_DHCP_OPTION_NTP_SERVER, REQPREFIX "ntp_servers", TRUE }, - { SD_DHCP_OPTION_SERVER_IDENTIFIER, REQPREFIX "dhcp_server_identifier", TRUE }, - { SD_DHCP_OPTION_DOMAIN_SEARCH_LIST, REQPREFIX "domain_search", TRUE }, - { SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, REQPREFIX "rfc3442_classless_static_routes", TRUE }, - { SD_DHCP_OPTION_PRIVATE_CLASSLESS_STATIC_ROUTE, REQPREFIX "ms_classless_static_routes", TRUE }, - { SD_DHCP_OPTION_PRIVATE_PROXY_AUTODISCOVERY, REQPREFIX "wpad", TRUE }, - { SD_DHCP_OPTION_ROOT_PATH, REQPREFIX "root_path", TRUE }, + REQ (SD_DHCP_OPTION_SUBNET_MASK, "subnet_mask", TRUE ), + REQ (SD_DHCP_OPTION_TIME_OFFSET, "time_offset", TRUE ), + REQ (SD_DHCP_OPTION_ROUTER, "routers", TRUE ), + REQ (SD_DHCP_OPTION_DOMAIN_NAME_SERVER, "domain_name_servers", TRUE ), + REQ (SD_DHCP_OPTION_HOST_NAME, "host_name", TRUE ), + REQ (SD_DHCP_OPTION_DOMAIN_NAME, "domain_name", TRUE ), + REQ (SD_DHCP_OPTION_INTERFACE_MTU, "interface_mtu", TRUE ), + REQ (SD_DHCP_OPTION_BROADCAST, "broadcast_address", TRUE ), + REQ (SD_DHCP_OPTION_STATIC_ROUTE, "static_routes", TRUE ), + REQ (DHCP_OPTION_NIS_DOMAIN, "nis_domain", TRUE ), + REQ (DHCP_OPTION_NIS_SERVERS, "nis_servers", TRUE ), + REQ (SD_DHCP_OPTION_NTP_SERVER, "ntp_servers", TRUE ), + REQ (SD_DHCP_OPTION_SERVER_IDENTIFIER, "dhcp_server_identifier", TRUE ), + REQ (SD_DHCP_OPTION_DOMAIN_SEARCH_LIST, "domain_search", TRUE ), + REQ (SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, "rfc3442_classless_static_routes", TRUE ), + REQ (SD_DHCP_OPTION_PRIVATE_CLASSLESS_STATIC_ROUTE, "ms_classless_static_routes", TRUE ), + REQ (SD_DHCP_OPTION_PRIVATE_PROXY_AUTODISCOVERY, "wpad", TRUE ), + REQ (SD_DHCP_OPTION_ROOT_PATH, "root_path", TRUE ), /* Internal values */ - { SD_DHCP_OPTION_IP_ADDRESS_LEASE_TIME, REQPREFIX "expiry", FALSE }, - { SD_DHCP_OPTION_CLIENT_IDENTIFIER, REQPREFIX "dhcp_client_identifier", FALSE }, - { DHCP_OPTION_IP_ADDRESS, REQPREFIX "ip_address", FALSE }, - { 0, NULL, FALSE } + REQ (SD_DHCP_OPTION_IP_ADDRESS_LEASE_TIME, "expiry", FALSE ), + REQ (SD_DHCP_OPTION_CLIENT_IDENTIFIER, "dhcp_client_identifier", FALSE ), + REQ (DHCP_OPTION_IP_ADDRESS, "ip_address", FALSE ), + + { 0 } }; static const ReqOption dhcp6_requests[] = { - { SD_DHCP6_OPTION_CLIENTID, REQPREFIX "dhcp6_client_id", FALSE }, + REQ (SD_DHCP6_OPTION_CLIENTID, "dhcp6_client_id", FALSE ), /* Don't request server ID by default; some servers don't reply to * Information Requests that request the Server ID. */ - { SD_DHCP6_OPTION_SERVERID, REQPREFIX "dhcp6_server_id", FALSE }, + REQ (SD_DHCP6_OPTION_SERVERID, "dhcp6_server_id", FALSE ), - { SD_DHCP6_OPTION_DNS_SERVERS, REQPREFIX "dhcp6_name_servers", TRUE }, - { SD_DHCP6_OPTION_DOMAIN_LIST, REQPREFIX "dhcp6_domain_search", TRUE }, - { SD_DHCP6_OPTION_SNTP_SERVERS, REQPREFIX "dhcp6_sntp_servers", TRUE }, + REQ (SD_DHCP6_OPTION_DNS_SERVERS, "dhcp6_name_servers", TRUE ), + REQ (SD_DHCP6_OPTION_DOMAIN_LIST, "dhcp6_domain_search", TRUE ), + REQ (SD_DHCP6_OPTION_SNTP_SERVERS, "dhcp6_sntp_servers", TRUE ), /* Internal values */ - { DHCP6_OPTION_IP_ADDRESS, REQPREFIX "ip6_address", FALSE }, - { DHCP6_OPTION_PREFIXLEN, REQPREFIX "ip6_prefixlen", FALSE }, - { DHCP6_OPTION_PREFERRED_LIFE, REQPREFIX "preferred_life", FALSE }, - { DHCP6_OPTION_MAX_LIFE, REQPREFIX "max_life", FALSE }, - { DHCP6_OPTION_STARTS, REQPREFIX "starts", FALSE }, - { DHCP6_OPTION_LIFE_STARTS, REQPREFIX "life_starts", FALSE }, - { DHCP6_OPTION_RENEW, REQPREFIX "renew", FALSE }, - { DHCP6_OPTION_REBIND, REQPREFIX "rebind", FALSE }, - { DHCP6_OPTION_IAID, REQPREFIX "iaid", FALSE }, - { 0, NULL, FALSE } + REQ (DHCP6_OPTION_IP_ADDRESS, "ip6_address", FALSE ), + REQ (DHCP6_OPTION_PREFIXLEN, "ip6_prefixlen", FALSE ), + REQ (DHCP6_OPTION_PREFERRED_LIFE, "preferred_life", FALSE ), + REQ (DHCP6_OPTION_MAX_LIFE, "max_life", FALSE ), + REQ (DHCP6_OPTION_STARTS, "starts", FALSE ), + REQ (DHCP6_OPTION_LIFE_STARTS, "life_starts", FALSE ), + REQ (DHCP6_OPTION_RENEW, "renew", FALSE ), + REQ (DHCP6_OPTION_REBIND, "rebind", FALSE ), + REQ (DHCP6_OPTION_IAID, "iaid", FALSE ), + + { 0 } }; static void @@ -165,18 +174,22 @@ take_option (GHashTable *options, { guint i; - g_return_if_fail (value != NULL); + nm_assert (options); + nm_assert (requests); + nm_assert (value); for (i = 0; requests[i].name; i++) { - if (requests[i].num == option) { + nm_assert (g_str_has_prefix (requests[i].name, REQPREFIX)); + if (requests[i].option_num == option) { g_hash_table_insert (options, (gpointer) (requests[i].name + NM_STRLEN (REQPREFIX)), value); - break; + return; } } + /* Option should always be found */ - g_assert (requests[i].name); + nm_assert_not_reached (); } static void @@ -186,13 +199,6 @@ add_option (GHashTable *options, const ReqOption *requests, guint option, const take_option (options, requests, option, g_strdup (value)); } -static void -add_option_u32 (GHashTable *options, const ReqOption *requests, guint option, guint32 value) -{ - if (options) - take_option (options, requests, option, g_strdup_printf ("%u", value)); -} - static void add_option_u64 (GHashTable *options, const ReqOption *requests, guint option, guint64 value) { @@ -205,7 +211,10 @@ add_requests_to_options (GHashTable *options, const ReqOption *requests) { guint i; - for (i = 0; options && requests[i].name; i++) { + if (!options) + return; + + for (i = 0; requests[i].name; i++) { if (requests[i].include) g_hash_table_insert (options, (gpointer) requests[i].name, g_strdup ("1")); } @@ -420,7 +429,7 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, r = sd_dhcp_lease_get_mtu (lease, &mtu); if (r == 0 && mtu) { nm_ip4_config_set_mtu (ip4_config, mtu, NM_IP_CONFIG_SOURCE_DHCP); - add_option_u32 (options, dhcp4_requests, SD_DHCP_OPTION_INTERFACE_MTU, mtu); + add_option_u64 (options, dhcp4_requests, SD_DHCP_OPTION_INTERFACE_MTU, mtu); LOG_LEASE (LOGD_DHCP4, "mtu %u", mtu); } @@ -671,8 +680,11 @@ ip4_start (NMDhcpClient *client, /* Add requested options */ for (i = 0; dhcp4_requests[i].name; i++) { - if (dhcp4_requests[i].include) - sd_dhcp_client_set_request_option (sd_client, dhcp4_requests[i].num); + if (dhcp4_requests[i].include) { + nm_assert (dhcp4_requests[i].option_num <= 255); + r = sd_dhcp_client_set_request_option (sd_client, dhcp4_requests[i].option_num); + nm_assert (r >= 0 || r == -EEXIST); + } } hostname = nm_dhcp_client_get_hostname (client); @@ -973,8 +985,10 @@ ip6_start (NMDhcpClient *client, /* Add requested options */ for (i = 0; dhcp6_requests[i].name; i++) { - if (dhcp6_requests[i].include) - sd_dhcp6_client_set_request_option (sd_client, dhcp6_requests[i].num); + if (dhcp6_requests[i].include) { + r = sd_dhcp6_client_set_request_option (sd_client, dhcp6_requests[i].option_num); + nm_assert (r >= 0 || r == -EEXIST); + } } r = sd_dhcp6_client_set_local_address (sd_client, ll_addr); From d11572ac42630d476058448fb988359443ddc0f0 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 27 Nov 2018 12:59:33 +0100 Subject: [PATCH 09/16] dhcp: fix signedness of loop variable in lease_to_ip4_config() The loop variable should have the same type as the variable that holds the number of elements ("num", in this case). --- src/dhcp/nm-dhcp-systemd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index e31a526366..bc90db0c64 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -244,13 +244,13 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, char addr_str[NM_UTILS_INET_ADDRSTRLEN]; char addr_str2[NM_UTILS_INET_ADDRSTRLEN]; const char *s; - guint32 lifetime = 0, i; + guint32 lifetime = 0; NMPlatformIP4Address address; nm_auto_free_gstring GString *str = NULL; gs_free sd_dhcp_route **routes = NULL; const char *const*search_domains = NULL; guint16 mtu; - int r, num; + int r, i, num; guint64 end_time; const void *data; gsize data_len; From 4aa7285dc2a6a7a757ba0ea2d7211aab486b3cfe Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 27 Nov 2018 13:47:08 +0100 Subject: [PATCH 10/16] dhcp: let lease_to_ip4_config() allocate option hash lease_to_ip4_config() can fail, if the lease is broken. As such, a function that fails should not modifiy an in/out parameter. Avoid that, by not having the caller pre-allocate the options hash, but instead allocate it by the lease_to_ip*_config() functions, and return it only on success. --- src/dhcp/nm-dhcp-systemd.c | 42 +++++++++++++++++++++++--------------- 1 file changed, 26 insertions(+), 16 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index bc90db0c64..a35ff0697e 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -220,6 +220,12 @@ add_requests_to_options (GHashTable *options, const ReqOption *requests) } } +static GHashTable * +create_options_dict (void) +{ + return g_hash_table_new_full (nm_str_hash, g_str_equal, NULL, g_free); +} + #define LOG_LEASE(domain, ...) \ G_STMT_START { \ if (log_lease) { \ @@ -232,13 +238,14 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, const char *iface, int ifindex, sd_dhcp_lease *lease, - GHashTable *options, guint32 route_table, guint32 route_metric, gboolean log_lease, + GHashTable **out_options, GError **error) { - NMIP4Config *ip4_config = NULL; + gs_unref_object NMIP4Config *ip4_config = NULL; + gs_unref_hashtable GHashTable *options = NULL; struct in_addr tmp_addr; const struct in_addr *addr_list; char addr_str[NM_UTILS_INET_ADDRSTRLEN]; @@ -263,6 +270,8 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, ip4_config = nm_ip4_config_new (multi_idx, ifindex); + options = out_options ? create_options_dict () : NULL; + /* Address */ sd_dhcp_lease_get_address (lease, &tmp_addr); memset (&address, 0, sizeof (address)); @@ -457,7 +466,8 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, metered = !!memmem (data, data_len, "ANDROID_METERED", NM_STRLEN ("ANDROID_METERED")); nm_ip4_config_set_metered (ip4_config, metered); - return ip4_config; + NM_SET_OUT (out_options, g_steal_pointer (&options)); + return g_steal_pointer (&ip4_config); } /*****************************************************************************/ @@ -513,16 +523,14 @@ bound4_handle (NMDhcpSystemd *self) _LOGD ("lease available"); - options = g_hash_table_new_full (nm_str_hash, g_str_equal, NULL, g_free); - ip4_config = lease_to_ip4_config (nm_dhcp_client_get_multi_idx (NM_DHCP_CLIENT (self)), iface, nm_dhcp_client_get_ifindex (NM_DHCP_CLIENT (self)), lease, - options, nm_dhcp_client_get_route_table (NM_DHCP_CLIENT (self)), nm_dhcp_client_get_route_metric (NM_DHCP_CLIENT (self)), TRUE, + &options, &error); if (!ip4_config) { _LOGW ("%s", error->message); @@ -730,23 +738,26 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, const char *iface, int ifindex, sd_dhcp6_lease *lease, - GHashTable *options, gboolean log_lease, gboolean info_only, + GHashTable **out_options, GError **error) { + gs_unref_object NMIP6Config *ip6_config = NULL; + gs_unref_hashtable GHashTable *options = NULL; struct in6_addr tmp_addr, *dns; uint32_t lft_pref, lft_valid; - NMIP6Config *ip6_config; char addr_str[NM_UTILS_INET_ADDRSTRLEN]; char **domains; nm_auto_free_gstring GString *str = NULL; int num, i; - gint32 ts; + const gint32 ts = nm_utils_get_monotonic_timestamp_s (); g_return_val_if_fail (lease, NULL); + ip6_config = nm_ip6_config_new (multi_idx, ifindex); - ts = nm_utils_get_monotonic_timestamp_s (); + + options = out_options ? create_options_dict () : NULL; /* Addresses */ sd_dhcp6_lease_reset_address_iter (lease); @@ -776,8 +787,8 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, if (str->len) add_option (options, dhcp6_requests, DHCP6_OPTION_IP_ADDRESS, str->str); - if (!info_only && nm_ip6_config_get_num_addresses (ip6_config) == 0) { - g_object_unref (ip6_config); + if ( !info_only + && nm_ip6_config_get_num_addresses (ip6_config) == 0) { g_set_error_literal (error, NM_MANAGER_ERROR, NM_MANAGER_ERROR_FAILED, @@ -812,7 +823,8 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, add_option (options, dhcp6_requests, SD_DHCP6_OPTION_DOMAIN_LIST, str->str); } - return ip6_config; + NM_SET_OUT (out_options, g_steal_pointer (&options)); + return g_steal_pointer (&ip6_config); } static void @@ -835,15 +847,13 @@ bound6_handle (NMDhcpSystemd *self) _LOGD ("lease available"); - options = g_hash_table_new_full (nm_str_hash, g_str_equal, NULL, g_free); - ip6_config = lease_to_ip6_config (nm_dhcp_client_get_multi_idx (NM_DHCP_CLIENT (self)), iface, nm_dhcp_client_get_ifindex (NM_DHCP_CLIENT (self)), lease, - options, TRUE, nm_dhcp_client_get_info_only (NM_DHCP_CLIENT (self)), + &options, &error); if (!ip6_config) { From 3f99d01c1a90bdaea16b256711eb7f87a98da499 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 27 Nov 2018 13:47:13 +0100 Subject: [PATCH 11/16] dhcp: cleanup parsing of DHCP lease for internal client - check errors when accessing the lease. Some errors, like a failure from sd_dhcp_lease_get_address() are fatal. - while parsing the individual options, don't incrementally build the NMPlatformIP4Address instance (and similar). Instead, parse the options to individual variales, and only package them as platform structure at the point where they are needed. It makes the code simpler, because all individual bits (like "r_plen" variable) are only initialized and used at one place. That is clearer than incrementally building a platform structure, where you have to follow the code to see how the structure mutates. - drop redundant comments that only serve as a visual separator for structuring the code. Instead, structure the code. --- src/dhcp/nm-dhcp-systemd.c | 213 +++++++++++++++++++------------------ 1 file changed, 107 insertions(+), 106 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index a35ff0697e..4e1ef7b80a 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -246,25 +246,28 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, { gs_unref_object NMIP4Config *ip4_config = NULL; gs_unref_hashtable GHashTable *options = NULL; - struct in_addr tmp_addr; const struct in_addr *addr_list; char addr_str[NM_UTILS_INET_ADDRSTRLEN]; char addr_str2[NM_UTILS_INET_ADDRSTRLEN]; const char *s; - guint32 lifetime = 0; - NMPlatformIP4Address address; nm_auto_free_gstring GString *str = NULL; gs_free sd_dhcp_route **routes = NULL; const char *const*search_domains = NULL; guint16 mtu; - int r, i, num; - guint64 end_time; + int i, num; const void *data; gsize data_len; gboolean metered = FALSE; gboolean static_default_gateway = FALSE; gboolean gateway_has = FALSE; in_addr_t gateway = 0; + const gint32 ts = nm_utils_get_monotonic_timestamp_s (); + gint64 ts_time = time (NULL); + struct in_addr a_address; + struct in_addr a_netmask; + struct in_addr a_router; + guint32 a_plen; + guint32 a_lifetime; g_return_val_if_fail (lease != NULL, NULL); @@ -272,39 +275,49 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, options = out_options ? create_options_dict () : NULL; - /* Address */ - sd_dhcp_lease_get_address (lease, &tmp_addr); - memset (&address, 0, sizeof (address)); - address.address = tmp_addr.s_addr; - address.peer_address = tmp_addr.s_addr; - nm_utils_inet4_ntop (tmp_addr.s_addr, addr_str); + if (sd_dhcp_lease_get_address (lease, &a_address) < 0) { + nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "could not get address from lease"); + return NULL; + } + nm_utils_inet4_ntop (a_address.s_addr, addr_str); LOG_LEASE (LOGD_DHCP4, "address %s", addr_str); add_option (options, dhcp4_requests, DHCP_OPTION_IP_ADDRESS, addr_str); - /* Prefix/netmask */ - sd_dhcp_lease_get_netmask (lease, &tmp_addr); - address.plen = nm_utils_ip4_netmask_to_prefix (tmp_addr.s_addr); - LOG_LEASE (LOGD_DHCP4, "plen %d", address.plen); + if (sd_dhcp_lease_get_netmask (lease, &a_netmask) < 0) { + nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "could not get netmask from lease"); + return NULL; + } + a_plen = nm_utils_ip4_netmask_to_prefix (a_netmask.s_addr); + LOG_LEASE (LOGD_DHCP4, "plen %u", (guint) a_plen); add_option (options, dhcp4_requests, SD_DHCP_OPTION_SUBNET_MASK, - nm_utils_inet4_ntop (tmp_addr.s_addr, addr_str)); + nm_utils_inet4_ntop (a_netmask.s_addr, addr_str)); - /* Lease time */ - sd_dhcp_lease_get_lifetime (lease, &lifetime); - address.timestamp = nm_utils_get_monotonic_timestamp_s (); - address.lifetime = address.preferred = lifetime; - end_time = (guint64) time (NULL) + lifetime; - LOG_LEASE (LOGD_DHCP4, "expires in %" G_GUINT32_FORMAT " seconds", lifetime); + if (sd_dhcp_lease_get_lifetime (lease, &a_lifetime) < 0) { + nm_utils_error_set_literal (error, NM_UTILS_ERROR_UNKNOWN, "could not get lifetime from lease"); + return NULL; + } + LOG_LEASE (LOGD_DHCP4, "expires in %u seconds (at %lld)", + (guint) a_lifetime, + (long long) (ts_time + a_lifetime)); add_option_u64 (options, dhcp4_requests, SD_DHCP_OPTION_IP_ADDRESS_LEASE_TIME, - end_time); + (guint64) (ts_time + a_lifetime)); - address.addr_source = NM_IP_CONFIG_SOURCE_DHCP; - nm_ip4_config_add_address (ip4_config, &address); + // TODO: ensure a_plen of zero is handled correctly. + nm_ip4_config_add_address (ip4_config, + &((const NMPlatformIP4Address) { + .address = a_address.s_addr, + .peer_address = a_address.s_addr, + .plen = a_plen, + .addr_source = NM_IP_CONFIG_SOURCE_DHCP, + .timestamp = ts, + .lifetime = a_lifetime, + .preferred = a_lifetime, + })); - /* DNS Servers */ num = sd_dhcp_lease_get_dns (lease, &addr_list); if (num > 0) { nm_gstring_prepare (&str); @@ -320,7 +333,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, add_option (options, dhcp4_requests, SD_DHCP_OPTION_DOMAIN_NAME_SERVER, str->str); } - /* Search domains */ num = sd_dhcp_lease_get_search_domains (lease, (char ***) &search_domains); if (num > 0) { nm_gstring_prepare (&str); @@ -332,71 +344,73 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, add_option (options, dhcp4_requests, SD_DHCP_OPTION_DOMAIN_SEARCH_LIST, str->str); } - /* Domain Name */ - r = sd_dhcp_lease_get_domainname (lease, &s); - if (r == 0) { - /* Multiple domains sometimes stuffed into option 15 "Domain Name". - * As systemd escapes such characters, split them at \\032. */ - char **domains = g_strsplit (s, "\\032", 0); + if ( sd_dhcp_lease_get_domainname (lease, &s) >= 0 + && s) { + gs_strfreev char **domains = NULL; char **d; + /* Multiple domains sometimes stuffed into option 15 "Domain Name". + * As systemd escapes such characters, split them at \\032. */ + domains = g_strsplit (s, "\\032", 0); for (d = domains; *d; d++) { LOG_LEASE (LOGD_DHCP4, "domain name '%s'", *d); nm_ip4_config_add_domain (ip4_config, *d); } - g_strfreev (domains); add_option (options, dhcp4_requests, SD_DHCP_OPTION_DOMAIN_NAME, s); } - /* Hostname */ - r = sd_dhcp_lease_get_hostname (lease, &s); - if (r == 0) { + if (sd_dhcp_lease_get_hostname (lease, &s) >= 0) { LOG_LEASE (LOGD_DHCP4, "hostname '%s'", s); add_option (options, dhcp4_requests, SD_DHCP_OPTION_HOST_NAME, s); } - /* Routes */ num = sd_dhcp_lease_get_routes (lease, &routes); if (num > 0) { nm_gstring_prepare (&str); for (i = 0; i < num; i++) { - NMPlatformIP4Route route = { 0 }; const char *gw_str; - guint8 plen; - struct in_addr a; + guint8 r_plen; + struct in_addr r_network; + struct in_addr r_gateway; - if (sd_dhcp_route_get_destination (routes[i], &a) < 0) + if (sd_dhcp_route_get_destination (routes[i], &r_network) < 0) + continue; + if ( sd_dhcp_route_get_destination_prefix_length (routes[i], &r_plen) < 0 + || r_plen > 32) + continue; + if (sd_dhcp_route_get_gateway (routes[i], &r_gateway) < 0) continue; - if ( sd_dhcp_route_get_destination_prefix_length (routes[i], &plen) < 0 - || plen > 32) - continue; + if (r_plen > 0) { + const in_addr_t network_net = nm_utils_ip4_address_clear_host_address (r_network.s_addr, + r_plen); - route.plen = plen; - route.network = nm_utils_ip4_address_clear_host_address (a.s_addr, plen); + nm_ip4_config_add_route (ip4_config, + &((const NMPlatformIP4Route) { + .network = network_net, + .plen = r_plen, + .gateway = r_gateway.s_addr, + .rt_source = NM_IP_CONFIG_SOURCE_DHCP, + .metric = route_metric, + .table_coerced = nm_platform_route_table_coerce (route_table), + }), + NULL); - if (sd_dhcp_route_get_gateway (routes[i], &a) < 0) - continue; - route.gateway = a.s_addr; - - if (route.plen) { - route.rt_source = NM_IP_CONFIG_SOURCE_DHCP; - route.metric = route_metric; - route.table_coerced = nm_platform_route_table_coerce (route_table); - nm_ip4_config_add_route (ip4_config, &route, NULL); - - s = nm_utils_inet4_ntop (route.network, addr_str); - gw_str = nm_utils_inet4_ntop (route.gateway, addr_str2); - LOG_LEASE (LOGD_DHCP4, "static route %s/%d gw %s", s, route.plen, gw_str); - - g_string_append_printf (str, "%s%s/%d %s", str->len ? " " : "", s, route.plen, gw_str); + s = nm_utils_inet4_ntop (network_net, addr_str); + gw_str = nm_utils_inet4_ntop (r_gateway.s_addr, addr_str2); + LOG_LEASE (LOGD_DHCP4, "static route %s/%d gw %s", s, (int) r_plen, gw_str); + g_string_append_printf (str, "%s%s/%d %s", + str->len ? " " : "", + s, + (int) r_plen, + gw_str); } else { if (!static_default_gateway) { static_default_gateway = TRUE; gateway_has = TRUE; - gateway = route.gateway; + gateway = r_gateway.s_addr; - s = nm_utils_inet4_ntop (route.gateway, addr_str); + s = nm_utils_inet4_ntop (gateway, addr_str); LOG_LEASE (LOGD_DHCP4, "gateway %s", s); add_option (options, dhcp4_requests, SD_DHCP_OPTION_ROUTER, s); } @@ -411,38 +425,33 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, * Be more lenient and ignore the Router option only if Classless Static * Routes contain a default gateway (as other DHCP backends do). */ - /* Gateway */ - if (!static_default_gateway) { - r = sd_dhcp_lease_get_router (lease, &tmp_addr); - if (r == 0) { - gateway_has = TRUE; - gateway = tmp_addr.s_addr; - s = nm_utils_inet4_ntop (tmp_addr.s_addr, addr_str); - LOG_LEASE (LOGD_DHCP4, "gateway %s", s); - add_option (options, dhcp4_requests, SD_DHCP_OPTION_ROUTER, s); - } + if ( !static_default_gateway + && sd_dhcp_lease_get_router (lease, &a_router) >= 0) { + gateway_has = TRUE; + gateway = a_router.s_addr; + s = nm_utils_inet4_ntop (a_router.s_addr, addr_str); + LOG_LEASE (LOGD_DHCP4, "gateway %s", s); + add_option (options, dhcp4_requests, SD_DHCP_OPTION_ROUTER, s); } if (gateway_has) { - const NMPlatformIP4Route rt = { - .rt_source = NM_IP_CONFIG_SOURCE_DHCP, - .gateway = gateway, - .table_coerced = nm_platform_route_table_coerce (route_table), - .metric = route_metric, - }; - - nm_ip4_config_add_route (ip4_config, &rt, NULL); + nm_ip4_config_add_route (ip4_config, + &((const NMPlatformIP4Route) { + .rt_source = NM_IP_CONFIG_SOURCE_DHCP, + .gateway = gateway, + .table_coerced = nm_platform_route_table_coerce (route_table), + .metric = route_metric, + }), + NULL); } - /* MTU */ - r = sd_dhcp_lease_get_mtu (lease, &mtu); - if (r == 0 && mtu) { + if ( sd_dhcp_lease_get_mtu (lease, &mtu) >= 0 + && mtu) { nm_ip4_config_set_mtu (ip4_config, mtu, NM_IP_CONFIG_SOURCE_DHCP); add_option_u64 (options, dhcp4_requests, SD_DHCP_OPTION_INTERFACE_MTU, mtu); LOG_LEASE (LOGD_DHCP4, "mtu %u", mtu); } - /* NTP servers */ num = sd_dhcp_lease_get_ntp (lease, &addr_list); if (num > 0) { nm_gstring_prepare (&str); @@ -454,15 +463,12 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, add_option (options, dhcp4_requests, SD_DHCP_OPTION_NTP_SERVER, str->str); } - /* Root path */ - r = sd_dhcp_lease_get_root_path (lease, &s); - if (r >= 0) { + if (sd_dhcp_lease_get_root_path (lease, &s) >= 0) { LOG_LEASE (LOGD_DHCP4, "root path '%s'", s); add_option (options, dhcp4_requests, SD_DHCP_OPTION_ROOT_PATH, s); } - r = sd_dhcp_lease_get_vendor_specific (lease, &data, &data_len); - if (r >= 0) + if (sd_dhcp_lease_get_vendor_specific (lease, &data, &data_len) >= 0) metered = !!memmem (data, data_len, "ANDROID_METERED", NM_STRLEN ("ANDROID_METERED")); nm_ip4_config_set_metered (ip4_config, metered); @@ -512,10 +518,9 @@ bound4_handle (NMDhcpSystemd *self) gs_unref_object NMIP4Config *ip4_config = NULL; gs_unref_hashtable GHashTable *options = NULL; GError *error = NULL; - int r; - r = sd_dhcp_client_get_lease (priv->client4, &lease); - if (r < 0 || !lease) { + if ( sd_dhcp_client_get_lease (priv->client4, &lease) < 0 + || !lease) { _LOGW ("no lease!"); nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), NM_DHCP_STATE_FAIL, NULL, NULL); return; @@ -759,16 +764,15 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, options = out_options ? create_options_dict () : NULL; - /* Addresses */ sd_dhcp6_lease_reset_address_iter (lease); nm_gstring_prepare (&str); while (sd_dhcp6_lease_get_address (lease, &tmp_addr, &lft_pref, &lft_valid) >= 0) { - NMPlatformIP6Address address = { - .plen = 128, - .address = tmp_addr, - .timestamp = ts, - .lifetime = lft_valid, - .preferred = lft_pref, + const NMPlatformIP6Address address = { + .plen = 128, + .address = tmp_addr, + .timestamp = ts, + .lifetime = lft_valid, + .preferred = lft_pref, .addr_source = NM_IP_CONFIG_SOURCE_DHCP, }; @@ -796,7 +800,6 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, return NULL; } - /* DNS servers */ num = sd_dhcp6_lease_get_dns (lease, &dns); if (num > 0) { nm_gstring_prepare (&str); @@ -811,7 +814,6 @@ lease_to_ip6_config (NMDedupMultiIndex *multi_idx, add_option (options, dhcp6_requests, SD_DHCP6_OPTION_DNS_SERVERS, str->str); } - /* Search domains */ num = sd_dhcp6_lease_get_domains (lease, &domains); if (num > 0) { nm_gstring_prepare (&str); @@ -836,10 +838,9 @@ bound6_handle (NMDhcpSystemd *self) gs_unref_hashtable GHashTable *options = NULL; gs_free_error GError *error = NULL; sd_dhcp6_lease *lease; - int r; - r = sd_dhcp6_client_get_lease (priv->client6, &lease); - if (r < 0 || !lease) { + if ( sd_dhcp6_client_get_lease (priv->client6, &lease) < 0 + || !lease) { _LOGW (" no lease!"); nm_dhcp_client_set_state (NM_DHCP_CLIENT (self), NM_DHCP_STATE_FAIL, NULL, NULL); return; From b05ebd54b7f595c961ef407927e777eda5a07505 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 27 Nov 2018 15:18:17 +0100 Subject: [PATCH 12/16] dhcp: minor cleanup parsing default route for internal client Combine same code. --- src/dhcp/nm-dhcp-systemd.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index 4e1ef7b80a..688fcd71eb 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -409,10 +409,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, static_default_gateway = TRUE; gateway_has = TRUE; gateway = r_gateway.s_addr; - - s = nm_utils_inet4_ntop (gateway, addr_str); - LOG_LEASE (LOGD_DHCP4, "gateway %s", s); - add_option (options, dhcp4_requests, SD_DHCP_OPTION_ROUTER, s); } } } @@ -429,18 +425,18 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, && sd_dhcp_lease_get_router (lease, &a_router) >= 0) { gateway_has = TRUE; gateway = a_router.s_addr; - s = nm_utils_inet4_ntop (a_router.s_addr, addr_str); - LOG_LEASE (LOGD_DHCP4, "gateway %s", s); - add_option (options, dhcp4_requests, SD_DHCP_OPTION_ROUTER, s); } if (gateway_has) { + s = nm_utils_inet4_ntop (gateway, addr_str); + LOG_LEASE (LOGD_DHCP4, "gateway %s", s); + add_option (options, dhcp4_requests, SD_DHCP_OPTION_ROUTER, s); nm_ip4_config_add_route (ip4_config, &((const NMPlatformIP4Route) { - .rt_source = NM_IP_CONFIG_SOURCE_DHCP, - .gateway = gateway, + .rt_source = NM_IP_CONFIG_SOURCE_DHCP, + .gateway = gateway, .table_coerced = nm_platform_route_table_coerce (route_table), - .metric = route_metric, + .metric = route_metric, }), NULL); } From 795facc2bab15251f228304913287f43db26aa58 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 27 Nov 2018 12:09:52 +0100 Subject: [PATCH 13/16] network: add sd_dhcp_route_get_option() accessor Since sd_dhcp_lease_get_routes() returns the list of all routes, the caller may need to differenciate whether the route was option 33 (static-routes) or 121 (classless-static-route). Add an accessor for the internal field. systemd-pull-request: #10951 --- src/systemd/src/libsystemd-network/sd-dhcp-lease.c | 6 ++++++ src/systemd/src/systemd/sd-dhcp-lease.h | 1 + 2 files changed, 7 insertions(+) diff --git a/src/systemd/src/libsystemd-network/sd-dhcp-lease.c b/src/systemd/src/libsystemd-network/sd-dhcp-lease.c index 71e825912c..b953efc3df 100644 --- a/src/systemd/src/libsystemd-network/sd-dhcp-lease.c +++ b/src/systemd/src/libsystemd-network/sd-dhcp-lease.c @@ -1301,3 +1301,9 @@ int sd_dhcp_route_get_gateway(sd_dhcp_route *route, struct in_addr *gateway) { *gateway = route->gw_addr; return 0; } + +int sd_dhcp_route_get_option(sd_dhcp_route *route) { + assert_return(route, -EINVAL); + + return route->option; +} diff --git a/src/systemd/src/systemd/sd-dhcp-lease.h b/src/systemd/src/systemd/sd-dhcp-lease.h index 2a60145f5b..4875f10555 100644 --- a/src/systemd/src/systemd/sd-dhcp-lease.h +++ b/src/systemd/src/systemd/sd-dhcp-lease.h @@ -57,6 +57,7 @@ int sd_dhcp_lease_get_timezone(sd_dhcp_lease *lease, const char **timezone); int sd_dhcp_route_get_destination(sd_dhcp_route *route, struct in_addr *destination); int sd_dhcp_route_get_destination_prefix_length(sd_dhcp_route *route, uint8_t *length); int sd_dhcp_route_get_gateway(sd_dhcp_route *route, struct in_addr *gateway); +int sd_dhcp_route_get_option(sd_dhcp_route *route); _SD_DEFINE_POINTER_CLEANUP_FUNC(sd_dhcp_lease, sd_dhcp_lease_unref); From 2f2b489d38509c5efdd17c72351749dd1fefa8bd Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 3 Dec 2018 16:08:34 +0100 Subject: [PATCH 14/16] dhcp: request classless-static-route option first according to RFC 3442 In ip4_start(), we iterate over @dhcp4_requests array and add the options that are to be included. We do so, by calling sd_dhcp_client_set_request_option(). Note that sd_dhcp_client_set_request_option() only appends the options to a list, not taking special care about the order in which options are added. RFC 3442 (The Classless Static Route Option for Dynamic Host Configuration Protocol (DHCP) version 4) says: DHCP clients that support this option and send a parameter request list MAY also request the Static Routes option, for compatibility with older servers that don't support Classless Static Routes. The Classless Static Routes option code MUST appear in the parameter request list prior to both the Router option code and the Static Routes option code, if present. Compare to RFC 2132 (DHCP Options and BOOTP Vendor Extensions) which says about the parameter request list: The client MAY list the options in order of preference. Note, with RFC 7844 (Anonymity Profiles for DHCP Clients), the order should be randomized. But since we don't follow RFC 7844 (yet), let's follow at least RFC 3442. --- src/dhcp/nm-dhcp-systemd.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index 688fcd71eb..2aa8b6bf1a 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -115,19 +115,24 @@ typedef struct { static const ReqOption dhcp4_requests[] = { REQ (SD_DHCP_OPTION_SUBNET_MASK, "subnet_mask", TRUE ), REQ (SD_DHCP_OPTION_TIME_OFFSET, "time_offset", TRUE ), - REQ (SD_DHCP_OPTION_ROUTER, "routers", TRUE ), REQ (SD_DHCP_OPTION_DOMAIN_NAME_SERVER, "domain_name_servers", TRUE ), REQ (SD_DHCP_OPTION_HOST_NAME, "host_name", TRUE ), REQ (SD_DHCP_OPTION_DOMAIN_NAME, "domain_name", TRUE ), REQ (SD_DHCP_OPTION_INTERFACE_MTU, "interface_mtu", TRUE ), REQ (SD_DHCP_OPTION_BROADCAST, "broadcast_address", TRUE ), + + /* RFC 3442: The Classless Static Routes option code MUST appear in the parameter + * request list prior to both the Router option code and the Static + * Routes option code, if present. */ + REQ (SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, "rfc3442_classless_static_routes", TRUE ), + REQ (SD_DHCP_OPTION_ROUTER, "routers", TRUE ), REQ (SD_DHCP_OPTION_STATIC_ROUTE, "static_routes", TRUE ), + REQ (DHCP_OPTION_NIS_DOMAIN, "nis_domain", TRUE ), REQ (DHCP_OPTION_NIS_SERVERS, "nis_servers", TRUE ), REQ (SD_DHCP_OPTION_NTP_SERVER, "ntp_servers", TRUE ), REQ (SD_DHCP_OPTION_SERVER_IDENTIFIER, "dhcp_server_identifier", TRUE ), REQ (SD_DHCP_OPTION_DOMAIN_SEARCH_LIST, "domain_search", TRUE ), - REQ (SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, "rfc3442_classless_static_routes", TRUE ), REQ (SD_DHCP_OPTION_PRIVATE_CLASSLESS_STATIC_ROUTE, "ms_classless_static_routes", TRUE ), REQ (SD_DHCP_OPTION_PRIVATE_PROXY_AUTODISCOVERY, "wpad", TRUE ), REQ (SD_DHCP_OPTION_ROOT_PATH, "root_path", TRUE ), From 9a6a35401322e2b387fb164f7f2d68963f4a1512 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Tue, 27 Nov 2018 12:56:40 +0100 Subject: [PATCH 15/16] dhcp: fix static-route handling for intenal client and support multiple default routes Preface: RFC 3442 (The Classless Static Route Option for Dynamic Host Configuration Protocol (DHCP) version 4) states: If the DHCP server returns both a Classless Static Routes option and a Router option, the DHCP client MUST ignore the Router option. Similarly, if the DHCP server returns both a Classless Static Routes option and a Static Routes option, the DHCP client MUST ignore the Static Routes option. Changes: - sd_dhcp_lease_get_routes() returns the combination of both option 33 (static routes) and 121 (classless static routes). If classless static routes are provided, the state routes must be ignored. - we collect the options hash that we expose on D-Bus. For that purpose, we must not merge both option types as classless static routes. Instead, we want to expose the values like we received them originally: as two different options. - we continue our deviation from RFC 3442, when receiving classless static routes with option 3 (Router), we only ignore the router if we didn't already receive a default route via classless static routes. - in the past, NetworkManager treated the default route specially, and one device could only have one default route. That limitation was already (partly) lifted by commit 5c299454b49b165f645c25fd3e083c0bb747ad91 (core: rework tracking of gateway/default-route in ip-config). However, from DHCP we still would only accept one default route. Fix that for internal client. Installing multiple default routes might make sense, as kernel apparently can skip unreachable routers (as it notes via ICMP messages) (rh#1634657). https://bugzilla.redhat.com/show_bug.cgi?id=1634657 --- shared/nm-utils/nm-macros-internal.h | 8 ++ src/dhcp/nm-dhcp-systemd.c | 170 ++++++++++++++++++--------- 2 files changed, 122 insertions(+), 56 deletions(-) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index d874275d3b..946864d08b 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -837,6 +837,14 @@ nm_gstring_prepare (GString **l) return *l; } +static inline GString * +nm_gstring_add_space_delimiter (GString *str) +{ + if (str->len > 0) + g_string_append_c (str, ' '); + return str; +} + static inline const char * nm_str_not_empty (const char *str) { diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index 2aa8b6bf1a..3f9530b661 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -253,7 +253,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, gs_unref_hashtable GHashTable *options = NULL; const struct in_addr *addr_list; char addr_str[NM_UTILS_INET_ADDRSTRLEN]; - char addr_str2[NM_UTILS_INET_ADDRSTRLEN]; const char *s; nm_auto_free_gstring GString *str = NULL; gs_free sd_dhcp_route **routes = NULL; @@ -263,9 +262,9 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, const void *data; gsize data_len; gboolean metered = FALSE; - gboolean static_default_gateway = FALSE; - gboolean gateway_has = FALSE; - in_addr_t gateway = 0; + gboolean has_router_from_classless = FALSE; + gboolean has_classless_route = FALSE; + gboolean has_static_route = FALSE; const gint32 ts = nm_utils_get_monotonic_timestamp_s (); gint64 ts_time = time (NULL); struct in_addr a_address; @@ -371,12 +370,40 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, num = sd_dhcp_lease_get_routes (lease, &routes); if (num > 0) { - nm_gstring_prepare (&str); + nm_auto_free_gstring GString *str_classless = NULL; + nm_auto_free_gstring GString *str_static = NULL; + guint32 default_route_metric = route_metric; + for (i = 0; i < num; i++) { - const char *gw_str; + switch (sd_dhcp_route_get_option (routes[i])) { + case SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE: + has_classless_route = TRUE; + break; + case SD_DHCP_OPTION_STATIC_ROUTE: + has_static_route = TRUE; + break; + } + } + + if (has_classless_route) + str_classless = g_string_sized_new (30); + if (has_static_route) + str_static = g_string_sized_new (30); + + for (i = 0; i < num; i++) { + char network_net_str[NM_UTILS_INET_ADDRSTRLEN]; + char gateway_str[NM_UTILS_INET_ADDRSTRLEN]; guint8 r_plen; struct in_addr r_network; struct in_addr r_gateway; + in_addr_t network_net; + int option; + guint32 m; + + option = sd_dhcp_route_get_option (routes[i]); + if (!NM_IN_SET (option, SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, + SD_DHCP_OPTION_STATIC_ROUTE)) + continue; if (sd_dhcp_route_get_destination (routes[i], &r_network) < 0) continue; @@ -386,64 +413,95 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, if (sd_dhcp_route_get_gateway (routes[i], &r_gateway) < 0) continue; - if (r_plen > 0) { - const in_addr_t network_net = nm_utils_ip4_address_clear_host_address (r_network.s_addr, - r_plen); + network_net = nm_utils_ip4_address_clear_host_address (r_network.s_addr, + r_plen); + nm_utils_inet4_ntop (network_net, network_net_str); + nm_utils_inet4_ntop (r_gateway.s_addr, gateway_str); - nm_ip4_config_add_route (ip4_config, - &((const NMPlatformIP4Route) { - .network = network_net, - .plen = r_plen, - .gateway = r_gateway.s_addr, - .rt_source = NM_IP_CONFIG_SOURCE_DHCP, - .metric = route_metric, - .table_coerced = nm_platform_route_table_coerce (route_table), - }), - NULL); + LOG_LEASE (LOGD_DHCP4, + "%sstatic route %s/%d gw %s", + option == SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE + ? "classless " + : "", + network_net_str, + (int) r_plen, + gateway_str); + g_string_append_printf (nm_gstring_add_space_delimiter ( option == SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE + ? str_classless + : str_static), + "%s/%d %s", + network_net_str, + (int) r_plen, + gateway_str); - s = nm_utils_inet4_ntop (network_net, addr_str); - gw_str = nm_utils_inet4_ntop (r_gateway.s_addr, addr_str2); - LOG_LEASE (LOGD_DHCP4, "static route %s/%d gw %s", s, (int) r_plen, gw_str); - g_string_append_printf (str, "%s%s/%d %s", - str->len ? " " : "", - s, - (int) r_plen, - gw_str); - } else { - if (!static_default_gateway) { - static_default_gateway = TRUE; - gateway_has = TRUE; - gateway = r_gateway.s_addr; - } + if ( option == SD_DHCP_OPTION_STATIC_ROUTE + && has_classless_route) { + /* RFC 3443: if the DHCP server returns both a Classless Static Routes + * option and a Static Routes option, the DHCP client MUST ignore the + * Static Routes option. */ + continue; } + + if ( r_plen == 0 + && option == SD_DHCP_OPTION_STATIC_ROUTE) { + /* for option 33 (static route), RFC 2132 says: + * + * The default route (0.0.0.0) is an illegal destination for a static + * route. */ + continue; + } + + if (r_plen == 0) { + /* if there are multiple default routes, we add them with differing + * metrics. */ + m = default_route_metric; + if (default_route_metric < G_MAXUINT32) + default_route_metric++; + + has_router_from_classless = TRUE; + } else + m = route_metric; + + nm_ip4_config_add_route (ip4_config, + &((const NMPlatformIP4Route) { + .network = network_net, + .plen = r_plen, + .gateway = r_gateway.s_addr, + .rt_source = NM_IP_CONFIG_SOURCE_DHCP, + .metric = m, + .table_coerced = nm_platform_route_table_coerce (route_table), + }), + NULL); } - if (str->len) - add_option (options, dhcp4_requests, SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, str->str); + + if (str_classless && str_classless->len > 0) + add_option (options, dhcp4_requests, SD_DHCP_OPTION_CLASSLESS_STATIC_ROUTE, str_classless->str); + if (str_static && str_static->len > 0) + add_option (options, dhcp4_requests, SD_DHCP_OPTION_STATIC_ROUTE, str_static->str); } - /* If the DHCP server returns both a Classless Static Routes option and a - * Router option, the DHCP client MUST ignore the Router option [RFC 3442]. - * Be more lenient and ignore the Router option only if Classless Static - * Routes contain a default gateway (as other DHCP backends do). - */ - if ( !static_default_gateway - && sd_dhcp_lease_get_router (lease, &a_router) >= 0) { - gateway_has = TRUE; - gateway = a_router.s_addr; - } - - if (gateway_has) { - s = nm_utils_inet4_ntop (gateway, addr_str); + /* FIXME: internal client only supports returing the first router. */ + if (sd_dhcp_lease_get_router (lease, &a_router) >= 0) { + s = nm_utils_inet4_ntop (a_router.s_addr, addr_str); LOG_LEASE (LOGD_DHCP4, "gateway %s", s); add_option (options, dhcp4_requests, SD_DHCP_OPTION_ROUTER, s); - nm_ip4_config_add_route (ip4_config, - &((const NMPlatformIP4Route) { - .rt_source = NM_IP_CONFIG_SOURCE_DHCP, - .gateway = gateway, - .table_coerced = nm_platform_route_table_coerce (route_table), - .metric = route_metric, - }), - NULL); + + /* If the DHCP server returns both a Classless Static Routes option and a + * Router option, the DHCP client MUST ignore the Router option [RFC 3442]. + * + * Be more lenient and ignore the Router option only if Classless Static + * Routes contain a default gateway (as other DHCP backends do). + */ + if (!has_router_from_classless) { + nm_ip4_config_add_route (ip4_config, + &((const NMPlatformIP4Route) { + .rt_source = NM_IP_CONFIG_SOURCE_DHCP, + .gateway = a_router.s_addr, + .table_coerced = nm_platform_route_table_coerce (route_table), + .metric = route_metric, + }), + NULL); + } } if ( sd_dhcp_lease_get_mtu (lease, &mtu) >= 0 From 3102b49f623c499638c8f284c2f651829f904dc2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 14 Dec 2018 17:04:21 +0100 Subject: [PATCH 16/16] core: allow addresses with zero prefix length There is really no problem here, allow it. Previously we would assert against a non-zero prefix length. But I am not sure that all callers really ensured that this couldn't happen. Anyway, there is no problem we such addresses, really. Only we need to make sure that nm_ip4_config_add_dependent_routes() and nm_ip6_config_add_dependent_routes() don't add prefix routes for such addresses (which is the case now). --- src/dhcp/nm-dhcp-systemd.c | 1 - src/nm-ip4-config.c | 2 +- src/nm-ip6-config.c | 4 +++- 3 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/dhcp/nm-dhcp-systemd.c b/src/dhcp/nm-dhcp-systemd.c index 3f9530b661..614485c4e7 100644 --- a/src/dhcp/nm-dhcp-systemd.c +++ b/src/dhcp/nm-dhcp-systemd.c @@ -310,7 +310,6 @@ lease_to_ip4_config (NMDedupMultiIndex *multi_idx, SD_DHCP_OPTION_IP_ADDRESS_LEASE_TIME, (guint64) (ts_time + a_lifetime)); - // TODO: ensure a_plen of zero is handled correctly. nm_ip4_config_add_address (ip4_config, &((const NMPlatformIP4Address) { .address = a_address.s_addr, diff --git a/src/nm-ip4-config.c b/src/nm-ip4-config.c index a6b6f83fec..1179a77ffa 100644 --- a/src/nm-ip4-config.c +++ b/src/nm-ip4-config.c @@ -2113,7 +2113,7 @@ nm_ip4_config_add_address (NMIP4Config *self, const NMPlatformIP4Address *new) { g_return_if_fail (self); g_return_if_fail (new); - g_return_if_fail (new->plen > 0 && new->plen <= 32); + g_return_if_fail (new->plen <= 32); g_return_if_fail (NM_IP4_CONFIG_GET_PRIVATE (self)->ifindex > 0); _add_address (self, NULL, new); diff --git a/src/nm-ip6-config.c b/src/nm-ip6-config.c index c1864d338a..6d54809edd 100644 --- a/src/nm-ip6-config.c +++ b/src/nm-ip6-config.c @@ -485,6 +485,8 @@ nm_ip6_config_add_dependent_routes (NMIP6Config *self, if (NM_FLAGS_HAS (my_addr->n_ifa_flags, IFA_F_NOPREFIXROUTE)) continue; + if (my_addr->plen == 0) + continue; has_peer = !IN6_IS_ADDR_UNSPECIFIED (&my_addr->peer_address); @@ -1593,7 +1595,7 @@ nm_ip6_config_add_address (NMIP6Config *self, const NMPlatformIP6Address *new) { g_return_if_fail (self); g_return_if_fail (new); - g_return_if_fail (new->plen > 0 && new->plen <= 128); + g_return_if_fail (new->plen <= 128); g_return_if_fail (NM_IP6_CONFIG_GET_PRIVATE (self)->ifindex > 0); _add_address (self, NULL, new);