From 67f02b2a1419ad1144b197a6448282833c4fa5c5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 13 Dec 2018 10:32:17 +0100 Subject: [PATCH 01/10] platform: assert length of stack allocation in NMP_SYSCTL_PATHID_NETDIR_unsafe() NMP_SYSCTL_PATHID_NETDIR_unsafe() uses alloca() to allocate the string. Assert that the "path" argument is reasonably short. In practice, that is of course the case, because there are only 2 callers which take care not to pass an untrusted, unbounded path argument. --- src/platform/nm-platform.h | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index dff5102d85..a1eab9effd 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -1106,7 +1106,14 @@ const char *nm_platform_error_to_string (NMPlatformError error, ((const char *) NULL), -1, (path) #define NMP_SYSCTL_PATHID_NETDIR_unsafe(dirfd, ifname, path) \ - nm_sprintf_bufa (NM_STRLEN ("net:/sys/class/net//\0") + NMP_IFNAMSIZ + strlen (path), \ + nm_sprintf_bufa ( NM_STRLEN ("net:/sys/class/net//\0") \ + + NMP_IFNAMSIZ \ + + ({ \ + const gsize _l = strlen (path); \ + \ + nm_assert (_l < 200); \ + _l; \ + }), \ "net:/sys/class/net/%s/%s", (ifname), (path)), \ (dirfd), (path) From 8b9fd01ef355d78dab9eb4c3811d56bcf2e749b8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 13 Dec 2018 11:08:32 +0100 Subject: [PATCH 02/10] platform: use struct initializer instead of memset() I think this is preferred over memset(), because it allows the compiler to better unstand what is happening. Also, strictly speaking in the C language, %NULL pointers are not guaranteed to have an all zero bit pattern. Of course, that is already required on any architecture where NetworkManager is running. --- src/platform/nm-linux-platform.c | 5 +++-- src/platform/nmp-object.c | 18 ++++++++++++------ 2 files changed, 15 insertions(+), 8 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 8b4b081f64..008a3bc973 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -1942,9 +1942,10 @@ _wireguard_update_from_allowed_ips_nla (NMPWireGuardAllowedIP *allowed_ip, _check_addr_or_return_val (tb, WGALLOWEDIP_A_IPADDR, addr_len, FALSE); - memset (allowed_ip, 0, sizeof (NMPWireGuardAllowedIP)); + *allowed_ip = (NMPWireGuardAllowedIP) { + .family = family, + }; - allowed_ip->family = family; nm_assert ((int) allowed_ip->family == family); if (tb[WGALLOWEDIP_A_IPADDR]) diff --git a/src/platform/nmp-object.c b/src/platform/nmp-object.c index fd505282e2..1218b5a224 100644 --- a/src/platform/nmp-object.c +++ b/src/platform/nmp-object.c @@ -630,9 +630,12 @@ _nmp_object_stackinit_from_class (NMPObject *obj, const NMPClass *klass) nm_assert (obj); nm_assert (klass); - memset (obj, 0, sizeof (NMPObject)); - obj->_class = klass; - obj->parent._ref_count = NM_OBJ_REF_COUNT_STACKINIT; + *obj = (NMPObject) { + .parent = { + .klass = (const NMDedupMultiObjClass *) klass, + ._ref_count = NM_OBJ_REF_COUNT_STACKINIT, + }, + }; } static NMPObject * @@ -644,9 +647,12 @@ _nmp_object_stackinit_from_type (NMPObject *obj, NMPObjectType obj_type) klass = nmp_class_from_type (obj_type); nm_assert (klass); - memset (obj, 0, sizeof (NMPObject)); - obj->_class = klass; - obj->parent._ref_count = NM_OBJ_REF_COUNT_STACKINIT; + *obj = (NMPObject) { + .parent = { + .klass = (const NMDedupMultiObjClass *) klass, + ._ref_count = NM_OBJ_REF_COUNT_STACKINIT, + }, + }; return obj; } From d27fa362722e9ccfdc29aae49b098bece0e4ca00 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 13 Dec 2018 11:09:45 +0100 Subject: [PATCH 03/10] device: merge IPv4 and IPv6 variants of nm_device_ipv4_sysctl_set() For one, next we will drop setting rp_filter, hence there are no more users of an IPv4 variant and nm_device_ipv4_sysctl_set() would have to be dropped anyway. However, instead of doing that, merge the IPv4 and IPv6 variant. With this, the fallback to the default is now also supported for IPv6 (though unused). Also, don't access nm_device_get_ip_iface(). The interface name might not be right, we should only rely on the ifindex. Load the interface name from platform cache instead. --- src/devices/nm-device-private.h | 4 +- src/devices/nm-device.c | 98 ++++++++++++++++-------------- src/devices/wwan/nm-device-modem.c | 4 +- 3 files changed, 57 insertions(+), 49 deletions(-) diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index d02d8c3b5d..8edcd3d1de 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -111,9 +111,9 @@ void nm_device_set_wwan_ip6_config (NMDevice *device, NMIP6Config *config); gboolean nm_device_hw_addr_is_explict (NMDevice *device); -void nm_device_ip_method_failed (NMDevice *self, int family, NMDeviceStateReason reason); +void nm_device_ip_method_failed (NMDevice *self, int addr_family, NMDeviceStateReason reason); -gboolean nm_device_ipv6_sysctl_set (NMDevice *self, const char *property, const char *value); +gboolean nm_device_ip_sysctl_set (NMDevice *self, int addr_family, const char *property, const char *value); /*****************************************************************************/ diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 386fa01dac..6cc5fdcc7a 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1135,29 +1135,46 @@ init_ip_config_dns_priority (NMDevice *self, NMIPConfig *config) /*****************************************************************************/ -static gboolean -nm_device_ipv4_sysctl_set (NMDevice *self, const char *property, const char *value) +gboolean +nm_device_ip_sysctl_set (NMDevice *self, + int addr_family, + const char *property, + const char *value) { NMPlatform *platform = nm_device_get_platform (self); gs_free char *value_to_free = NULL; - const char *value_to_set; char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; + const char *ifname; + int ifindex; - if (!nm_device_get_ip_ifindex (self)) + nm_assert_addr_family (addr_family); + + ifindex = nm_device_get_ip_ifindex (self); + if (ifindex <= 0) return FALSE; - if (value) { - value_to_set = value; - } else { + ifname = nm_platform_link_get_name (platform, ifindex); + if (!ifname) + return FALSE; + + if (!value) { /* Set to a default value when we've got a NULL @value. */ value_to_free = nm_platform_sysctl_get (platform, - NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET, buf, "default", property))); - value_to_set = value_to_free; + NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (addr_family, + buf, + "default", + property))); + value = value_to_free; + if (!value) + return FALSE; } return nm_platform_sysctl_set (platform, - NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET, buf, nm_device_get_ip_iface (self), property)), - value_to_set); + NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (addr_family, + buf, + ifname, + property)), + value); } static guint32 @@ -1198,17 +1215,6 @@ nm_device_ipv4_sysctl_get_effective_uint32 (NMDevice *self, const char *property return v > -1 ? (guint32) v : fallback; } -gboolean -nm_device_ipv6_sysctl_set (NMDevice *self, const char *property, const char *value) -{ - char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; - - if (!nm_device_get_ip_ifindex (self)) - return FALSE; - - return nm_platform_sysctl_set (nm_device_get_platform (self), NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, buf, nm_device_get_ip_iface (self), property)), value); -} - static guint32 nm_device_ipv6_sysctl_get_uint32 (NMDevice *self, const char *property, guint32 fallback) { @@ -1225,6 +1231,8 @@ nm_device_ipv6_sysctl_get_uint32 (NMDevice *self, const char *property, guint32 fallback); } +/*****************************************************************************/ + gboolean nm_device_has_capability (NMDevice *self, NMDeviceCapabilities caps) { @@ -4023,7 +4031,7 @@ ip4_rp_filter_update (NMDevice *self) } if (ip4_rp_filter != priv->ip4_rp_filter) { - nm_device_ipv4_sysctl_set (self, "rp_filter", ip4_rp_filter); + nm_device_ip_sysctl_set (self, AF_INET, "rp_filter", ip4_rp_filter); priv->ip4_rp_filter = ip4_rp_filter; } } @@ -9269,8 +9277,8 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config) } if (ip6_mtu && ip6_mtu != _IP6_MTU_SYS ()) { - if (!nm_device_ipv6_sysctl_set (self, "mtu", - nm_sprintf_buf (sbuf, "%u", (unsigned) ip6_mtu))) { + if (!nm_device_ip_sysctl_set (self, AF_INET6, "mtu", + nm_sprintf_buf (sbuf, "%u", (unsigned) ip6_mtu))) { int errsv = errno; _NMLOG (anticipated_failure && errsv == EINVAL ? LOGL_DEBUG : LOGL_WARN, @@ -9479,14 +9487,14 @@ addrconf6_start_with_link_ready (NMDevice *self) switch (nm_ndisc_get_node_type (priv->ndisc)) { case NM_NDISC_NODE_TYPE_HOST: /* Accepting prefixes from discovered routers. */ - nm_device_ipv6_sysctl_set (self, "accept_ra", "1"); - nm_device_ipv6_sysctl_set (self, "accept_ra_defrtr", "0"); - nm_device_ipv6_sysctl_set (self, "accept_ra_pinfo", "0"); - nm_device_ipv6_sysctl_set (self, "accept_ra_rtr_pref", "0"); + nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra", "1"); + nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_defrtr", "0"); + nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_pinfo", "0"); + nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_rtr_pref", "0"); break; case NM_NDISC_NODE_TYPE_ROUTER: /* We're the router. */ - nm_device_ipv6_sysctl_set (self, "forwarding", "1"); + nm_device_ip_sysctl_set (self, AF_INET6, "forwarding", "1"); nm_device_activate_schedule_ip6_config_result (self); priv->needs_ip6_subnet = TRUE; g_signal_emit (self, signals[IP6_SUBNET_NEEDED], 0); @@ -9648,7 +9656,7 @@ restore_ip6_properties (NMDevice *self) if ( priv->ipv6ll_handle && nm_streq (key, "disable_ipv6")) continue; - nm_device_ipv6_sysctl_set (self, key, value); + nm_device_ip_sysctl_set (self, AF_INET6, key, value); } } @@ -9657,7 +9665,7 @@ set_disable_ipv6 (NMDevice *self, const char *value) { /* We only touch disable_ipv6 when NM is not managing the IPv6LL address */ if (!NM_DEVICE_GET_PRIVATE (self)->ipv6ll_handle) - nm_device_ipv6_sysctl_set (self, "disable_ipv6", value); + nm_device_ip_sysctl_set (self, AF_INET6, "disable_ipv6", value); } static inline void @@ -9694,11 +9702,11 @@ set_nm_ipv6ll (NMDevice *self, gboolean enable) value = nm_platform_sysctl_get (nm_device_get_platform (self), NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, buf, nm_device_get_ip_iface (self), "disable_ipv6"))); if (g_strcmp0 (value, "0") == 0) - nm_device_ipv6_sysctl_set (self, "disable_ipv6", "1"); + nm_device_ip_sysctl_set (self, AF_INET6, "disable_ipv6", "1"); g_free (value); /* Ensure IPv6 is enabled */ - nm_device_ipv6_sysctl_set (self, "disable_ipv6", "0"); + nm_device_ip_sysctl_set (self, AF_INET6, "disable_ipv6", "0"); } } @@ -9826,7 +9834,7 @@ act_stage3_ip6_config_start (NMDevice *self, */ set_nm_ipv6ll (self, FALSE); if (ipv6ll_handle_old) - nm_device_ipv6_sysctl_set (self, "disable_ipv6", "1"); + nm_device_ip_sysctl_set (self, AF_INET6, "disable_ipv6", "1"); restore_ip6_properties (self); } return NM_ACT_STAGE_RETURN_IP_DONE; @@ -9898,7 +9906,7 @@ act_stage3_ip6_config_start (NMDevice *self, ip6_privacy_str = "2"; break; } - nm_device_ipv6_sysctl_set (self, "use_tempaddr", ip6_privacy_str); + nm_device_ip_sysctl_set (self, AF_INET6, "use_tempaddr", ip6_privacy_str); } return ret; @@ -14468,8 +14476,8 @@ nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, CleanupType clean /* Turn off kernel IPv6 */ if (cleanup_type == CLEANUP_TYPE_DECONFIGURE) { set_disable_ipv6 (self, "1"); - nm_device_ipv6_sysctl_set (self, "accept_ra", "0"); - nm_device_ipv6_sysctl_set (self, "use_tempaddr", "0"); + nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra", "0"); + nm_device_ip_sysctl_set (self, AF_INET6, "use_tempaddr", "0"); } /* Call device type-specific deactivation */ @@ -14535,8 +14543,8 @@ nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, CleanupType clean if (priv->ip6_mtu_initial) { char sbuf[64]; - nm_device_ipv6_sysctl_set (self, "mtu", - nm_sprintf_buf (sbuf, "%u", (unsigned) priv->ip6_mtu_initial)); + nm_device_ip_sysctl_set (self, AF_INET6, "mtu", + nm_sprintf_buf (sbuf, "%u", (unsigned) priv->ip6_mtu_initial)); } } priv->mtu_initial = 0; @@ -14754,11 +14762,11 @@ ip6_managed_setup (NMDevice *self) { set_nm_ipv6ll (self, TRUE); set_disable_ipv6 (self, "1"); - nm_device_ipv6_sysctl_set (self, "accept_ra_defrtr", "0"); - nm_device_ipv6_sysctl_set (self, "accept_ra_pinfo", "0"); - nm_device_ipv6_sysctl_set (self, "accept_ra_rtr_pref", "0"); - nm_device_ipv6_sysctl_set (self, "use_tempaddr", "0"); - nm_device_ipv6_sysctl_set (self, "forwarding", "0"); + nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_defrtr", "0"); + nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_pinfo", "0"); + nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_rtr_pref", "0"); + nm_device_ip_sysctl_set (self, AF_INET6, "use_tempaddr", "0"); + nm_device_ip_sysctl_set (self, AF_INET6, "forwarding", "0"); } static void diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index be08498608..ae606009e4 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -244,7 +244,7 @@ modem_ip6_config_result (NMModem *modem, } /* Re-enable IPv6 on the interface */ - nm_device_ipv6_sysctl_set (device, "disable_ipv6", "0"); + nm_device_ip_sysctl_set (device, AF_INET6, "disable_ipv6", "0"); if (config) nm_device_set_wwan_ip6_config (device, config); @@ -303,7 +303,7 @@ ip_ifindex_changed_cb (NMModem *modem, GParamSpec *pspec, gpointer user_data) * internally, and leaving it enabled could allow the kernel's IPv6 * RA handling code to run before NM is ready. */ - nm_device_ipv6_sysctl_set (device, "disable_ipv6", "1"); + nm_device_ip_sysctl_set (device, AF_INET6, "disable_ipv6", "1"); } static void From 395374cfbebf877d19b06fae13c35ebb74c07b4f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 13 Dec 2018 11:34:39 +0100 Subject: [PATCH 04/10] device/trivial: rename device's sysctl function These functions call platform's sysctl getter and setters. Note that the called platform functions are called nm_platform_sysctl_get() and nm_platform_sysctl_set(). Also, in this case they use the ip-conf path via nm_utils_sysctl_ip_conf_path(). Also, next we will add API nm_platform_sysctl_ip_conf_get() and nm_platform_sysctl_ip_conf_set(), which will be wrappers around nm_platform_sysctl_get() and nm_platform_sysctl_set(), using the ip-conf paths as well. Rename the device functions, to be more similar to the existing and future naming in platform. --- src/devices/nm-device-private.h | 5 ++- src/devices/nm-device.c | 62 +++++++++++++++--------------- src/devices/wwan/nm-device-modem.c | 4 +- 3 files changed, 37 insertions(+), 34 deletions(-) diff --git a/src/devices/nm-device-private.h b/src/devices/nm-device-private.h index 8edcd3d1de..6c0f473d53 100644 --- a/src/devices/nm-device-private.h +++ b/src/devices/nm-device-private.h @@ -113,7 +113,10 @@ gboolean nm_device_hw_addr_is_explict (NMDevice *device); void nm_device_ip_method_failed (NMDevice *self, int addr_family, NMDeviceStateReason reason); -gboolean nm_device_ip_sysctl_set (NMDevice *self, int addr_family, const char *property, const char *value); +gboolean nm_device_sysctl_ip_conf_set (NMDevice *self, + int addr_family, + const char *property, + const char *value); /*****************************************************************************/ diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 6cc5fdcc7a..28c263aca0 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1136,10 +1136,10 @@ init_ip_config_dns_priority (NMDevice *self, NMIPConfig *config) /*****************************************************************************/ gboolean -nm_device_ip_sysctl_set (NMDevice *self, - int addr_family, - const char *property, - const char *value) +nm_device_sysctl_ip_conf_set (NMDevice *self, + int addr_family, + const char *property, + const char *value) { NMPlatform *platform = nm_device_get_platform (self); gs_free char *value_to_free = NULL; @@ -1178,7 +1178,7 @@ nm_device_ip_sysctl_set (NMDevice *self, } static guint32 -nm_device_ipv4_sysctl_get_effective_uint32 (NMDevice *self, const char *property, guint32 fallback) +nm_device_sysctl_ip_conf_get_effective_uint32 (NMDevice *self, const char *property, guint32 fallback) { char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; gint64 v, v_all; @@ -1216,7 +1216,7 @@ nm_device_ipv4_sysctl_get_effective_uint32 (NMDevice *self, const char *property } static guint32 -nm_device_ipv6_sysctl_get_uint32 (NMDevice *self, const char *property, guint32 fallback) +nm_device_sysctl_ip_conf_get_uint32 (NMDevice *self, const char *property, guint32 fallback) { char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; @@ -4019,7 +4019,7 @@ ip4_rp_filter_update (NMDevice *self) if ( priv->v4_has_shadowed_routes || nm_device_get_best_default_route (self, AF_INET)) { - if (nm_device_ipv4_sysctl_get_effective_uint32 (self, "rp_filter", 0) != 1) { + if (nm_device_sysctl_ip_conf_get_effective_uint32 (self, "rp_filter", 0) != 1) { /* Don't touch the rp_filter if it's not strict. */ return; } @@ -4031,7 +4031,7 @@ ip4_rp_filter_update (NMDevice *self) } if (ip4_rp_filter != priv->ip4_rp_filter) { - nm_device_ip_sysctl_set (self, AF_INET, "rp_filter", ip4_rp_filter); + nm_device_sysctl_ip_conf_set (self, AF_INET, "rp_filter", ip4_rp_filter); priv->ip4_rp_filter = ip4_rp_filter; } } @@ -9246,7 +9246,7 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config) #define _IP6_MTU_SYS() \ ({ \ if (!ip6_mtu_sysctl.initialized) { \ - ip6_mtu_sysctl.value = nm_device_ipv6_sysctl_get_uint32 (self, "mtu", 0); \ + ip6_mtu_sysctl.value = nm_device_sysctl_ip_conf_get_uint32 (self, "mtu", 0); \ ip6_mtu_sysctl.initialized = TRUE; \ } \ ip6_mtu_sysctl.value; \ @@ -9277,8 +9277,8 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config) } if (ip6_mtu && ip6_mtu != _IP6_MTU_SYS ()) { - if (!nm_device_ip_sysctl_set (self, AF_INET6, "mtu", - nm_sprintf_buf (sbuf, "%u", (unsigned) ip6_mtu))) { + if (!nm_device_sysctl_ip_conf_set (self, AF_INET6, "mtu", + nm_sprintf_buf (sbuf, "%u", (unsigned) ip6_mtu))) { int errsv = errno; _NMLOG (anticipated_failure && errsv == EINVAL ? LOGL_DEBUG : LOGL_WARN, @@ -9487,14 +9487,14 @@ addrconf6_start_with_link_ready (NMDevice *self) switch (nm_ndisc_get_node_type (priv->ndisc)) { case NM_NDISC_NODE_TYPE_HOST: /* Accepting prefixes from discovered routers. */ - nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra", "1"); - nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_defrtr", "0"); - nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_pinfo", "0"); - nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_rtr_pref", "0"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "accept_ra", "1"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "accept_ra_defrtr", "0"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "accept_ra_pinfo", "0"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "accept_ra_rtr_pref", "0"); break; case NM_NDISC_NODE_TYPE_ROUTER: /* We're the router. */ - nm_device_ip_sysctl_set (self, AF_INET6, "forwarding", "1"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "forwarding", "1"); nm_device_activate_schedule_ip6_config_result (self); priv->needs_ip6_subnet = TRUE; g_signal_emit (self, signals[IP6_SUBNET_NEEDED], 0); @@ -9656,7 +9656,7 @@ restore_ip6_properties (NMDevice *self) if ( priv->ipv6ll_handle && nm_streq (key, "disable_ipv6")) continue; - nm_device_ip_sysctl_set (self, AF_INET6, key, value); + nm_device_sysctl_ip_conf_set (self, AF_INET6, key, value); } } @@ -9665,7 +9665,7 @@ set_disable_ipv6 (NMDevice *self, const char *value) { /* We only touch disable_ipv6 when NM is not managing the IPv6LL address */ if (!NM_DEVICE_GET_PRIVATE (self)->ipv6ll_handle) - nm_device_ip_sysctl_set (self, AF_INET6, "disable_ipv6", value); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "disable_ipv6", value); } static inline void @@ -9702,11 +9702,11 @@ set_nm_ipv6ll (NMDevice *self, gboolean enable) value = nm_platform_sysctl_get (nm_device_get_platform (self), NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, buf, nm_device_get_ip_iface (self), "disable_ipv6"))); if (g_strcmp0 (value, "0") == 0) - nm_device_ip_sysctl_set (self, AF_INET6, "disable_ipv6", "1"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "disable_ipv6", "1"); g_free (value); /* Ensure IPv6 is enabled */ - nm_device_ip_sysctl_set (self, AF_INET6, "disable_ipv6", "0"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "disable_ipv6", "0"); } } @@ -9834,7 +9834,7 @@ act_stage3_ip6_config_start (NMDevice *self, */ set_nm_ipv6ll (self, FALSE); if (ipv6ll_handle_old) - nm_device_ip_sysctl_set (self, AF_INET6, "disable_ipv6", "1"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "disable_ipv6", "1"); restore_ip6_properties (self); } return NM_ACT_STAGE_RETURN_IP_DONE; @@ -9906,7 +9906,7 @@ act_stage3_ip6_config_start (NMDevice *self, ip6_privacy_str = "2"; break; } - nm_device_ip_sysctl_set (self, AF_INET6, "use_tempaddr", ip6_privacy_str); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "use_tempaddr", ip6_privacy_str); } return ret; @@ -14476,8 +14476,8 @@ nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, CleanupType clean /* Turn off kernel IPv6 */ if (cleanup_type == CLEANUP_TYPE_DECONFIGURE) { set_disable_ipv6 (self, "1"); - nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra", "0"); - nm_device_ip_sysctl_set (self, AF_INET6, "use_tempaddr", "0"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "accept_ra", "0"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "use_tempaddr", "0"); } /* Call device type-specific deactivation */ @@ -14543,8 +14543,8 @@ nm_device_cleanup (NMDevice *self, NMDeviceStateReason reason, CleanupType clean if (priv->ip6_mtu_initial) { char sbuf[64]; - nm_device_ip_sysctl_set (self, AF_INET6, "mtu", - nm_sprintf_buf (sbuf, "%u", (unsigned) priv->ip6_mtu_initial)); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "mtu", + nm_sprintf_buf (sbuf, "%u", (unsigned) priv->ip6_mtu_initial)); } } priv->mtu_initial = 0; @@ -14762,11 +14762,11 @@ ip6_managed_setup (NMDevice *self) { set_nm_ipv6ll (self, TRUE); set_disable_ipv6 (self, "1"); - nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_defrtr", "0"); - nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_pinfo", "0"); - nm_device_ip_sysctl_set (self, AF_INET6, "accept_ra_rtr_pref", "0"); - nm_device_ip_sysctl_set (self, AF_INET6, "use_tempaddr", "0"); - nm_device_ip_sysctl_set (self, AF_INET6, "forwarding", "0"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "accept_ra_defrtr", "0"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "accept_ra_pinfo", "0"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "accept_ra_rtr_pref", "0"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "use_tempaddr", "0"); + nm_device_sysctl_ip_conf_set (self, AF_INET6, "forwarding", "0"); } static void diff --git a/src/devices/wwan/nm-device-modem.c b/src/devices/wwan/nm-device-modem.c index ae606009e4..bd9ee3bb8e 100644 --- a/src/devices/wwan/nm-device-modem.c +++ b/src/devices/wwan/nm-device-modem.c @@ -244,7 +244,7 @@ modem_ip6_config_result (NMModem *modem, } /* Re-enable IPv6 on the interface */ - nm_device_ip_sysctl_set (device, AF_INET6, "disable_ipv6", "0"); + nm_device_sysctl_ip_conf_set (device, AF_INET6, "disable_ipv6", "0"); if (config) nm_device_set_wwan_ip6_config (device, config); @@ -303,7 +303,7 @@ ip_ifindex_changed_cb (NMModem *modem, GParamSpec *pspec, gpointer user_data) * internally, and leaving it enabled could allow the kernel's IPv6 * RA handling code to run before NM is ready. */ - nm_device_ip_sysctl_set (device, AF_INET6, "disable_ipv6", "1"); + nm_device_sysctl_ip_conf_set (device, AF_INET6, "disable_ipv6", "1"); } static void From 7fa398d59674c0eedc468971b6a346719b6dcc97 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 13 Dec 2018 11:48:06 +0100 Subject: [PATCH 05/10] platform: add nm_platform_sysctl_ip_conf_*() wrappers --- src/platform/nm-platform.c | 84 +++++++++++++++++++++++++++++++++++++- src/platform/nm-platform.h | 26 ++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index f41517bd41..6adbb815ce 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -570,7 +570,14 @@ nm_platform_sysctl_get_int32 (NMPlatform *self, const char *pathid, int dirfd, c * (inclusive) or @fallback. */ gint64 -nm_platform_sysctl_get_int_checked (NMPlatform *self, const char *pathid, int dirfd, const char *path, guint base, gint64 min, gint64 max, gint64 fallback) +nm_platform_sysctl_get_int_checked (NMPlatform *self, + const char *pathid, + int dirfd, + const char *path, + guint base, + gint64 min, + gint64 max, + gint64 fallback) { char *value = NULL; gint32 ret; @@ -597,6 +604,81 @@ nm_platform_sysctl_get_int_checked (NMPlatform *self, const char *pathid, int di /*****************************************************************************/ +char * +nm_platform_sysctl_ip_conf_get (NMPlatform *platform, + int addr_family, + const char *ifname, + const char *property) +{ + char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; + + return nm_platform_sysctl_get (platform, + NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (addr_family, + buf, + ifname, + property))); +} + +gint64 +nm_platform_sysctl_ip_conf_get_int_checked (NMPlatform *platform, + int addr_family, + const char *ifname, + const char *property, + guint base, + gint64 min, + gint64 max, + gint64 fallback) +{ + char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; + + return nm_platform_sysctl_get_int_checked (platform, + NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (addr_family, + buf, + ifname, + property)), + base, + min, + max, + fallback); +} + +gboolean +nm_platform_sysctl_ip_conf_set (NMPlatform *platform, + int addr_family, + const char *ifname, + const char *property, + const char *value) +{ + char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; + + return nm_platform_sysctl_set (platform, + NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (addr_family, + buf, + ifname, + property)), + value); +} + +gboolean +nm_platform_sysctl_ip_conf_set_int64 (NMPlatform *platform, + int addr_family, + const char *ifname, + const char *property, + gint64 value) +{ + char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; + char s[64]; + + return nm_platform_sysctl_set (platform, + NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (addr_family, + buf, + ifname, + property)), + nm_sprintf_buf (s, "%"G_GINT64_FORMAT, value)); +} + +/*****************************************************************************/ + static int _link_get_all_presort (gconstpointer p_a, gconstpointer p_b, diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index a1eab9effd..ece106bfe2 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -1128,6 +1128,32 @@ char *nm_platform_sysctl_get (NMPlatform *self, const char *pathid, int dirfd, c gint32 nm_platform_sysctl_get_int32 (NMPlatform *self, const char *pathid, int dirfd, const char *path, gint32 fallback); gint64 nm_platform_sysctl_get_int_checked (NMPlatform *self, const char *pathid, int dirfd, const char *path, guint base, gint64 min, gint64 max, gint64 fallback); +char *nm_platform_sysctl_ip_conf_get (NMPlatform *platform, + int addr_family, + const char *ifname, + const char *property); + +gint64 nm_platform_sysctl_ip_conf_get_int_checked (NMPlatform *platform, + int addr_family, + const char *ifname, + const char *property, + guint base, + gint64 min, + gint64 max, + gint64 fallback); + +gboolean nm_platform_sysctl_ip_conf_set (NMPlatform *platform, + int addr_family, + const char *ifname, + const char *property, + const char *value); + +gboolean nm_platform_sysctl_ip_conf_set_int64 (NMPlatform *platform, + int addr_family, + const char *ifname, + const char *property, + gint64 value); + gboolean nm_platform_sysctl_set_ip6_hop_limit_safe (NMPlatform *self, const char *iface, int value); const char *nm_platform_if_indextoname (NMPlatform *self, int ifindex, char *out_ifname/* of size IFNAMSIZ */); From 18a99e8652452e5c31ce6e9796e7b0bf398be519 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 13 Dec 2018 12:41:20 +0100 Subject: [PATCH 06/10] core: use nm_platform_sysctl_ip_conf_*() wrappers --- src/devices/nm-device.c | 91 ++++++++++++++++++--------------------- src/ndisc/nm-lndp-ndisc.c | 16 +++---- src/nm-iface-helper.c | 25 ++++++----- 3 files changed, 65 insertions(+), 67 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 28c263aca0..4e9ac076ff 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1143,7 +1143,6 @@ nm_device_sysctl_ip_conf_set (NMDevice *self, { NMPlatform *platform = nm_device_get_platform (self); gs_free char *value_to_free = NULL; - char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; const char *ifname; int ifindex; @@ -1159,28 +1158,25 @@ nm_device_sysctl_ip_conf_set (NMDevice *self, if (!value) { /* Set to a default value when we've got a NULL @value. */ - value_to_free = nm_platform_sysctl_get (platform, - NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (addr_family, - buf, - "default", - property))); + value_to_free = nm_platform_sysctl_ip_conf_get (platform, + addr_family, + "default", + property); value = value_to_free; if (!value) return FALSE; } - return nm_platform_sysctl_set (platform, - NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (addr_family, - buf, - ifname, - property)), - value); + return nm_platform_sysctl_ip_conf_set (platform, + addr_family, + ifname, + property, + value); } static guint32 nm_device_sysctl_ip_conf_get_effective_uint32 (NMDevice *self, const char *property, guint32 fallback) { - char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; gint64 v, v_all; if (!nm_device_get_ip_ifindex (self)) @@ -1191,25 +1187,23 @@ nm_device_sysctl_ip_conf_get_effective_uint32 (NMDevice *self, const char *prope * * Also do that, by reading both sysctls and return the maximum. */ - v = nm_platform_sysctl_get_int_checked (nm_device_get_platform (self), - NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET, - buf, - nm_device_get_ip_iface (self), - property)), - 10, - 0, - G_MAXUINT32, - -1); + v = nm_platform_sysctl_ip_conf_get_int_checked (nm_device_get_platform (self), + AF_INET, + nm_device_get_ip_iface (self), + property, + 10, + 0, + G_MAXUINT32, + -1); - v_all = nm_platform_sysctl_get_int_checked (nm_device_get_platform (self), - NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET, - buf, - "all", - property)), - 10, - 0, - G_MAXUINT32, - -1); + v_all = nm_platform_sysctl_ip_conf_get_int_checked (nm_device_get_platform (self), + AF_INET, + "all", + property, + 10, + 0, + G_MAXUINT32, + -1); v = NM_MAX (v, v_all); return v > -1 ? (guint32) v : fallback; @@ -1218,17 +1212,17 @@ nm_device_sysctl_ip_conf_get_effective_uint32 (NMDevice *self, const char *prope static guint32 nm_device_sysctl_ip_conf_get_uint32 (NMDevice *self, const char *property, guint32 fallback) { - char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; - if (!nm_device_get_ip_ifindex (self)) return fallback; - return nm_platform_sysctl_get_int_checked (nm_device_get_platform (self), - NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, buf, nm_device_get_ip_iface (self), property)), - 10, - 0, - G_MAXUINT32, - fallback); + return nm_platform_sysctl_ip_conf_get_int_checked (nm_device_get_platform (self), + AF_INET6, + nm_device_get_ip_iface (self), + property, + 10, + 0, + G_MAXUINT32, + fallback); } /*****************************************************************************/ @@ -9632,9 +9626,10 @@ save_ip6_properties (NMDevice *self) return; for (i = 0; i < G_N_ELEMENTS (ip6_properties_to_save); i++) { - char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; - - value = nm_platform_sysctl_get (nm_device_get_platform (self), NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, buf, ifname, ip6_properties_to_save[i]))); + value = nm_platform_sysctl_ip_conf_get (nm_device_get_platform (self), + AF_INET6, + ifname, + ip6_properties_to_save[i]); if (value) { g_hash_table_insert (priv->ip6_saved_properties, (char *) ip6_properties_to_save[i], @@ -9673,7 +9668,6 @@ set_nm_ipv6ll (NMDevice *self, gboolean enable) { NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); int ifindex = nm_device_get_ip_ifindex (self); - char *value; if (!nm_platform_check_kernel_support (nm_device_get_platform (self), NM_PLATFORM_KERNEL_SUPPORT_USER_IPV6LL)) @@ -9696,14 +9690,15 @@ set_nm_ipv6ll (NMDevice *self, gboolean enable) } if (enable) { - char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; + gs_free char *value = NULL; /* Bounce IPv6 to ensure the kernel stops IPv6LL address generation */ - value = nm_platform_sysctl_get (nm_device_get_platform (self), - NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, buf, nm_device_get_ip_iface (self), "disable_ipv6"))); - if (g_strcmp0 (value, "0") == 0) + value = nm_platform_sysctl_ip_conf_get (nm_device_get_platform (self), + AF_INET6, + nm_device_get_ip_iface (self), + "disable_ipv6"); + if (nm_streq0 (value, "0")) nm_device_sysctl_ip_conf_set (self, AF_INET6, "disable_ipv6", "1"); - g_free (value); /* Ensure IPv6 is enabled */ nm_device_sysctl_ip_conf_set (self, AF_INET6, "disable_ipv6", "0"); diff --git a/src/ndisc/nm-lndp-ndisc.c b/src/ndisc/nm-lndp-ndisc.c index 7f059a4102..535480501f 100644 --- a/src/ndisc/nm-lndp-ndisc.c +++ b/src/ndisc/nm-lndp-ndisc.c @@ -539,14 +539,14 @@ start (NMNDisc *ndisc) static inline int ipv6_sysctl_get (NMPlatform *platform, const char *ifname, const char *property, int min, int max, int defval) { - char buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; - - return (int) nm_platform_sysctl_get_int_checked (platform, - NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, buf, ifname, property)), - 10, - min, - max, - defval); + return nm_platform_sysctl_ip_conf_get_int_checked (platform, + AF_INET6, + ifname, + property, + 10, + min, + max, + defval); } static void diff --git a/src/nm-iface-helper.c b/src/nm-iface-helper.c index 6ae2b9d2f7..f79b930ec2 100644 --- a/src/nm-iface-helper.c +++ b/src/nm-iface-helper.c @@ -226,11 +226,11 @@ ndisc_config_changed (NMNDisc *ndisc, const NMNDiscData *rdata, guint changed_in nm_platform_sysctl_set_ip6_hop_limit_safe (NM_PLATFORM_GET, global_opt.ifname, rdata->hop_limit); if (changed & NM_NDISC_CONFIG_MTU) { - char val[16]; - char sysctl_path_buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; - - g_snprintf (val, sizeof (val), "%d", rdata->mtu); - nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, sysctl_path_buf, global_opt.ifname, "mtu")), val); + nm_platform_sysctl_ip_conf_set_int64 (NM_PLATFORM_GET, + AF_INET6, + global_opt.ifname, + "mtu", + rdata->mtu); } nm_ip6_config_merge (existing, ndisc_config, NM_IP_CONFIG_MERGE_DEFAULT, 0); @@ -389,7 +389,6 @@ main (int argc, char *argv[]) gs_unref_bytes GBytes *client_id = NULL; gs_free NMUtilsIPv6IfaceId *iid = NULL; guint sd_id; - char sysctl_path_buf[NM_UTILS_SYSCTL_IP_CONF_PATH_BUFSIZE]; c_list_init (&gl.dad_failed_lst_head); @@ -500,7 +499,11 @@ main (int argc, char *argv[]) } if (global_opt.dhcp4_address) { - nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET, sysctl_path_buf, global_opt.ifname, "promote_secondaries")), "1"); + nm_platform_sysctl_ip_conf_set (NM_PLATFORM_GET, + AF_INET, + global_opt.ifname, + "promote_secondaries", + "1"); dhcp4_client = nm_dhcp_manager_start_ip4 (nm_dhcp_manager_get (), nm_platform_get_multi_idx (NM_PLATFORM_GET), @@ -552,10 +555,10 @@ main (int argc, char *argv[]) if (iid) nm_ndisc_set_iid (ndisc, *iid); - nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, sysctl_path_buf, global_opt.ifname, "accept_ra")), "1"); - nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, sysctl_path_buf, global_opt.ifname, "accept_ra_defrtr")), "0"); - nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, sysctl_path_buf, global_opt.ifname, "accept_ra_pinfo")), "0"); - nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_sysctl_ip_conf_path (AF_INET6, sysctl_path_buf, global_opt.ifname, "accept_ra_rtr_pref")), "0"); + nm_platform_sysctl_ip_conf_set (NM_PLATFORM_GET, AF_INET6, global_opt.ifname, "accept_ra", "1"); + nm_platform_sysctl_ip_conf_set (NM_PLATFORM_GET, AF_INET6, global_opt.ifname, "accept_ra_defrtr", "0"); + nm_platform_sysctl_ip_conf_set (NM_PLATFORM_GET, AF_INET6, global_opt.ifname, "accept_ra_pinfo", "0"); + nm_platform_sysctl_ip_conf_set (NM_PLATFORM_GET, AF_INET6, global_opt.ifname, "accept_ra_rtr_pref", "0"); g_signal_connect (NM_PLATFORM_GET, NM_PLATFORM_SIGNAL_IP6_ADDRESS_CHANGED, From 91b5babff2dcab7739b8002c16aec70921356f09 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 13 Dec 2018 12:55:51 +0100 Subject: [PATCH 07/10] core/trivial: rename nm_platform_sysctl_set_ip6_hop_limit_safe() Now that we have other helper function on platfrom for setting IP configuration sysctls, rename the function to set the hop-limit to match the pattern. --- src/devices/nm-device.c | 2 +- src/nm-iface-helper.c | 2 +- src/platform/nm-platform.c | 4 +++- src/platform/nm-platform.h | 4 +++- 4 files changed, 8 insertions(+), 4 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 4e9ac076ff..2acbc554f3 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -9414,7 +9414,7 @@ ndisc_config_changed (NMNDisc *ndisc, const NMNDiscData *rdata, guint changed_in } if (changed & NM_NDISC_CONFIG_HOP_LIMIT) - nm_platform_sysctl_set_ip6_hop_limit_safe (nm_device_get_platform (self), nm_device_get_ip_iface (self), rdata->hop_limit); + nm_platform_sysctl_ip_conf_set_ipv6_hop_limit_safe (nm_device_get_platform (self), nm_device_get_ip_iface (self), rdata->hop_limit); if (changed & NM_NDISC_CONFIG_MTU) { if (priv->ip6_mtu != rdata->mtu) { diff --git a/src/nm-iface-helper.c b/src/nm-iface-helper.c index f79b930ec2..1229ad35cf 100644 --- a/src/nm-iface-helper.c +++ b/src/nm-iface-helper.c @@ -223,7 +223,7 @@ ndisc_config_changed (NMNDisc *ndisc, const NMNDiscData *rdata, guint changed_in } if (changed & NM_NDISC_CONFIG_HOP_LIMIT) - nm_platform_sysctl_set_ip6_hop_limit_safe (NM_PLATFORM_GET, global_opt.ifname, rdata->hop_limit); + nm_platform_sysctl_ip_conf_set_ipv6_hop_limit_safe (NM_PLATFORM_GET, global_opt.ifname, rdata->hop_limit); if (changed & NM_NDISC_CONFIG_MTU) { nm_platform_sysctl_ip_conf_set_int64 (NM_PLATFORM_GET, diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 6adbb815ce..dd119b9970 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -476,7 +476,9 @@ nm_platform_sysctl_set (NMPlatform *self, const char *pathid, int dirfd, const c } gboolean -nm_platform_sysctl_set_ip6_hop_limit_safe (NMPlatform *self, const char *iface, int value) +nm_platform_sysctl_ip_conf_set_ipv6_hop_limit_safe (NMPlatform *self, + const char *iface, + int value) { const char *path; gint64 cur; diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index ece106bfe2..59d4eb45c5 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -1154,7 +1154,9 @@ gboolean nm_platform_sysctl_ip_conf_set_int64 (NMPlatform *platform, const char *property, gint64 value); -gboolean nm_platform_sysctl_set_ip6_hop_limit_safe (NMPlatform *self, const char *iface, int value); +gboolean nm_platform_sysctl_ip_conf_set_ipv6_hop_limit_safe (NMPlatform *self, + const char *iface, + int value); const char *nm_platform_if_indextoname (NMPlatform *self, int ifindex, char *out_ifname/* of size IFNAMSIZ */); int nm_platform_if_nametoindex (NMPlatform *self, const char *ifname); From f9077fa74da0dc2657d81bc8f696e7f49d0a91a7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 13 Dec 2018 13:18:13 +0100 Subject: [PATCH 08/10] device: add nm_device_get_ip_iface_from_platform() We have a cached nm_device_get_ip_iface() property. However, the interface name is not an identifier for a link because it can change at any time. Also, we already have the (ip) ifindex as proper identifier for the platform link. We shouldn't use two redundant identifiers to refer to a link. Clearly, sometimes we need an ifname. For example for ethtool ioctl or sysctl path names. For ethtool API, we resolve the actual name as late as possible, and for sysctl API we prefer NMP_SYSCTL_PATHID_NETDIR*(). However, that is not always possible, for example for /proc/sys/net/ipv6/conf/ sysctls. Add a function that resolves the ifname by looking into the cache. This of course is still racy, but it minimizes the time. Also, we should less and less rely on the ifname, and resolve it as late as possible. This patch adds a small wrapper going into that direction. --- src/devices/nm-device.c | 12 ++++++++++++ src/devices/nm-device.h | 1 + 2 files changed, 13 insertions(+) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 2acbc554f3..bee031fd1b 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1480,6 +1480,18 @@ nm_device_get_ip_iface (NMDevice *self) return priv->ip_iface ?: priv->iface; } +const char * +nm_device_get_ip_iface_from_platform (NMDevice *self) +{ + int ifindex; + + ifindex = nm_device_get_ip_ifindex (self); + if (ifindex <= 0) + return NULL; + + return nm_platform_link_get_name (nm_device_get_platform (self), ifindex); +} + int nm_device_get_ip_ifindex (const NMDevice *self) { diff --git a/src/devices/nm-device.h b/src/devices/nm-device.h index 1778faef92..c2e3c474fb 100644 --- a/src/devices/nm-device.h +++ b/src/devices/nm-device.h @@ -485,6 +485,7 @@ int nm_device_get_ifindex (NMDevice *dev); gboolean nm_device_is_software (NMDevice *dev); gboolean nm_device_is_real (NMDevice *dev); const char * nm_device_get_ip_iface (NMDevice *dev); +const char * nm_device_get_ip_iface_from_platform (NMDevice *dev); int nm_device_get_ip_ifindex (const NMDevice *dev); const char * nm_device_get_driver (NMDevice *dev); const char * nm_device_get_driver_version (NMDevice *dev); From 8bf6ae1b7f9b4d56f7ce14716f150457a4ae2bda Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 13 Dec 2018 11:32:05 +0100 Subject: [PATCH 09/10] device: add sysctl-ip-conf getter and use it - add nm_device_sysctl_ip_conf_get() and nm_device_sysctl_ip_conf_get_int_checked(). These functions don't use nm_device_get_ip_iface(), but resolve the ifname from the platform cache. - in general, resolve the name first with nm_device_get_ip_iface_from_platform(). --- src/devices/nm-device.c | 134 ++++++++++++++++++++++++---------------- 1 file changed, 80 insertions(+), 54 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index bee031fd1b..a6fe682f4c 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -1135,6 +1135,49 @@ init_ip_config_dns_priority (NMDevice *self, NMIPConfig *config) /*****************************************************************************/ +static char * +nm_device_sysctl_ip_conf_get (NMDevice *self, + int addr_family, + const char *property) +{ + const char *ifname; + + nm_assert_addr_family (addr_family); + + ifname = nm_device_get_ip_iface_from_platform (self); + if (!ifname) + return NULL; + return nm_platform_sysctl_ip_conf_get (nm_device_get_platform (self), addr_family, ifname, property); +} + +static gint64 +nm_device_sysctl_ip_conf_get_int_checked (NMDevice *self, + int addr_family, + const char *property, + guint base, + gint64 min, + gint64 max, + gint64 fallback) +{ + const char *ifname; + + nm_assert_addr_family (addr_family); + + ifname = nm_device_get_ip_iface_from_platform (self); + if (!ifname) { + errno = EINVAL; + return fallback; + } + return nm_platform_sysctl_ip_conf_get_int_checked (nm_device_get_platform (self), + addr_family, + ifname, + property, + base, + min, + max, + fallback); +} + gboolean nm_device_sysctl_ip_conf_set (NMDevice *self, int addr_family, @@ -1144,15 +1187,10 @@ nm_device_sysctl_ip_conf_set (NMDevice *self, NMPlatform *platform = nm_device_get_platform (self); gs_free char *value_to_free = NULL; const char *ifname; - int ifindex; nm_assert_addr_family (addr_family); - ifindex = nm_device_get_ip_ifindex (self); - if (ifindex <= 0) - return FALSE; - - ifname = nm_platform_link_get_name (platform, ifindex); + ifname = nm_device_get_ip_iface_from_platform (self); if (!ifname) return FALSE; @@ -1177,9 +1215,11 @@ nm_device_sysctl_ip_conf_set (NMDevice *self, static guint32 nm_device_sysctl_ip_conf_get_effective_uint32 (NMDevice *self, const char *property, guint32 fallback) { - gint64 v, v_all; + const char *ifname; + gint64 v_cur, v_all; - if (!nm_device_get_ip_ifindex (self)) + ifname = nm_device_get_ip_iface_from_platform (self); + if (!ifname) return fallback; /* for this kind of sysctl (e.g. "rp_filter"), kernel effectively uses the @@ -1187,14 +1227,14 @@ nm_device_sysctl_ip_conf_get_effective_uint32 (NMDevice *self, const char *prope * * Also do that, by reading both sysctls and return the maximum. */ - v = nm_platform_sysctl_ip_conf_get_int_checked (nm_device_get_platform (self), - AF_INET, - nm_device_get_ip_iface (self), - property, - 10, - 0, - G_MAXUINT32, - -1); + v_cur = nm_platform_sysctl_ip_conf_get_int_checked (nm_device_get_platform (self), + AF_INET, + ifname, + property, + 10, + 0, + G_MAXUINT32, + -1); v_all = nm_platform_sysctl_ip_conf_get_int_checked (nm_device_get_platform (self), AF_INET, @@ -1205,24 +1245,8 @@ nm_device_sysctl_ip_conf_get_effective_uint32 (NMDevice *self, const char *prope G_MAXUINT32, -1); - v = NM_MAX (v, v_all); - return v > -1 ? (guint32) v : fallback; -} - -static guint32 -nm_device_sysctl_ip_conf_get_uint32 (NMDevice *self, const char *property, guint32 fallback) -{ - if (!nm_device_get_ip_ifindex (self)) - return fallback; - - return nm_platform_sysctl_ip_conf_get_int_checked (nm_device_get_platform (self), - AF_INET6, - nm_device_get_ip_iface (self), - property, - 10, - 0, - G_MAXUINT32, - fallback); + v_cur = NM_MAX (v_cur, v_all); + return v_cur > -1 ? (guint32) v_cur : fallback; } /*****************************************************************************/ @@ -9252,7 +9276,7 @@ _commit_mtu (NMDevice *self, const NMIP4Config *config) #define _IP6_MTU_SYS() \ ({ \ if (!ip6_mtu_sysctl.initialized) { \ - ip6_mtu_sysctl.value = nm_device_sysctl_ip_conf_get_uint32 (self, "mtu", 0); \ + ip6_mtu_sysctl.value = nm_device_sysctl_ip_conf_get_int_checked (self, AF_INET6, "mtu", 10, 0, G_MAXUINT32, 0); \ ip6_mtu_sysctl.initialized = TRUE; \ } \ ip6_mtu_sysctl.value; \ @@ -9613,32 +9637,33 @@ addrconf6_cleanup (NMDevice *self) /*****************************************************************************/ -static const char *ip6_properties_to_save[] = { - "accept_ra", - "accept_ra_defrtr", - "accept_ra_pinfo", - "accept_ra_rtr_pref", - "forwarding", - "disable_ipv6", - "hop_limit", - "use_tempaddr", -}; - static void save_ip6_properties (NMDevice *self) { + static const char *const ip6_properties_to_save[] = { + "accept_ra", + "accept_ra_defrtr", + "accept_ra_pinfo", + "accept_ra_rtr_pref", + "forwarding", + "disable_ipv6", + "hop_limit", + "use_tempaddr", + }; NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - const char *ifname = nm_device_get_ip_iface (self); + NMPlatform *platform = nm_device_get_platform (self); + const char *ifname; char *value; int i; g_hash_table_remove_all (priv->ip6_saved_properties); - if (!nm_device_get_ip_ifindex (self)) + ifname = nm_device_get_ip_iface_from_platform (self); + if (!ifname) return; for (i = 0; i < G_N_ELEMENTS (ip6_properties_to_save); i++) { - value = nm_platform_sysctl_ip_conf_get (nm_device_get_platform (self), + value = nm_platform_sysctl_ip_conf_get (platform, AF_INET6, ifname, ip6_properties_to_save[i]); @@ -9705,10 +9730,9 @@ set_nm_ipv6ll (NMDevice *self, gboolean enable) gs_free char *value = NULL; /* Bounce IPv6 to ensure the kernel stops IPv6LL address generation */ - value = nm_platform_sysctl_ip_conf_get (nm_device_get_platform (self), - AF_INET6, - nm_device_get_ip_iface (self), - "disable_ipv6"); + value = nm_device_sysctl_ip_conf_get (self, + AF_INET6, + "disable_ipv6"); if (nm_streq0 (value, "0")) nm_device_sysctl_ip_conf_set (self, AF_INET6, "disable_ipv6", "1"); @@ -9774,7 +9798,9 @@ _ip6_privacy_get (NMDevice *self) * Instead of reading static config files in /etc, just read the current sysctl value. * This works as NM only writes to "/proc/sys/net/ipv6/conf/IFNAME/use_tempaddr", but leaves * the "default" entry untouched. */ - ip6_privacy = nm_platform_sysctl_get_int32 (nm_device_get_platform (self), NMP_SYSCTL_PATHID_ABSOLUTE ("/proc/sys/net/ipv6/conf/default/use_tempaddr"), NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN); + ip6_privacy = nm_platform_sysctl_get_int32 (nm_device_get_platform (self), + NMP_SYSCTL_PATHID_ABSOLUTE ("/proc/sys/net/ipv6/conf/default/use_tempaddr"), + NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN); return _ip6_privacy_clamp (ip6_privacy); } From a936086d14becf59b2db2bd0c94bd852d48c65c9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 13 Dec 2018 11:14:06 +0100 Subject: [PATCH 10/10] device: drop rp_filter handling After commit b1082aa9a711deb96652e5b2fcaefcf399d127b8 (device: disable rp_filter handling) drop the now unused code. https://bugzilla.redhat.com/show_bug.cgi?id=1651097 --- src/devices/nm-device.c | 172 ---------------------------------------- 1 file changed, 172 deletions(-) diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index a6fe682f4c..3dbe5839ee 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -457,9 +457,6 @@ typedef struct _NMDevicePrivate { AppliedConfig wwan_ip_config_x[2]; }; - bool v4_has_shadowed_routes; - const char *ip4_rp_filter; - /* DHCPv4 tracking */ struct { NMDhcpClient * client; @@ -1212,43 +1209,6 @@ nm_device_sysctl_ip_conf_set (NMDevice *self, value); } -static guint32 -nm_device_sysctl_ip_conf_get_effective_uint32 (NMDevice *self, const char *property, guint32 fallback) -{ - const char *ifname; - gint64 v_cur, v_all; - - ifname = nm_device_get_ip_iface_from_platform (self); - if (!ifname) - return fallback; - - /* for this kind of sysctl (e.g. "rp_filter"), kernel effectively uses the - * MAX of the per-device value and the "all" value. - * - * Also do that, by reading both sysctls and return the maximum. */ - - v_cur = nm_platform_sysctl_ip_conf_get_int_checked (nm_device_get_platform (self), - AF_INET, - ifname, - property, - 10, - 0, - G_MAXUINT32, - -1); - - v_all = nm_platform_sysctl_ip_conf_get_int_checked (nm_device_get_platform (self), - AF_INET, - "all", - property, - 10, - 0, - G_MAXUINT32, - -1); - - v_cur = NM_MAX (v_cur, v_all); - return v_cur > -1 ? (guint32) v_cur : fallback; -} - /*****************************************************************************/ gboolean @@ -3946,126 +3906,6 @@ link_changed_cb (NMPlatform *platform, /*****************************************************************************/ -typedef struct { - in_addr_t network; - guint8 plen; -} IP4RPFilterData; - -static guint -_v4_has_shadowed_routes_detect_hash (const IP4RPFilterData *d) -{ - NMHashState h; - - nm_hash_init (&h, 1105201169u); - nm_hash_update_vals (&h, - d->network, - d->plen); - return nm_hash_complete (&h); -} - -static gboolean -_v4_has_shadowed_routes_detect_equal (const IP4RPFilterData *d1, const IP4RPFilterData *d2) -{ - return d1->network == d2->network && d1->plen == d2->plen; -} - -static gboolean -_v4_has_shadowed_routes_detect (NMDevice *self) -{ - NMPlatform *platform; - int ifindex; - NMPLookup lookup; - const NMDedupMultiHeadEntry *head_entry; - NMDedupMultiIter iter; - const NMPObject *o; - guint data_len; - gs_unref_hashtable GHashTable *data_hash = NULL; - gs_free IP4RPFilterData *data_arr = NULL; - - ifindex = nm_device_get_ip_ifindex (self); - if (ifindex <= 0) - return FALSE; - - platform = nm_device_get_platform (self); - - head_entry = nm_platform_lookup (platform, - nmp_lookup_init_object (&lookup, - NMP_OBJECT_TYPE_IP4_ROUTE, - ifindex)); - if (!head_entry) - return FALSE; - - /* first, create a lookup index @data_hash for all network/plen pairs. */ - data_len = 0; - data_arr = g_new (IP4RPFilterData, head_entry->len); - data_hash = g_hash_table_new ((GHashFunc) _v4_has_shadowed_routes_detect_hash, - (GEqualFunc) _v4_has_shadowed_routes_detect_equal); - - nmp_cache_iter_for_each (&iter, head_entry, &o) { - const NMPlatformIP4Route *r = NMP_OBJECT_CAST_IP4_ROUTE (o); - IP4RPFilterData *d; - - nm_assert (r->ifindex == ifindex); - - if ( NM_PLATFORM_IP_ROUTE_IS_DEFAULT (r) - || r->table_coerced) - continue; - - d = &data_arr[data_len++]; - d->network = nm_utils_ip4_address_clear_host_address (r->network, r->plen); - d->plen = r->plen; - g_hash_table_add (data_hash, d); - } - - /* then, search if there is any route on another interface with the same - * network/plen destination. If yes, we consider this a multihoming - * setup. */ - head_entry = nm_platform_lookup (platform, - nmp_lookup_init_obj_type (&lookup, - NMP_OBJECT_TYPE_IP4_ROUTE)); - nmp_cache_iter_for_each (&iter, head_entry, &o) { - const NMPlatformIP4Route *r = NMP_OBJECT_CAST_IP4_ROUTE (o); - IP4RPFilterData d; - - if ( r->ifindex == ifindex - || NM_PLATFORM_IP_ROUTE_IS_DEFAULT (r) - || r->table_coerced) - continue; - - d.network = nm_utils_ip4_address_clear_host_address (r->network, r->plen); - d.plen = r->plen; - if (g_hash_table_contains (data_hash, &d)) - return TRUE; - } - - return FALSE; -} - -static void -ip4_rp_filter_update (NMDevice *self) -{ - NMDevicePrivate *priv = NM_DEVICE_GET_PRIVATE (self); - const char *ip4_rp_filter; - - if ( priv->v4_has_shadowed_routes - || nm_device_get_best_default_route (self, AF_INET)) { - if (nm_device_sysctl_ip_conf_get_effective_uint32 (self, "rp_filter", 0) != 1) { - /* Don't touch the rp_filter if it's not strict. */ - return; - } - /* Loose rp_filter */ - ip4_rp_filter = "2"; - } else { - /* Default rp_filter */ - ip4_rp_filter = NULL; - } - - if (ip4_rp_filter != priv->ip4_rp_filter) { - nm_device_sysctl_ip_conf_set (self, AF_INET, "rp_filter", ip4_rp_filter); - priv->ip4_rp_filter = ip4_rp_filter; - } -} - static void link_changed (NMDevice *self, const NMPlatformLink *pllink) { @@ -12254,11 +12094,6 @@ nm_device_set_ip_config (NMDevice *self, priv->needs_ip6_subnet = FALSE; } - if (IS_IPv4 && FALSE /* rp_filter handling is disabled */) { - if (!nm_device_sys_iface_state_is_external_or_assume (self)) - ip4_rp_filter_update (self); - } - if (has_changes) { if (IS_IPv4) @@ -13153,13 +12988,6 @@ queued_ip_config_change (NMDevice *self, int addr_family) set_unmanaged_external_down (self, TRUE); - if (IS_IPv4 && FALSE /* rp_filter handling is disabled */) { - if (!nm_device_sys_iface_state_is_external_or_assume (self)) { - priv->v4_has_shadowed_routes = _v4_has_shadowed_routes_detect (self);; - ip4_rp_filter_update (self); - } - } - return G_SOURCE_REMOVE; }