From eab817d34a38227a79b10e9c52d450bb8c7fa907 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 20 Apr 2016 11:44:23 +0200 Subject: [PATCH 1/6] platform: restrict valid p_key for infiniband partitions --- src/platform/nm-linux-platform.c | 2 ++ src/platform/nm-platform.c | 6 +++++- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 254f9c855d..f550a85cfa 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5085,6 +5085,8 @@ _infiniband_partition_action (NMPlatform *platform, int parent, int p_key, const gs_free char *path = NULL; gs_free char *id = NULL; + nm_assert (p_key > 0 && p_key <= 0xffff && p_key != 0x8000); + obj_parent = nmp_cache_lookup_link (priv->cache, parent); if (!obj_parent || !obj_parent->link.name[0]) g_return_val_if_reached (FALSE); diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 0139867a0c..43532ebf3e 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -1902,7 +1902,11 @@ _infiniband_add_add_or_delete (NMPlatform *self, _CHECK_SELF (self, klass, NM_PLATFORM_ERROR_BUG); g_return_val_if_fail (parent >= 0, NM_PLATFORM_ERROR_BUG); - g_return_val_if_fail (p_key >= 0, NM_PLATFORM_ERROR_BUG); + g_return_val_if_fail (p_key >= 0 && p_key <= 0xffff, NM_PLATFORM_ERROR_BUG); + + /* the special keys 0x0000 and 0x8000 are not allowed. */ + if (NM_IN_SET (p_key, 0, 0x8000)) + return NM_PLATFORM_ERROR_UNSPECIFIED; parent_link = nm_platform_link_get (self, parent); if (!parent_link) From 67d45ea1d3871392fa2450e07f6db88bff19039e Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 20 Apr 2016 11:09:25 +0200 Subject: [PATCH 2/6] platform: preserve errno in nm_platform_sysctl_set() We want to preserve the relevant errno during nm_platform_sysctl_set(). Also, if the final close() fails, fail altogether. --- src/platform/nm-linux-platform.c | 40 +++++++++++++++++++++++++------- 1 file changed, 31 insertions(+), 9 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index f550a85cfa..ab2c440f0d 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -2473,6 +2473,7 @@ sysctl_set (NMPlatform *platform, const char *path, const char *value) gsize len; char *actual; gs_free char *actual_free = NULL; + int errsv; g_return_val_if_fail (path != NULL, FALSE); g_return_val_if_fail (value != NULL, FALSE); @@ -2483,18 +2484,22 @@ sysctl_set (NMPlatform *platform, const char *path, const char *value) /* Don't write to suspicious locations */ g_assert (!strstr (path, "/../")); - if (!nm_platform_netns_push (platform, &netns)) + if (!nm_platform_netns_push (platform, &netns)) { + errno = ENETDOWN; return FALSE; + } fd = open (path, O_WRONLY | O_TRUNC); if (fd == -1) { - if (errno == ENOENT) { + errsv = errno; + if (errsv == ENOENT) { _LOGD ("sysctl: failed to open '%s': (%d) %s", - path, errno, strerror (errno)); + path, errsv, strerror (errsv)); } else { _LOGE ("sysctl: failed to open '%s': (%d) %s", - path, errno, strerror (errno)); + path, errsv, strerror (errsv)); } + errno = errsv; return FALSE; } @@ -2515,26 +2520,43 @@ sysctl_set (NMPlatform *platform, const char *path, const char *value) actual[len] = '\0'; /* Try to write the entire value three times if a partial write occurs */ + errsv = 0; for (tries = 0, nwrote = 0; tries < 3 && nwrote != len; tries++) { nwrote = write (fd, actual, len); if (nwrote == -1) { - if (errno == EINTR) { + errsv = errno; + if (errsv == EINTR) { _LOGD ("sysctl: interrupted, will try again"); continue; } break; } } - if (nwrote == -1 && errno != EEXIST) { + if (nwrote == -1 && errsv != EEXIST) { _LOGE ("sysctl: failed to set '%s' to '%s': (%d) %s", - path, value, errno, strerror (errno)); + path, value, errsv, strerror (errsv)); } else if (nwrote < len) { _LOGE ("sysctl: failed to set '%s' to '%s' after three attempts", path, value); } - close (fd); - return (nwrote == len); + if (nwrote != len) { + if (close (fd) != 0) { + if (errsv != 0) + errno = errsv; + } else if (errsv != 0) + errno = errsv; + else + errno = EIO; + return FALSE; + } + if (close (fd) != 0) { + /* errno is already properly set. */ + return FALSE; + } + + /* success. errno is undefined (no need to set). */ + return TRUE; } static GSList *sysctl_clear_cache_list; From 8f7029d132d81f8d454cccfeb8381001bf405fa9 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 20 Apr 2016 10:45:31 +0200 Subject: [PATCH 3/6] core: add nm_utils_new_infiniband_name() util --- src/nm-core-utils.c | 27 +++++++++++++++++++++++++++ src/nm-core-utils.h | 1 + 2 files changed, 28 insertions(+) diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index bbe466545d..6ece1b089c 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -1866,6 +1866,33 @@ nm_utils_new_vlan_name (const char *parent_iface, guint32 vlan_id) return ifname; } +/* nm_utils_new_infiniband_name: + * @name: the output-buffer where the value will be written. Must be + * not %NULL and point to a string buffer of at least IFNAMSIZ bytes. + * @parent_name: the parent interface name + * @p_key: the partition key. + * + * Returns: the infiniband name will be written to @name and @name + * is returned. + */ +const char * +nm_utils_new_infiniband_name (char *name, const char *parent_name, int p_key) +{ + g_return_val_if_fail (name, NULL); + g_return_val_if_fail (parent_name && parent_name[0], NULL); + g_return_val_if_fail (strlen (parent_name) < IFNAMSIZ, NULL); + + /* technically, p_key of 0x0000 and 0x8000 is not allowed either. But we don't + * want to assert against that in nm_utils_new_infiniband_name(). So be more + * resilient here, and accept those. */ + g_return_val_if_fail (p_key >= 0 && p_key <= 0xffff, NULL); + + /* If parent+suffix is too long, kernel would just truncate + * the name. We do the same. See ipoib_vlan_add(). */ + g_snprintf (name, IFNAMSIZ, "%s.%04x", parent_name, p_key); + return name; +} + /** * nm_utils_read_resolv_conf_nameservers(): * @rc_contents: contents of a resolv.conf; or %NULL to read /etc/resolv.conf diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h index 528288c34c..486942a22a 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -281,6 +281,7 @@ const char *nm_utils_get_ip_config_method (NMConnection *connection, GType ip_setting_type); char *nm_utils_new_vlan_name (const char *parent_iface, guint32 vlan_id); +const char *nm_utils_new_infiniband_name (char *name, const char *parent_name, int p_key); GPtrArray *nm_utils_read_resolv_conf_nameservers (const char *rc_contents); GPtrArray *nm_utils_read_resolv_conf_dns_options (const char *rc_contents); From 9c323261ea15fb8162c9ce3f7db39e8049b0f5f8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 20 Apr 2016 11:28:38 +0200 Subject: [PATCH 4/6] platform: use nm_utils_new_infiniband_name() --- src/platform/nm-fake-platform.c | 8 ++++---- src/platform/nm-linux-platform.c | 23 ++++++++++++----------- src/platform/nm-platform.c | 4 ++-- 3 files changed, 18 insertions(+), 17 deletions(-) diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index 7b54507696..b0d4411507 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -709,12 +709,13 @@ static gboolean infiniband_partition_add (NMPlatform *platform, int parent, int p_key, const NMPlatformLink **out_link) { NMFakePlatformLink *device, *parent_device; - gs_free char *name = NULL; + char name[IFNAMSIZ]; parent_device = link_get (platform, parent); g_return_val_if_fail (parent_device != NULL, FALSE); - name = g_strdup_printf ("%s.%04x", parent_device->link.name, p_key); + nm_utils_new_infiniband_name (name, parent_device->link.name, p_key); + if (!link_add (platform, name, NM_LINK_TYPE_INFINIBAND, NULL, 0, out_link)) return FALSE; @@ -726,7 +727,6 @@ infiniband_partition_add (NMPlatform *platform, int parent, int p_key, const NMP device->lnk->lnk_infiniband.p_key = p_key; device->lnk->lnk_infiniband.mode = "datagram"; device->link.parent = parent; - return TRUE; } @@ -739,7 +739,7 @@ infiniband_partition_delete (NMPlatform *platform, int parent, int p_key) parent_device = link_get (platform, parent); g_return_val_if_fail (parent_device != NULL, FALSE); - name = g_strdup_printf ("%s.%04x", parent_device->link.name, p_key); + nm_utils_new_infiniband_name (name, parent_device->link.name, p_key); return link_delete (platform, nm_platform_link_get_ifindex (platform, name)); } diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index ab2c440f0d..529d1dd2d1 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5100,7 +5100,7 @@ link_release (NMPlatform *platform, int master, int slave) /******************************************************************/ static gboolean -_infiniband_partition_action (NMPlatform *platform, int parent, int p_key, const char *action, char **ifname) +_infiniband_partition_action (NMPlatform *platform, int parent, int p_key, const char *action, char *out_partition_name) { NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); const NMPObject *obj_parent; @@ -5110,10 +5110,13 @@ _infiniband_partition_action (NMPlatform *platform, int parent, int p_key, const nm_assert (p_key > 0 && p_key <= 0xffff && p_key != 0x8000); obj_parent = nmp_cache_lookup_link (priv->cache, parent); - if (!obj_parent || !obj_parent->link.name[0]) - g_return_val_if_reached (FALSE); + if (!obj_parent || !obj_parent->link.name[0]) { + errno = ENOENT; + return FALSE; + } - *ifname = g_strdup_printf ("%s.%04x", obj_parent->link.name, p_key); + if (out_partition_name) + nm_utils_new_infiniband_name (out_partition_name, obj_parent->link.name, p_key); path = g_strdup_printf ("/sys/class/net/%s/%s", NM_ASSERT_VALID_PATH_COMPONENT (obj_parent->link.name), @@ -5128,15 +5131,15 @@ static gboolean infiniband_partition_add (NMPlatform *platform, int parent, int p_key, const NMPlatformLink **out_link) { const NMPObject *obj; - gs_free char *ifname = NULL; + char name[IFNAMSIZ]; - if (!_infiniband_partition_action (platform, parent, p_key, "create_child", &ifname)) + if (!_infiniband_partition_action (platform, parent, p_key, "create_child", name)) return FALSE; - do_request_link (platform, 0, ifname); + do_request_link (platform, 0, name); obj = nmp_cache_lookup_link_full (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, - 0, ifname, FALSE, NM_LINK_TYPE_INFINIBAND, NULL, NULL); + 0, name, FALSE, NM_LINK_TYPE_INFINIBAND, NULL, NULL); if (out_link) *out_link = obj ? &obj->link : NULL; return !!obj; @@ -5145,9 +5148,7 @@ infiniband_partition_add (NMPlatform *platform, int parent, int p_key, const NMP static gboolean infiniband_partition_delete (NMPlatform *platform, int parent, int p_key) { - gs_free char *ifname = NULL; - - if (!_infiniband_partition_action (platform, parent, p_key, "delete_child", &ifname)) { + if (!_infiniband_partition_action (platform, parent, p_key, "delete_child", NULL)) { if (errno != ENODEV) return FALSE; } diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 43532ebf3e..58c54cb9af 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -1895,7 +1895,7 @@ _infiniband_add_add_or_delete (NMPlatform *self, gboolean add, const NMPlatformLink **out_link) { - gs_free char *name = NULL; + char name[IFNAMSIZ]; const NMPlatformLink *parent_link; NMPlatformError plerr; @@ -1915,7 +1915,7 @@ _infiniband_add_add_or_delete (NMPlatform *self, if (parent_link->type != NM_LINK_TYPE_INFINIBAND) return NM_PLATFORM_ERROR_WRONG_TYPE; - name = g_strdup_printf ("%s.%04x", parent_link->name, p_key); + nm_utils_new_infiniband_name (name, parent_link->name, p_key); if (add) { plerr = _link_add_check_existing (self, name, NM_LINK_TYPE_INFINIBAND, out_link); From b103af0f1ed2e25fd441177d18909af10c724875 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 20 Apr 2016 11:50:45 +0200 Subject: [PATCH 5/6] platform: stack allocate string buffers in _infiniband_partition_action() --- src/platform/nm-linux-platform.c | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 529d1dd2d1..e06aa4607a 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5104,8 +5104,8 @@ _infiniband_partition_action (NMPlatform *platform, int parent, int p_key, const { NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); const NMPObject *obj_parent; - gs_free char *path = NULL; - gs_free char *id = NULL; + char path[NM_STRLEN ("/sys/class/net/%s/%s") + IFNAMSIZ + 100]; + char id[20]; nm_assert (p_key > 0 && p_key <= 0xffff && p_key != 0x8000); @@ -5118,11 +5118,11 @@ _infiniband_partition_action (NMPlatform *platform, int parent, int p_key, const if (out_partition_name) nm_utils_new_infiniband_name (out_partition_name, obj_parent->link.name, p_key); - path = g_strdup_printf ("/sys/class/net/%s/%s", - NM_ASSERT_VALID_PATH_COMPONENT (obj_parent->link.name), - action); - id = g_strdup_printf ("0x%04x", p_key); - + nm_sprintf_buf (path, + "/sys/class/net/%s/%s", + NM_ASSERT_VALID_PATH_COMPONENT (obj_parent->link.name), + action); + nm_sprintf_buf (id, "0x%04x", p_key); return nm_platform_sysctl_set (platform, path, id); } From d52a88a3f8524de999db2279839d579671b81dd1 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Wed, 20 Apr 2016 12:06:43 +0200 Subject: [PATCH 6/6] platform: request link after deleting inifiniband partition After issuing the sysctl "delete_child", we must request the link to get the platform cache in sync. --- src/platform/nm-linux-platform.c | 60 +++++++++++++++++++------------- 1 file changed, 36 insertions(+), 24 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index e06aa4607a..1ce4818a13 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -176,6 +176,11 @@ * Forward declarations and enums ******************************************************************/ +typedef enum { + INFINIBAND_ACTION_CREATE_CHILD, + INFINIBAND_ACTION_DELETE_CHILD, +} InfinibandAction; + enum { DELAYED_ACTION_IDX_REFRESH_ALL_LINKS, DELAYED_ACTION_IDX_REFRESH_ALL_IP4_ADDRESSES, @@ -5100,13 +5105,21 @@ link_release (NMPlatform *platform, int master, int slave) /******************************************************************/ static gboolean -_infiniband_partition_action (NMPlatform *platform, int parent, int p_key, const char *action, char *out_partition_name) +_infiniband_partition_action (NMPlatform *platform, + InfinibandAction action, + int parent, + int p_key, + const NMPlatformLink **out_link) { NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); const NMPObject *obj_parent; + const NMPObject *obj; char path[NM_STRLEN ("/sys/class/net/%s/%s") + IFNAMSIZ + 100]; char id[20]; + char name[IFNAMSIZ]; + gboolean success; + nm_assert (NM_IN_SET (action, INFINIBAND_ACTION_CREATE_CHILD, INFINIBAND_ACTION_DELETE_CHILD)); nm_assert (p_key > 0 && p_key <= 0xffff && p_key != 0x8000); obj_parent = nmp_cache_lookup_link (priv->cache, parent); @@ -5115,45 +5128,44 @@ _infiniband_partition_action (NMPlatform *platform, int parent, int p_key, const return FALSE; } - if (out_partition_name) - nm_utils_new_infiniband_name (out_partition_name, obj_parent->link.name, p_key); - nm_sprintf_buf (path, "/sys/class/net/%s/%s", NM_ASSERT_VALID_PATH_COMPONENT (obj_parent->link.name), - action); + (action == INFINIBAND_ACTION_CREATE_CHILD + ? "create_child" + : "delete_child")); nm_sprintf_buf (id, "0x%04x", p_key); - return nm_platform_sysctl_set (platform, path, id); -} - - -static gboolean -infiniband_partition_add (NMPlatform *platform, int parent, int p_key, const NMPlatformLink **out_link) -{ - const NMPObject *obj; - char name[IFNAMSIZ]; - - if (!_infiniband_partition_action (platform, parent, p_key, "create_child", name)) + success = nm_platform_sysctl_set (platform, path, id); + if (!success) { + if ( action == INFINIBAND_ACTION_DELETE_CHILD + && errno == ENODEV) + return TRUE; return FALSE; + } + nm_utils_new_infiniband_name (name, obj_parent->link.name, p_key); do_request_link (platform, 0, name); - obj = nmp_cache_lookup_link_full (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, - 0, name, FALSE, NM_LINK_TYPE_INFINIBAND, NULL, NULL); + if (action == INFINIBAND_ACTION_DELETE_CHILD) + return TRUE; + + obj = nmp_cache_lookup_link_full (priv->cache, 0, name, FALSE, + NM_LINK_TYPE_INFINIBAND, NULL, NULL); if (out_link) *out_link = obj ? &obj->link : NULL; return !!obj; } +static gboolean +infiniband_partition_add (NMPlatform *platform, int parent, int p_key, const NMPlatformLink **out_link) +{ + return _infiniband_partition_action (platform, INFINIBAND_ACTION_CREATE_CHILD, parent, p_key, out_link); +} + static gboolean infiniband_partition_delete (NMPlatform *platform, int parent, int p_key) { - if (!_infiniband_partition_action (platform, parent, p_key, "delete_child", NULL)) { - if (errno != ENODEV) - return FALSE; - } - - return TRUE; + return _infiniband_partition_action (platform, INFINIBAND_ACTION_DELETE_CHILD, parent, p_key, NULL); } /******************************************************************/