From 7744fdd00f9bb32643cd02f81becf34926e2f1c7 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 9 Dec 2016 10:11:29 +0100 Subject: [PATCH 01/27] platform: rename _assert_netns_current() to a ASSERT_NETNS_CURRENT() This assert, although being a regular function has more the character of a macro as it only contains asserts itself. --- src/platform/nm-linux-platform.c | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index ab2ab9b017..79717681da 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -239,7 +239,7 @@ static void do_request_all_no_delayed_actions (NMPlatform *platform, DelayedActi static void cache_pre_hook (NMPCache *cache, const NMPObject *old, const NMPObject *new, NMPCacheOpsType ops_type, gpointer user_data); static void cache_prune_candidates_prune (NMPlatform *platform); static gboolean event_handler_read_netlink (NMPlatform *platform, gboolean wait_for_acks); -static void _assert_netns_current (NMPlatform *platform); +static void ASSERT_NETNS_CURRENT (NMPlatform *platform); /*****************************************************************************/ @@ -614,7 +614,7 @@ _linktype_get_type (NMPlatform *platform, { guint i; - _assert_netns_current (platform); + ASSERT_NETNS_CURRENT (platform); if (completed_from_cache) { const NMPObject *obj; @@ -2481,18 +2481,15 @@ nm_linux_platform_setup (void) NULL); } -/*****************************************************************************/ - static void -_assert_netns_current (NMPlatform *platform) +ASSERT_NETNS_CURRENT (NMPlatform *platform) { -#if NM_MORE_ASSERTS nm_assert (NM_IS_LINUX_PLATFORM (platform)); - nm_assert (NM_IN_SET (nm_platform_netns_get (platform), NULL, nmp_netns_get_current ())); -#endif } +/*****************************************************************************/ + static void _log_dbg_sysctl_set_impl (NMPlatform *platform, const char *path, const char *value) { @@ -2763,8 +2760,7 @@ do_emit_signal (NMPlatform *platform, const NMPObject *obj, NMPCacheOpsType cach nm_assert (!obj || cache_op == NMP_CACHE_OPS_REMOVED || obj == nmp_cache_lookup_obj (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, obj)); nm_assert (!obj || cache_op != NMP_CACHE_OPS_REMOVED || obj != nmp_cache_lookup_obj (NM_LINUX_PLATFORM_GET_PRIVATE (platform)->cache, obj)); - /* we raise the signals inside the namespace of the NMPlatform instance. */ - _assert_netns_current (platform); + ASSERT_NETNS_CURRENT (platform); switch (cache_op) { case NMP_CACHE_OPS_ADDED: From 312cea870dfbc363da44074bd6f56ccd283c5420 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 9 Dec 2016 09:22:29 +0100 Subject: [PATCH 02/27] shared: add nm_auto_close and nm_auto_fclose We already have gs_fd_close, which however doesn't preserve errno and only checks for fd != -1. Add our own define. Downside is, we have to include stdio.h and errno.h, which effectively ends up to be included *everywhere*. --- shared/nm-utils/nm-macros-internal.h | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index fdb45f9291..9fb62b447c 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -22,7 +22,9 @@ #ifndef __NM_MACROS_INTERNAL_H__ #define __NM_MACROS_INTERNAL_H__ +#include #include +#include #include "nm-glib.h" @@ -59,6 +61,30 @@ _nm_auto_free_gstring_impl (GString **str) } #define nm_auto_free_gstring nm_auto(_nm_auto_free_gstring_impl) +static inline void +_nm_auto_close_impl (int *pfd) +{ + if (*pfd >= 0) { + int errsv = errno; + + (void) close (*pfd); + errno = errsv; + } +} +#define nm_auto_close nm_auto(_nm_auto_close_impl) + +static inline void +_nm_auto_fclose_impl (FILE **pfd) +{ + if (*pfd) { + int errsv = errno; + + (void) fclose (*pfd); + errno = errsv; + } +} +#define nm_auto_fclose nm_auto(_nm_auto_fclose_impl) + /*****************************************************************************/ /* http://stackoverflow.com/a/11172679 */ From ed299cc8605a8291a61b3a514f8dc20390b18c77 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 9 Dec 2016 09:27:02 +0100 Subject: [PATCH 03/27] device/wwan: use nm_auto_close instead of gs_fd_close --- src/devices/wwan/nm-modem.c | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index df71017745..1c72c70e04 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -497,18 +497,18 @@ ppp_stats (NMPPPManager *ppp_manager, static gboolean port_speed_is_zero (const char *port) { - struct termios options; - gs_fd_close int fd = -1; + struct termios options; + nm_auto_close int fd = -1; - fd = open (port, O_RDWR | O_NONBLOCK | O_NOCTTY); - if (fd < 0) + fd = open (port, O_RDWR | O_NONBLOCK | O_NOCTTY); + if (fd < 0) return FALSE; - memset (&options, 0, sizeof (struct termios)); - if (tcgetattr (fd, &options) != 0) - return FALSE; + memset (&options, 0, sizeof (struct termios)); + if (tcgetattr (fd, &options) != 0) + return FALSE; - return cfgetospeed (&options) == B0; + return cfgetospeed (&options) == B0; } static NMActStageReturn From ccf766f6596d47b6491553d607ffbd3d9295e0fb Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 9 Dec 2016 09:29:27 +0100 Subject: [PATCH 04/27] platform: preserve errno in nm_auto_pop_netns --- src/platform/nmp-netns.h | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/src/platform/nmp-netns.h b/src/platform/nmp-netns.h index eb33f2ed64..56c1e7e83a 100644 --- a/src/platform/nmp-netns.h +++ b/src/platform/nmp-netns.h @@ -53,8 +53,12 @@ int nmp_netns_get_fd_mnt (NMPNetns *self); static inline void _nm_auto_pop_netns (NMPNetns **p) { - if (*p) + if (*p) { + int errsv = errno; + nmp_netns_pop (*p); + errno = errsv; + } } #define nm_auto_pop_netns __attribute__((cleanup(_nm_auto_pop_netns))) From 215c50922d75ecf75eebc9c871adcdc0fabe3f2d Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 11 Dec 2016 22:33:19 +0100 Subject: [PATCH 05/27] logging: preserve errno in logging functions It would be nice that our logging functions are guaranteed to preserve errno. We are currently not very consistent about handling errno, let's improve on that. --- src/nm-logging.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/nm-logging.c b/src/nm-logging.c index 50927f8caf..c242d90319 100644 --- a/src/nm-logging.c +++ b/src/nm-logging.c @@ -599,6 +599,7 @@ _nm_log_impl (const char *file, va_list args; char *msg; GTimeVal tv; + int errno_saved; if ((guint) level >= G_N_ELEMENTS (_nm_logging_enabled_state)) g_return_if_reached (); @@ -606,6 +607,8 @@ _nm_log_impl (const char *file, if (!(_nm_logging_enabled_state[level] & domain)) return; + errno_saved = errno; + /* Make sure that %m maps to the specified error */ if (error != 0) { if (error < 0) @@ -719,6 +722,8 @@ _nm_log_impl (const char *file, } g_free (msg); + + errno = errno_saved; } /*****************************************************************************/ From 99e1e4d6a19f8fc183ceaea39830c60cdeb1e778 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 11 Dec 2016 22:35:44 +0100 Subject: [PATCH 06/27] shared: add NM_AUTO_PROTECT_ERRNO Similar to systemd's PROTECT_ERRNO. The difference it, that it doesn't treat the auto-variable as internal, so it is allowed to use it. E.g. if (!(fd = open (...)) { NM_AUTO_PROTECT_ERRNO (errno_saved); printf ("error: %s", g_strerror (errno_saved)); return FALSE; } --- shared/nm-utils/nm-macros-internal.h | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/shared/nm-utils/nm-macros-internal.h b/shared/nm-utils/nm-macros-internal.h index 9fb62b447c..24499172ca 100644 --- a/shared/nm-utils/nm-macros-internal.h +++ b/shared/nm-utils/nm-macros-internal.h @@ -85,6 +85,13 @@ _nm_auto_fclose_impl (FILE **pfd) } #define nm_auto_fclose nm_auto(_nm_auto_fclose_impl) +static inline void +_nm_auto_protect_errno (int *p_saved_errno) +{ + errno = *p_saved_errno; +} +#define NM_AUTO_PROTECT_ERRNO(errsv_saved) nm_auto(_nm_auto_protect_errno) _nm_unused const int errsv_saved = (errno) + /*****************************************************************************/ /* http://stackoverflow.com/a/11172679 */ From 713c74f6e4a88f874cf3e9908b3fb153f2ea5b83 Mon Sep 17 00:00:00 2001 From: Kai-Heng Feng Date: Wed, 7 Dec 2016 18:15:13 +0800 Subject: [PATCH 07/27] platform: add a new function nmp_utils_open_sysctl() A race condition may happen when NetworkManager opens sysfs and udev renames interface name at the same time. Thomas Haller provides a new function [1] which can avoid the race condition when opening sysfs. This patch is a direct copy from [1]. [1] https://mail.gnome.org/archives/networkmanager-list/2016-December/msg00004.html Signed-off-by: Kai-Heng Feng --- src/platform/nm-platform-utils.c | 50 ++++++++++++++++++++++++++++++++ src/platform/nm-platform-utils.h | 2 ++ 2 files changed, 52 insertions(+) diff --git a/src/platform/nm-platform-utils.c b/src/platform/nm-platform-utils.c index 9ef0707d58..6c585c21fa 100644 --- a/src/platform/nm-platform-utils.c +++ b/src/platform/nm-platform-utils.c @@ -31,6 +31,7 @@ #include #include #include +#include #include "nm-utils.h" #include "nm-setting-wired.h" @@ -47,6 +48,7 @@ G_STMT_START { (pedata)->speed = (guint16) (speed); } G_STMT_END #endif +extern char *if_indextoname (unsigned int __ifindex, char *__ifname); static gboolean ethtool_get (const char *name, gpointer edata) @@ -623,3 +625,51 @@ nmp_utils_ip_config_source_to_string (NMIPConfigSource source, char *buf, gsize return buf; } +int +nmp_utils_open_sysctl(int ifindex, const char *ifname) +{ + #define SYS_CLASS_NET "/sys/class/net/" + char ifname_buf[IFNAMSIZ]; + guint try_count = 0; + char sysdir[NM_STRLEN (SYS_CLASS_NET) + IFNAMSIZ + 1] = SYS_CLASS_NET; + char fd_buf[256]; + int fd; + int fd_ifindex; + ssize_t nn; + + while (++try_count < 4) { + if (!ifname) { + ifname = if_indextoname (ifindex, ifname_buf); + if (!ifname) + return -1; + } + + nm_utils_ifname_cpy (&sysdir[NM_STRLEN (SYS_CLASS_NET)], ifname); + fd = open (sysdir, O_DIRECTORY); + if (fd < 0) + goto next; + fd_ifindex = openat (fd, "ifindex", 0); + if (fd_ifindex < 0) { + close (fd); + goto next; + } + /* read ifindex file, and compare it to @ifindex. If match, return fd. */ + nn = nm_utils_fd_read_loop (fd_ifindex, fd_buf, sizeof (fd_buf) - 1, FALSE); + if (nn < 0) { + close (fd); + close (fd_ifindex); + goto next; + } + fd_buf[sizeof (fd_buf) - 1] = '\0'; + + if (ifindex != _nm_utils_ascii_str_to_int64 (fd_buf, 10, 1, G_MAXINT, -1)) { + close (fd); + close (fd_ifindex); + goto next; + } + return fd; +next: + ifname = NULL; + } + return -1; +} diff --git a/src/platform/nm-platform-utils.h b/src/platform/nm-platform-utils.h index 4b4868c1e8..72a06a8a92 100644 --- a/src/platform/nm-platform-utils.h +++ b/src/platform/nm-platform-utils.h @@ -61,4 +61,6 @@ NMIPConfigSource nmp_utils_ip_config_source_coerce_from_rtprot (NMIPConfigSource NMIPConfigSource nmp_utils_ip_config_source_round_trip_rtprot (NMIPConfigSource source) _nm_const; const char * nmp_utils_ip_config_source_to_string (NMIPConfigSource source, char *buf, gsize len); +int nmp_utils_open_sysctl(int ifindex, const char *ifname); + #endif /* __NM_PLATFORM_UTILS_H__ */ From b95556eb781a18ee1c96470f40b9e1e162b0ee60 Mon Sep 17 00:00:00 2001 From: Kai-Heng Feng Date: Wed, 7 Dec 2016 18:40:09 +0800 Subject: [PATCH 08/27] platform: wifi: use nmp_utils_open_sysctl() to check if device is wifi Since function nmp_utils_open_sysctl() can avoid race condition, use it in wifi_utils_is_wifi() to open sysfs and correctly check if it's a wifi device. https://bugzilla.gnome.org/show_bug.cgi?id=775613 Signed-off-by: Kai-Heng Feng --- src/platform/nm-linux-platform.c | 2 +- src/platform/wifi/wifi-utils.c | 33 +++++++++++++++++++++++--------- src/platform/wifi/wifi-utils.h | 2 +- 3 files changed, 26 insertions(+), 11 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 79717681da..98f6cc0701 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -721,7 +721,7 @@ _linktype_get_type (NMPlatform *platform, } /* Fallback for drivers that don't call SET_NETDEV_DEVTYPE() */ - if (wifi_utils_is_wifi (ifname)) + if (wifi_utils_is_wifi (ifindex, ifname)) return NM_LINK_TYPE_WIFI; if (arptype == ARPHRD_ETHER) { diff --git a/src/platform/wifi/wifi-utils.c b/src/platform/wifi/wifi-utils.c index de858e499a..3cfad6bd48 100644 --- a/src/platform/wifi/wifi-utils.c +++ b/src/platform/wifi/wifi-utils.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "wifi-utils-private.h" #include "wifi-utils-nl80211.h" @@ -34,6 +35,8 @@ #endif #include "nm-core-utils.h" +#include "platform/nm-platform-utils.h" + gpointer wifi_data_new (const char *iface, int ifindex, gsize len) { @@ -180,23 +183,35 @@ wifi_utils_deinit (WifiData *data) } gboolean -wifi_utils_is_wifi (const char *iface) +wifi_utils_is_wifi (int ifindex, const char *ifname) { - char phy80211_path[NM_STRLEN ("/sys/class/net/123456789012345/phy80211\0") + 100 /*safety*/]; + int fd_sysnet; + int fd_phy80211; struct stat s; - g_return_val_if_fail (iface != NULL, FALSE); + g_return_val_if_fail (ifname != NULL, FALSE); - nm_sprintf_buf (phy80211_path, - "/sys/class/net/%s/phy80211", - NM_ASSERT_VALID_PATH_COMPONENT (iface)); - nm_assert (strlen (phy80211_path) < sizeof (phy80211_path) - 1); + fd_sysnet = nmp_utils_open_sysctl (ifindex, ifname); + if (fd_sysnet < 0) + return FALSE; - if ((stat (phy80211_path, &s) == 0 && (s.st_mode & S_IFDIR))) + fd_phy80211 = openat (fd_sysnet, "phy80211", 0); + if (fd_phy80211 < 0) { + close (fd_sysnet); + return FALSE; + } + + if ((fstat (fd_phy80211, &s) == 0 && (s.st_mode & S_IFDIR))) { + close (fd_sysnet); + close (fd_phy80211); return TRUE; + } + + close (fd_sysnet); + close (fd_phy80211); #if HAVE_WEXT - if (wifi_wext_is_wifi (iface)) + if (wifi_wext_is_wifi (ifname)) return TRUE; #endif diff --git a/src/platform/wifi/wifi-utils.h b/src/platform/wifi/wifi-utils.h index 8e2b93f1f0..3dca2ac1d5 100644 --- a/src/platform/wifi/wifi-utils.h +++ b/src/platform/wifi/wifi-utils.h @@ -28,7 +28,7 @@ typedef struct WifiData WifiData; -gboolean wifi_utils_is_wifi (const char *iface); +gboolean wifi_utils_is_wifi (int ifindex, const char *ifname); WifiData *wifi_utils_init (const char *iface, int ifindex, gboolean check_scan); From 76876e896c242fd82d048743ffcf2c0481442dc5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Dec 2016 13:55:17 +0100 Subject: [PATCH 09/27] platform: refactor nmp_utils_sysctl_open_netdir() - use nm_auto_close cleanup attribute - optionally, return the found ifname - don't stat "phy80211". If such an entity can be opened, just assume it's a directory. --- src/platform/nm-platform-utils.c | 95 +++++++++++++++++++++----------- src/platform/nm-platform-utils.h | 4 +- src/platform/wifi/wifi-utils.c | 23 ++++---- 3 files changed, 77 insertions(+), 45 deletions(-) diff --git a/src/platform/nm-platform-utils.c b/src/platform/nm-platform-utils.c index 6c585c21fa..78da4365fe 100644 --- a/src/platform/nm-platform-utils.c +++ b/src/platform/nm-platform-utils.c @@ -38,9 +38,12 @@ #include "nm-core-utils.h" +extern char *if_indextoname (unsigned int __ifindex, char *__ifname); + /****************************************************************** * ethtool ******************************************************************/ + #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,27) #define ethtool_cmd_speed(pedata) ((pedata)->speed) @@ -48,8 +51,6 @@ G_STMT_START { (pedata)->speed = (guint16) (speed); } G_STMT_END #endif -extern char *if_indextoname (unsigned int __ifindex, char *__ifname); - static gboolean ethtool_get (const char *name, gpointer edata) { @@ -625,51 +626,83 @@ nmp_utils_ip_config_source_to_string (NMIPConfigSource source, char *buf, gsize return buf; } +/** + * nmp_utils_sysctl_open_netdir: + * @ifindex: the ifindex for which to open "/sys/class/net/%s" + * @ifname_guess: (allow-none): optional argument, if present used as initial + * guess as the current name for @ifindex. If guessed right, + * it saves an addtional if_indextoname() call. + * @out_ifname: (allow-none): if present, must be at least IFNAMSIZ + * characters. On success, this will contain the actual ifname + * found while opening the directory. + * + * Returns: a negative value on failure, on success returns the open fd + * to the "/sys/class/net/%s" directory for @ifindex. + */ int -nmp_utils_open_sysctl(int ifindex, const char *ifname) +nmp_utils_sysctl_open_netdir (int ifindex, + const char *ifname_guess, + char *out_ifname) { #define SYS_CLASS_NET "/sys/class/net/" + const char *ifname = ifname_guess; + char ifname_buf_last_try[IFNAMSIZ]; char ifname_buf[IFNAMSIZ]; guint try_count = 0; - char sysdir[NM_STRLEN (SYS_CLASS_NET) + IFNAMSIZ + 1] = SYS_CLASS_NET; + char sysdir[NM_STRLEN (SYS_CLASS_NET) + IFNAMSIZ] = SYS_CLASS_NET; char fd_buf[256]; - int fd; - int fd_ifindex; ssize_t nn; - while (++try_count < 4) { + g_return_val_if_fail (ifindex >= 0, -1); + + ifname_buf_last_try[0] = '\0'; + + for (try_count = 0; try_count < 10; try_count++, ifname = NULL) { + nm_auto_close int fd_dir = -1; + nm_auto_close int fd_ifindex = -1; + int fd; + if (!ifname) { ifname = if_indextoname (ifindex, ifname_buf); if (!ifname) return -1; } - nm_utils_ifname_cpy (&sysdir[NM_STRLEN (SYS_CLASS_NET)], ifname); - fd = open (sysdir, O_DIRECTORY); - if (fd < 0) - goto next; - fd_ifindex = openat (fd, "ifindex", 0); - if (fd_ifindex < 0) { - close (fd); - goto next; - } - /* read ifindex file, and compare it to @ifindex. If match, return fd. */ - nn = nm_utils_fd_read_loop (fd_ifindex, fd_buf, sizeof (fd_buf) - 1, FALSE); - if (nn < 0) { - close (fd); - close (fd_ifindex); - goto next; - } - fd_buf[sizeof (fd_buf) - 1] = '\0'; + nm_assert (nm_utils_iface_valid_name (ifname)); - if (ifindex != _nm_utils_ascii_str_to_int64 (fd_buf, 10, 1, G_MAXINT, -1)) { - close (fd); - close (fd_ifindex); - goto next; - } + if (g_strlcpy (&sysdir[NM_STRLEN (SYS_CLASS_NET)], ifname, IFNAMSIZ) >= IFNAMSIZ) + g_return_val_if_reached (-1); + + /* we only retry, if the name changed since previous attempt. + * Hence, it is extremely unlikely that this loop runes until the + * end of the @try_count. */ + if (nm_streq (ifname, ifname_buf_last_try)) + return -1; + strcpy (ifname_buf_last_try, ifname); + + fd_dir = open (sysdir, O_DIRECTORY | O_CLOEXEC); + if (fd_dir < 0) + continue; + + fd_ifindex = openat (fd_dir, "ifindex", O_CLOEXEC); + if (fd_ifindex < 0) + continue; + + nn = nm_utils_fd_read_loop (fd_ifindex, fd_buf, sizeof (fd_buf) - 2, FALSE); + if (nn <= 0) + continue; + fd_buf[nn] = '\0'; + + if (ifindex != _nm_utils_ascii_str_to_int64 (fd_buf, 10, 1, G_MAXINT, -1)) + continue; + + if (out_ifname) + strcpy (out_ifname, ifname); + + fd = fd_dir; + fd_dir = -1; return fd; -next: - ifname = NULL; } + return -1; } diff --git a/src/platform/nm-platform-utils.h b/src/platform/nm-platform-utils.h index 72a06a8a92..c99babf86f 100644 --- a/src/platform/nm-platform-utils.h +++ b/src/platform/nm-platform-utils.h @@ -61,6 +61,8 @@ NMIPConfigSource nmp_utils_ip_config_source_coerce_from_rtprot (NMIPConfigSource NMIPConfigSource nmp_utils_ip_config_source_round_trip_rtprot (NMIPConfigSource source) _nm_const; const char * nmp_utils_ip_config_source_to_string (NMIPConfigSource source, char *buf, gsize len); -int nmp_utils_open_sysctl(int ifindex, const char *ifname); +int nmp_utils_sysctl_open_netdir (int ifindex, + const char *ifname_guess, + char *out_ifname); #endif /* __NM_PLATFORM_UTILS_H__ */ diff --git a/src/platform/wifi/wifi-utils.c b/src/platform/wifi/wifi-utils.c index 3cfad6bd48..32210e0099 100644 --- a/src/platform/wifi/wifi-utils.c +++ b/src/platform/wifi/wifi-utils.c @@ -187,29 +187,26 @@ wifi_utils_is_wifi (int ifindex, const char *ifname) { int fd_sysnet; int fd_phy80211; - struct stat s; + char ifname_verified[IFNAMSIZ]; - g_return_val_if_fail (ifname != NULL, FALSE); + g_return_val_if_fail (ifindex > 0, FALSE); - fd_sysnet = nmp_utils_open_sysctl (ifindex, ifname); + fd_sysnet = nmp_utils_sysctl_open_netdir (ifindex, ifname, ifname_verified); if (fd_sysnet < 0) return FALSE; - fd_phy80211 = openat (fd_sysnet, "phy80211", 0); - if (fd_phy80211 < 0) { - close (fd_sysnet); - return FALSE; - } + /* there might have been a race and ifname might be wrong. Below for checking + * wext, use the possibly improved name that we just verified. */ + ifname = ifname_verified; - if ((fstat (fd_phy80211, &s) == 0 && (s.st_mode & S_IFDIR))) { - close (fd_sysnet); + fd_phy80211 = openat (fd_sysnet, "phy80211", O_CLOEXEC); + close (fd_sysnet); + + if (fd_phy80211 >= 0) { close (fd_phy80211); return TRUE; } - close (fd_sysnet); - close (fd_phy80211); - #if HAVE_WEXT if (wifi_wext_is_wifi (ifname)) return TRUE; From 1d9bdad1dfd33601288de5510ff466acbe2c4b08 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 9 Dec 2016 09:14:26 +0100 Subject: [PATCH 10/27] core: add nm_utils_file_get_contents() and nm_utils_fd_get_contents() A reimplementation of g_file_get_contents() to overcome two limitations: - nm_utils_file_get_contents() accepts a @dirfd argument to open the file relative using openat(). - nm_utils_fd_get_contents() allows to read the content from a file filedescriptor. - both support a max_length argument, to fail gracefully if we get tricked into loading a huge file. --- src/nm-core-utils.c | 240 ++++++++++++++++++++++++++++++++++++++++++++ src/nm-core-utils.h | 25 +++-- 2 files changed, 259 insertions(+), 6 deletions(-) diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index a0067aafc1..2577330647 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -2802,6 +2802,246 @@ nm_utils_fd_read_loop_exact (int fd, void *buf, size_t nbytes, bool do_poll) return 0; } +_nm_printf (3, 4) +static int +_get_contents_error (GError **error, int errsv, const char *format, ...) +{ + if (errsv < 0) + errsv = -errsv; + else if (!errsv) + errsv = errno; + + if (error) { + char *msg; + va_list args; + + va_start (args, format); + msg = g_strdup_vprintf (format, args); + va_end (args); + g_set_error (error, + G_FILE_ERROR, + g_file_error_from_errno (errsv), + "%s: %s", + msg, g_strerror (errsv)); + g_free (msg); + } + return -errsv; +} + +/** + * nm_utils_fd_get_contents: + * @fd: open file descriptor to read. The fd will not be closed, + * but don't rely on it's state afterwards. + * @max_length: allocate at most @max_length bytes. If the + * file is larger, reading will fail. Set to zero to use + * a very large default. + * + * WARNING: @max_length is here to avoid a crash for huge/unlimited files. + * For example, stat(/sys/class/net/enp0s25/ifindex) gives a filesize of + * 4K, although the actual real is small. @max_length is the memory + * allocated in the process of reading the file, thus it must be at least + * the size reported by fstat. + * If you set it to 1K, read will fail because fstat() claims the + * file is larger. + * + * @contents: the output buffer with the file read. It is always + * NUL terminated. The buffer is at most @max_length long, including + * the NUL byte. That is, it reads only files up to a length of + * @max_length - 1 bytes. + * @length: optional output argument of the read file size. + * + * A reimplementation of g_file_get_contents() with a few differences: + * - accepts an open fd, instead of a path name. This allows you to + * use openat(). + * - limits the maxium filesize to max_length. + * + * Returns: a negative error code on failure. + */ +int +nm_utils_fd_get_contents (int fd, + gsize max_length, + char **contents, + gsize *length, + GError **error) +{ + struct stat stat_buf; + gs_free char *str = NULL; + + g_return_val_if_fail (fd >= 0, -EINVAL); + g_return_val_if_fail (contents, -EINVAL); + g_return_val_if_fail (!error || !*error, -EINVAL); + + if (fstat (fd, &stat_buf) < 0) + return _get_contents_error (error, 0, "failure during fstat"); + + if (!max_length) { + /* default to a very large size, but not extreme */ + max_length = 2 * 1024 * 1024; + } + + if ( stat_buf.st_size > 0 + && S_ISREG (stat_buf.st_mode)) { + const gsize n_stat = stat_buf.st_size; + ssize_t n_read; + + if (n_stat > max_length - 1) + return _get_contents_error (error, EMSGSIZE, "file too large (%zu+1 bytes with maximum %zu bytes)", n_stat, max_length); + + str = g_try_malloc (n_stat + 1); + if (!str) + return _get_contents_error (error, ENOMEM, "failure to allocate buffer of %zu+1 bytes", n_stat); + + n_read = nm_utils_fd_read_loop (fd, str, n_stat, TRUE); + if (n_read < 0) + return _get_contents_error (error, n_read, "error reading %zu bytes from file descriptor", n_stat); + str[n_read] = '\0'; + + if (n_read < n_stat) { + char *tmp; + + tmp = g_try_realloc (str, n_read + 1); + if (!tmp) + return _get_contents_error (error, ENOMEM, "failure to reallocate buffer with %zu bytes", n_read + 1); + str = tmp; + } + NM_SET_OUT (length, n_read); + } else { + nm_auto_fclose FILE *f = NULL; + char buf[4096]; + gsize n_have, n_alloc; + + if (!(f = fdopen (fd, "r"))) + return _get_contents_error (error, 0, "failure during fdopen"); + + n_have = 0; + n_alloc = 0; + + while (!feof (f)) { + int errsv; + gsize n_read; + + n_read = fread (buf, 1, sizeof (buf), f); + errsv = errno; + if (ferror (f)) + return _get_contents_error (error, errsv, "error during fread"); + + if ( n_have > G_MAXSIZE - 1 - n_read + || n_have + n_read + 1 > max_length) { + return _get_contents_error (error, EMSGSIZE, "file stream too large (%zu+1 bytes with maximum %zu bytes)", + (n_have > G_MAXSIZE - 1 - n_read) ? G_MAXSIZE : n_have + n_read, + max_length); + } + + if (n_have + n_read + 1 >= n_alloc) { + char *tmp; + + if (str) { + if (n_alloc >= max_length / 2) + n_alloc = max_length; + else + n_alloc *= 2; + } else + n_alloc = NM_MIN (n_read + 1, sizeof (buf)); + + tmp = g_try_realloc (str, n_alloc); + if (!tmp) + return _get_contents_error (error, ENOMEM, "failure to allocate buffer of %zu bytes", n_alloc); + str = tmp; + } + + memcpy (str + n_have, buf, n_read); + n_have += n_read; + } + + if (n_alloc == 0) + str = g_new0 (gchar, 1); + else { + str[n_have] = '\0'; + if (n_have + 1 < n_alloc) { + char *tmp; + + tmp = g_try_realloc (str, n_have + 1); + if (!tmp) + return _get_contents_error (error, ENOMEM, "failure to truncate buffer to %zu bytes", n_have + 1); + str = tmp; + } + } + + NM_SET_OUT (length, n_have); + } + + *contents = g_steal_pointer (&str); + return 0; +} + +/** + * nm_utils_file_get_contents: + * @dirfd: optional file descriptor to use openat(). If negative, use plain open(). + * @filename: the filename to open. Possibly relative to @dirfd. + * @max_length: allocate at most @max_length bytes. + * WARNING: see nm_utils_fd_get_contents() hint about @max_length. + * @contents: the output buffer with the file read. It is always + * NUL terminated. The buffer is at most @max_length long, including + * the NUL byte. That is, it reads only files up to a length of + * @max_length - 1 bytes. + * @length: optional output argument of the read file size. + * + * A reimplementation of g_file_get_contents() with a few differences: + * - accepts an @dirfd to open @filename relative to that path via openat(). + * - limits the maxium filesize to max_length. + * - uses O_CLOEXEC on internal file descriptor + * + * Returns: a negative error code on failure. + */ +int +nm_utils_file_get_contents (int dirfd, + const char *filename, + gsize max_length, + char **contents, + gsize *length, + GError **error) +{ + nm_auto_close int fd = -1; + int errsv; + + g_return_val_if_fail (filename && filename[0], -EINVAL); + + if (dirfd >= 0) { + fd = openat (dirfd, filename, O_RDONLY | O_CLOEXEC); + if (fd < 0) { + errsv = errno; + + g_set_error (error, + G_FILE_ERROR, + g_file_error_from_errno (errsv), + "Failed to open file \"%s\" with openat: %s", + filename, + g_strerror (errsv)); + return -errsv; + } + } else { + fd = open (filename, O_RDONLY | O_CLOEXEC); + if (fd < 0) { + errsv = errno; + + g_set_error (error, + G_FILE_ERROR, + g_file_error_from_errno (errsv), + "Failed to open file \"%s\": %s", + filename, + g_strerror (errsv)); + return -errsv; + } + } + return nm_utils_fd_get_contents (fd, + max_length, + contents, + length, + error); +} + +/*****************************************************************************/ + /* taken from systemd's dev_urandom(). */ int nm_utils_read_urandom (void *p, size_t nbytes) diff --git a/src/nm-core-utils.h b/src/nm-core-utils.h index 9cf196f709..f9d30b5ce4 100644 --- a/src/nm-core-utils.h +++ b/src/nm-core-utils.h @@ -313,6 +313,25 @@ int nm_utils_fd_wait_for_event (int fd, int event, gint64 timeout_ns); ssize_t nm_utils_fd_read_loop (int fd, void *buf, size_t nbytes, bool do_poll); int nm_utils_fd_read_loop_exact (int fd, void *buf, size_t nbytes, bool do_poll); +int nm_utils_fd_get_contents (int fd, + gsize max_length, + char **contents, + gsize *length, + GError **error); + +int nm_utils_file_get_contents (int dirfd, + const char *filename, + gsize max_length, + char **contents, + gsize *length, + GError **error); + +gboolean nm_utils_file_set_contents (const gchar *filename, + const gchar *contents, + gssize length, + mode_t mode, + GError **error); + int nm_utils_read_urandom (void *p, size_t n); char *nm_utils_machine_id_read (void); @@ -445,12 +464,6 @@ const char *nm_utils_dnsmasq_status_to_string (int status, char *dest, gsize siz void nm_utils_get_reverse_dns_domains_ip4 (guint32 ip, guint8 plen, GPtrArray *domains); void nm_utils_get_reverse_dns_domains_ip6 (const struct in6_addr *ip, guint8 plen, GPtrArray *domains); -gboolean nm_utils_file_set_contents (const gchar *filename, - const gchar *contents, - gssize length, - mode_t mode, - GError **error); - struct stat; gboolean nm_utils_validate_plugin (const char *path, struct stat *stat, GError **error); From d8cefd57fbc0723a8b435bcba324e0c43b56e23f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Dec 2016 14:29:00 +0100 Subject: [PATCH 11/27] platform: add optional dirfd argument to sysctl functions Still unused. --- src/devices/adsl/nm-atm-manager.c | 2 +- src/devices/adsl/nm-device-adsl.c | 2 +- src/devices/nm-device-ethernet.c | 2 +- src/devices/nm-device-infiniband.c | 2 +- src/devices/nm-device.c | 16 +++++------ src/ndisc/nm-lndp-ndisc.c | 2 +- src/nm-iface-helper.c | 12 ++++---- src/platform/nm-fake-platform.c | 26 +++++++++++++++-- src/platform/nm-linux-platform.c | 41 ++++++++++++++++---------- src/platform/nm-platform.c | 46 +++++++++++++++++++----------- src/platform/nm-platform.h | 15 ++++++---- src/platform/tests/test-link.c | 6 ++-- 12 files changed, 110 insertions(+), 62 deletions(-) diff --git a/src/devices/adsl/nm-atm-manager.c b/src/devices/adsl/nm-atm-manager.c index 2ff2fc1840..b04e9fe717 100644 --- a/src/devices/adsl/nm-atm-manager.c +++ b/src/devices/adsl/nm-atm-manager.c @@ -137,7 +137,7 @@ adsl_add (NMAtmManager *self, GUdevDevice *udev_device) atm_index_path = g_strdup_printf ("/sys/class/atm/%s/atmindex", NM_ASSERT_VALID_PATH_COMPONENT (ifname)); atm_index = (int) nm_platform_sysctl_get_int_checked (NM_PLATFORM_GET, - atm_index_path, + NMP_SYSCTL_PATHID_ABSOLUTE (atm_index_path), 10, 0, G_MAXINT, -1); if (atm_index < 0) { diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c index 6cf808e687..8f8037c25f 100644 --- a/src/devices/adsl/nm-device-adsl.c +++ b/src/devices/adsl/nm-device-adsl.c @@ -547,7 +547,7 @@ carrier_update_cb (gpointer user_data) path = g_strdup_printf ("/sys/class/atm/%s/carrier", NM_ASSERT_VALID_PATH_COMPONENT (nm_device_get_iface (NM_DEVICE (self)))); - carrier = (int) nm_platform_sysctl_get_int_checked (NM_PLATFORM_GET, path, 10, 0, 1, -1); + carrier = (int) nm_platform_sysctl_get_int_checked (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (path), 10, 0, 1, -1); g_free (path); if (carrier != -1) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index 43084d49b9..5496bd9a51 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -208,7 +208,7 @@ _update_s390_subchannels (NMDeviceEthernet *self) gs_free char *path = NULL, *value = NULL; path = g_strdup_printf ("%s/%s", parent_path, item); - value = nm_platform_sysctl_get (NM_PLATFORM_GET, path); + value = nm_platform_sysctl_get (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (path)); if ( !strcmp (item, "portname") && !g_strcmp0 (value, "no portname required")) { diff --git a/src/devices/nm-device-infiniband.c b/src/devices/nm-device-infiniband.c index 47c2a01656..adcd073047 100644 --- a/src/devices/nm-device-infiniband.c +++ b/src/devices/nm-device-infiniband.c @@ -109,7 +109,7 @@ act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *reason) /* With some drivers the interface must be down to set transport mode */ nm_device_take_down (dev, TRUE); - ok = nm_platform_sysctl_set (NM_PLATFORM_GET, mode_path, transport_mode); + ok = nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (mode_path), transport_mode); g_free (mode_path); nm_device_bring_up (dev, TRUE, &no_firmware); diff --git a/src/devices/nm-device.c b/src/devices/nm-device.c index 2785a1a890..af201d8ce9 100644 --- a/src/devices/nm-device.c +++ b/src/devices/nm-device.c @@ -629,13 +629,13 @@ init_ip6_config_dns_priority (NMDevice *self, NMIP6Config *config) gboolean nm_device_ipv6_sysctl_set (NMDevice *self, const char *property, const char *value) { - return nm_platform_sysctl_set (NM_PLATFORM_GET, nm_utils_ip6_property_path (nm_device_get_ip_iface (self), property), value); + return nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip6_property_path (nm_device_get_ip_iface (self), property)), value); } static guint32 nm_device_ipv6_sysctl_get_int32 (NMDevice *self, const char *property, gint32 fallback) { - return nm_platform_sysctl_get_int32 (NM_PLATFORM_GET, nm_utils_ip6_property_path (nm_device_get_ip_iface (self), property), fallback); + return nm_platform_sysctl_get_int32 (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip6_property_path (nm_device_get_ip_iface (self), property)), fallback); } gboolean @@ -6771,7 +6771,7 @@ save_ip6_properties (NMDevice *self) g_hash_table_remove_all (priv->ip6_saved_properties); for (i = 0; i < G_N_ELEMENTS (ip6_properties_to_save); i++) { - value = nm_platform_sysctl_get (NM_PLATFORM_GET, nm_utils_ip6_property_path (ifname, ip6_properties_to_save[i])); + value = nm_platform_sysctl_get (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip6_property_path (ifname, ip6_properties_to_save[i]))); if (value) { g_hash_table_insert (priv->ip6_saved_properties, (char *) ip6_properties_to_save[i], @@ -6832,7 +6832,7 @@ set_nm_ipv6ll (NMDevice *self, gboolean enable) if (enable) { /* Bounce IPv6 to ensure the kernel stops IPv6LL address generation */ value = nm_platform_sysctl_get (NM_PLATFORM_GET, - nm_utils_ip6_property_path (nm_device_get_ip_iface (self), "disable_ipv6")); + NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip6_property_path (nm_device_get_ip_iface (self), "disable_ipv6"))); if (g_strcmp0 (value, "0") == 0) nm_device_ipv6_sysctl_set (self, "disable_ipv6", "1"); g_free (value); @@ -6898,7 +6898,7 @@ _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_PLATFORM_GET, "/proc/sys/net/ipv6/conf/default/use_tempaddr", NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN); + ip6_privacy = nm_platform_sysctl_get_int32 (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE ("/proc/sys/net/ipv6/conf/default/use_tempaddr"), NM_SETTING_IP6_CONFIG_PRIVACY_UNKNOWN); return _ip6_privacy_clamp (ip6_privacy); } @@ -7406,14 +7406,14 @@ share_init (void) char **iter; int errsv; - if (!nm_platform_sysctl_set (NM_PLATFORM_GET, "/proc/sys/net/ipv4/ip_forward", "1")) { + if (!nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE ("/proc/sys/net/ipv4/ip_forward"), "1")) { errsv = errno; nm_log_err (LOGD_SHARING, "share: error enabling IPv4 forwarding: (%d) %s", errsv, strerror (errsv)); return FALSE; } - if (!nm_platform_sysctl_set (NM_PLATFORM_GET, "/proc/sys/net/ipv4/ip_dynaddr", "1")) { + if (!nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE ("/proc/sys/net/ipv4/ip_dynaddr"), "1")) { errsv = errno; nm_log_err (LOGD_SHARING, "share: error enabling dynamic addresses: (%d) %s", errsv, strerror (errsv)); @@ -7754,7 +7754,7 @@ activate_stage5_ip6_config_commit (NMDevice *self) method = nm_utils_get_ip_config_method (connection, NM_TYPE_SETTING_IP6_CONFIG); if (strcmp (method, NM_SETTING_IP6_CONFIG_METHOD_SHARED) == 0) { - if (!nm_platform_sysctl_set (NM_PLATFORM_GET, "/proc/sys/net/ipv6/conf/all/forwarding", "1")) { + if (!nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE ("/proc/sys/net/ipv6/conf/all/forwarding"), "1")) { errsv = errno; _LOGE (LOGD_SHARING, "share: error enabling IPv6 forwarding: (%d) %s", errsv, strerror (errsv)); nm_device_ip_method_failed (self, AF_INET6, NM_DEVICE_STATE_REASON_SHARED_START_FAILED); diff --git a/src/ndisc/nm-lndp-ndisc.c b/src/ndisc/nm-lndp-ndisc.c index 6378c90039..9e5cdaa059 100644 --- a/src/ndisc/nm-lndp-ndisc.c +++ b/src/ndisc/nm-lndp-ndisc.c @@ -521,7 +521,7 @@ static inline int ipv6_sysctl_get (NMPlatform *platform, const char *ifname, const char *property, int min, int max, int defval) { return (int) nm_platform_sysctl_get_int_checked (platform, - nm_utils_ip6_property_path (ifname, property), + NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip6_property_path (ifname, property)), 10, min, max, diff --git a/src/nm-iface-helper.c b/src/nm-iface-helper.c index aa14b085fe..393e8acc57 100644 --- a/src/nm-iface-helper.c +++ b/src/nm-iface-helper.c @@ -246,7 +246,7 @@ ndisc_config_changed (NMNDisc *ndisc, const NMNDiscData *rdata, guint changed_in char val[16]; g_snprintf (val, sizeof (val), "%d", rdata->mtu); - nm_platform_sysctl_set (NM_PLATFORM_GET, nm_utils_ip6_property_path (global_opt.ifname, "mtu"), val); + nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip6_property_path (global_opt.ifname, "mtu")), val); } nm_ip6_config_merge (existing, ndisc_config, NM_IP_CONFIG_MERGE_DEFAULT); @@ -465,7 +465,7 @@ main (int argc, char *argv[]) } if (global_opt.dhcp4_address) { - nm_platform_sysctl_set (NM_PLATFORM_GET, nm_utils_ip4_property_path (global_opt.ifname, "promote_secondaries"), "1"); + nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip4_property_path (global_opt.ifname, "promote_secondaries")), "1"); dhcp4_client = nm_dhcp_manager_start_ip4 (nm_dhcp_manager_get (), global_opt.ifname, @@ -512,10 +512,10 @@ main (int argc, char *argv[]) if (iid) nm_ndisc_set_iid (ndisc, *iid); - nm_platform_sysctl_set (NM_PLATFORM_GET, nm_utils_ip6_property_path (global_opt.ifname, "accept_ra"), "1"); - nm_platform_sysctl_set (NM_PLATFORM_GET, nm_utils_ip6_property_path (global_opt.ifname, "accept_ra_defrtr"), "0"); - nm_platform_sysctl_set (NM_PLATFORM_GET, nm_utils_ip6_property_path (global_opt.ifname, "accept_ra_pinfo"), "0"); - nm_platform_sysctl_set (NM_PLATFORM_GET, nm_utils_ip6_property_path (global_opt.ifname, "accept_ra_rtr_pref"), "0"); + nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip6_property_path (global_opt.ifname, "accept_ra")), "1"); + nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip6_property_path (global_opt.ifname, "accept_ra_defrtr")), "0"); + nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip6_property_path (global_opt.ifname, "accept_ra_pinfo")), "0"); + nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (nm_utils_ip6_property_path (global_opt.ifname, "accept_ra_rtr_pref")), "0"); g_signal_connect (NM_PLATFORM_GET, NM_PLATFORM_SIGNAL_IP6_ADDRESS_CHANGED, diff --git a/src/platform/nm-fake-platform.c b/src/platform/nm-fake-platform.c index 0f34b8453f..b92f56ce21 100644 --- a/src/platform/nm-fake-platform.c +++ b/src/platform/nm-fake-platform.c @@ -118,21 +118,43 @@ _ip4_address_equal_peer_net (in_addr_t peer1, in_addr_t peer2, guint8 plen) /*****************************************************************************/ +#define ASSERT_SYSCTL_ARGS(pathid, dirfd, path) \ + G_STMT_START { \ + const char *const _pathid = (pathid); \ + const int _dirfd = (dirfd); \ + const char *const _path = (path); \ + \ + g_assert (_path && _path[0]); \ + g_assert (!strstr (_path, "/../")); \ + if (_dirfd < 0) { \ + g_assert (!_pathid); \ + g_assert (_path[0] == '/'); \ + g_assert ( g_str_has_prefix (_path, "/proc/sys/") \ + || g_str_has_prefix (_path, "/sys/")); \ + } else { \ + g_assert_not_reached (); \ + } \ + } G_STMT_END + static gboolean -sysctl_set (NMPlatform *platform, const char *path, const char *value) +sysctl_set (NMPlatform *platform, const char *pathid, int dirfd, const char *path, const char *value) { NMFakePlatformPrivate *priv = NM_FAKE_PLATFORM_GET_PRIVATE ((NMFakePlatform *) platform); + ASSERT_SYSCTL_ARGS (pathid, dirfd, path); + g_hash_table_insert (priv->options, g_strdup (path), g_strdup (value)); return TRUE; } static char * -sysctl_get (NMPlatform *platform, const char *path) +sysctl_get (NMPlatform *platform, const char *pathid, int dirfd, const char *path) { NMFakePlatformPrivate *priv = NM_FAKE_PLATFORM_GET_PRIVATE ((NMFakePlatform *) platform); + ASSERT_SYSCTL_ARGS (pathid, dirfd, path); + return g_strdup (g_hash_table_lookup (priv->options, path)); } diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 98f6cc0701..6e7f4ccf67 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -2490,6 +2490,25 @@ ASSERT_NETNS_CURRENT (NMPlatform *platform) /*****************************************************************************/ +#define ASSERT_SYSCTL_ARGS(pathid, dirfd, path) \ + G_STMT_START { \ + const char *const _pathid = (pathid); \ + const int _dirfd = (dirfd); \ + const char *const _path = (path); \ + \ + nm_assert (_path && _path[0]); \ + g_assert (!strstr (_path, "/../")); \ + if (_dirfd < 0) { \ + nm_assert (!_pathid); \ + nm_assert (_path[0] == '/'); \ + nm_assert ( g_str_has_prefix (_path, "/proc/sys/") \ + || g_str_has_prefix (_path, "/sys/")); \ + } else { \ + nm_assert (_pathid && _pathid[0] && _pathid[0] != '/'); \ + nm_assert (_path[0] != '/'); \ + } \ + } G_STMT_END + static void _log_dbg_sysctl_set_impl (NMPlatform *platform, const char *path, const char *value) { @@ -2521,7 +2540,7 @@ _log_dbg_sysctl_set_impl (NMPlatform *platform, const char *path, const char *va } G_STMT_END static gboolean -sysctl_set (NMPlatform *platform, const char *path, const char *value) +sysctl_set (NMPlatform *platform, const char *pathid, int dirfd, const char *path, const char *value) { nm_auto_pop_netns NMPNetns *netns = NULL; int fd, tries; @@ -2534,11 +2553,7 @@ sysctl_set (NMPlatform *platform, const char *path, const char *value) g_return_val_if_fail (path != NULL, FALSE); g_return_val_if_fail (value != NULL, FALSE); - /* Don't write outside known locations */ - g_assert (g_str_has_prefix (path, "/proc/sys/") - || g_str_has_prefix (path, "/sys/")); - /* Don't write to suspicious locations */ - g_assert (!strstr (path, "/../")); + ASSERT_SYSCTL_ARGS (pathid, dirfd, path); if (!nm_platform_netns_push (platform, &netns)) { errno = ENETDOWN; @@ -2676,17 +2691,13 @@ _log_dbg_sysctl_get_impl (NMPlatform *platform, const char *path, const char *co } G_STMT_END static char * -sysctl_get (NMPlatform *platform, const char *path) +sysctl_get (NMPlatform *platform, const char *pathid, int dirfd, const char *path) { nm_auto_pop_netns NMPNetns *netns = NULL; GError *error = NULL; char *contents; - /* Don't write outside known locations */ - g_assert (g_str_has_prefix (path, "/proc/sys/") - || g_str_has_prefix (path, "/sys/")); - /* Don't write to suspicious locations */ - g_assert (!strstr (path, "/../")); + ASSERT_SYSCTL_ARGS (pathid, dirfd, path); if (!nm_platform_netns_push (platform, &netns)) return NULL; @@ -4561,7 +4572,7 @@ link_get_physical_port_id (NMPlatform *platform, int ifindex) ifname = NM_ASSERT_VALID_PATH_COMPONENT (ifname); path = g_strdup_printf ("/sys/class/net/%s/phys_port_id", ifname); - id = sysctl_get (platform, path); + id = sysctl_get (platform, NMP_SYSCTL_PATHID_ABSOLUTE (path)); g_free (path); return id; @@ -4581,7 +4592,7 @@ link_get_dev_id (NMPlatform *platform, int ifindex) ifname = NM_ASSERT_VALID_PATH_COMPONENT (ifname); path = g_strdup_printf ("/sys/class/net/%s/dev_id", ifname); - id = sysctl_get (platform, path); + id = sysctl_get (platform, NMP_SYSCTL_PATHID_ABSOLUTE (path)); if (!id || !*id) return 0; @@ -5269,7 +5280,7 @@ _infiniband_partition_action (NMPlatform *platform, ? "create_child" : "delete_child")); nm_sprintf_buf (id, "0x%04x", p_key); - success = nm_platform_sysctl_set (platform, path, id); + success = nm_platform_sysctl_set (platform, NMP_SYSCTL_PATHID_ABSOLUTE (path), id); if (!success) { if ( action == INFINIBAND_ACTION_DELETE_CHILD && errno == ENODEV) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index af574a355c..57f69c9884 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -268,6 +268,9 @@ nm_platform_process_events (NMPlatform *self) /** * nm_platform_sysctl_set: * @self: platform instance + * @pathid: if @dirfd is present, this must be the full path that is looked up. + * It is required for logging. + * @dirfd: optional file descriptor for parent directory for openat() * @path: Absolute option path * @value: Value to write * @@ -278,14 +281,14 @@ nm_platform_process_events (NMPlatform *self) * Returns: %TRUE on success. */ gboolean -nm_platform_sysctl_set (NMPlatform *self, const char *path, const char *value) +nm_platform_sysctl_set (NMPlatform *self, const char *pathid, int dirfd, const char *path, const char *value) { _CHECK_SELF (self, klass, FALSE); g_return_val_if_fail (path, FALSE); g_return_val_if_fail (value, FALSE); - return klass->sysctl_set (self, path, value); + return klass->sysctl_set (self, pathid, dirfd, path, value); } gboolean @@ -305,7 +308,7 @@ nm_platform_sysctl_set_ip6_hop_limit_safe (NMPlatform *self, const char *iface, return FALSE; path = nm_utils_ip6_property_path (iface, "hop_limit"); - cur = nm_platform_sysctl_get_int_checked (self, path, 10, 1, G_MAXINT32, -1); + cur = nm_platform_sysctl_get_int_checked (self, NMP_SYSCTL_PATHID_ABSOLUTE (path), 10, 1, G_MAXINT32, -1); /* only allow increasing the hop-limit to avoid DOS by an attacker * setting a low hop-limit (CVE-2015-2924, rh#1209902) */ @@ -316,7 +319,7 @@ nm_platform_sysctl_set_ip6_hop_limit_safe (NMPlatform *self, const char *iface, char svalue[20]; sprintf (svalue, "%d", value); - nm_platform_sysctl_set (self, path, svalue); + nm_platform_sysctl_set (self, NMP_SYSCTL_PATHID_ABSOLUTE (path), svalue); } return TRUE; @@ -325,23 +328,29 @@ nm_platform_sysctl_set_ip6_hop_limit_safe (NMPlatform *self, const char *iface, /** * nm_platform_sysctl_get: * @self: platform instance + * @dirfd: if non-negative, used to lookup the path via openat(). + * @pathid: if @dirfd is present, this must be the full path that is looked up. + * It is required for logging. * @path: Absolute path to sysctl * * Returns: (transfer full): Contents of the virtual sysctl file. */ char * -nm_platform_sysctl_get (NMPlatform *self, const char *path) +nm_platform_sysctl_get (NMPlatform *self, const char *pathid, int dirfd, const char *path) { _CHECK_SELF (self, klass, NULL); g_return_val_if_fail (path, NULL); - return klass->sysctl_get (self, path); + return klass->sysctl_get (self, pathid, dirfd, path); } /** * nm_platform_sysctl_get_int32: * @self: platform instance + * @pathid: if @dirfd is present, this must be the full path that is looked up. + * It is required for logging. + * @dirfd: if non-negative, used to lookup the path via openat(). * @path: Absolute path to sysctl * @fallback: default value, if the content of path could not be read * as decimal integer. @@ -351,14 +360,17 @@ nm_platform_sysctl_get (NMPlatform *self, const char *path) * value, on success %errno will be set to zero. */ gint32 -nm_platform_sysctl_get_int32 (NMPlatform *self, const char *path, gint32 fallback) +nm_platform_sysctl_get_int32 (NMPlatform *self, const char *pathid, int dirfd, const char *path, gint32 fallback) { - return nm_platform_sysctl_get_int_checked (self, path, 10, G_MININT32, G_MAXINT32, fallback); + return nm_platform_sysctl_get_int_checked (self, pathid, dirfd, path, 10, G_MININT32, G_MAXINT32, fallback); } /** * nm_platform_sysctl_get_int_checked: * @self: platform instance + * @pathid: if @dirfd is present, this must be the full path that is looked up. + * It is required for logging. + * @dirfd: if non-negative, used to lookup the path via openat(). * @path: Absolute path to sysctl * @base: base of numeric conversion * @min: minimal value that is still valid @@ -373,7 +385,7 @@ nm_platform_sysctl_get_int32 (NMPlatform *self, const char *path, gint32 fallbac * (inclusive) or @fallback. */ gint64 -nm_platform_sysctl_get_int_checked (NMPlatform *self, 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; @@ -383,7 +395,7 @@ nm_platform_sysctl_get_int_checked (NMPlatform *self, const char *path, guint ba g_return_val_if_fail (path, fallback); if (path) - value = nm_platform_sysctl_get (self, path); + value = nm_platform_sysctl_get (self, pathid, dirfd, path); if (!value) { errno = EINVAL; @@ -1687,7 +1699,7 @@ link_set_option (NMPlatform *self, int master, const char *category, const char { gs_free char *path = link_option_path (self, master, category, option); - return path && nm_platform_sysctl_set (self, path, value); + return path && nm_platform_sysctl_set (self, NMP_SYSCTL_PATHID_ABSOLUTE (path), value); } static char * @@ -1695,7 +1707,7 @@ link_get_option (NMPlatform *self, int master, const char *category, const char { gs_free char *path = link_option_path (self, master, category, option); - return path ? nm_platform_sysctl_get (self, path) : NULL; + return path ? nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)) : NULL; } static const char * @@ -2004,7 +2016,7 @@ nm_platform_link_infiniband_get_properties (NMPlatform *self, /* Fall back to reading sysfs */ path = g_strdup_printf ("/sys/class/net/%s/mode", iface); - contents = nm_platform_sysctl_get (self, path); + contents = nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)); g_free (path); if (!contents) return FALSE; @@ -2018,7 +2030,7 @@ nm_platform_link_infiniband_get_properties (NMPlatform *self, g_free (contents); path = g_strdup_printf ("/sys/class/net/%s/pkey", iface); - contents = nm_platform_sysctl_get (self, path); + contents = nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)); g_free (path); if (!contents) return FALSE; @@ -2244,7 +2256,7 @@ nm_platform_link_tun_get_properties_ifname (NMPlatform *self, const char *ifname return FALSE; nm_sprintf_buf (path, "/sys/class/net/%s/owner", ifname); - val = nm_platform_sysctl_get (self, path); + val = nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)); if (val) { props->owner = _nm_utils_ascii_str_to_int64 (val, 10, -1, G_MAXINT64, -1); if (errno) @@ -2254,7 +2266,7 @@ nm_platform_link_tun_get_properties_ifname (NMPlatform *self, const char *ifname success = FALSE; nm_sprintf_buf (path, "/sys/class/net/%s/group", ifname); - val = nm_platform_sysctl_get (self, path); + val = nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)); if (val) { props->group = _nm_utils_ascii_str_to_int64 (val, 10, -1, G_MAXINT64, -1); if (errno) @@ -2264,7 +2276,7 @@ nm_platform_link_tun_get_properties_ifname (NMPlatform *self, const char *ifname success = FALSE; nm_sprintf_buf (path, "/sys/class/net/%s/tun_flags", ifname); - val = nm_platform_sysctl_get (self, path); + val = nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)); if (val) { gint64 flags; diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index a546e4eaec..5ed978e469 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -500,8 +500,8 @@ struct _NMPlatform { typedef struct { GObjectClass parent; - gboolean (*sysctl_set) (NMPlatform *, const char *path, const char *value); - char * (*sysctl_get) (NMPlatform *, const char *path); + gboolean (*sysctl_set) (NMPlatform *, const char *pathid, int dirfd, const char *path, const char *value); + char * (*sysctl_get) (NMPlatform *, const char *pathid, int dirfd, const char *path); const NMPlatformLink *(*link_get) (NMPlatform *platform, int ifindex); const NMPlatformLink *(*link_get_by_ifname) (NMPlatform *platform, const char *ifname); @@ -717,10 +717,13 @@ const char *nm_link_type_to_string (NMLinkType link_type); const char *_nm_platform_error_to_string (NMPlatformError error); #define nm_platform_error_to_string(error) NM_UTILS_LOOKUP_STR (_nm_platform_error_to_string, error) -gboolean nm_platform_sysctl_set (NMPlatform *self, const char *path, const char *value); -char *nm_platform_sysctl_get (NMPlatform *self, const char *path); -gint32 nm_platform_sysctl_get_int32 (NMPlatform *self, const char *path, gint32 fallback); -gint64 nm_platform_sysctl_get_int_checked (NMPlatform *self, const char *path, guint base, gint64 min, gint64 max, gint64 fallback); +#define NMP_SYSCTL_PATHID_ABSOLUTE(path) \ + ((const char *) NULL), -1, (path) + +gboolean nm_platform_sysctl_set (NMPlatform *self, const char *pathid, int dirfd, const char *path, const char *value); +char *nm_platform_sysctl_get (NMPlatform *self, const char *pathid, int dirfd, const char *path); +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); gboolean nm_platform_sysctl_set_ip6_hop_limit_safe (NMPlatform *self, const char *iface, int value); diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index e3b55e73c8..ebeea1b30a 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -1941,7 +1941,7 @@ _test_netns_check_skip (void) G_STMT_START { \ gs_free char *_val = NULL; \ \ - _val = nm_platform_sysctl_get (plat, path); \ + _val = nm_platform_sysctl_get (plat, NMP_SYSCTL_PATHID_ABSOLUTE (path)); \ g_assert_cmpstr (_val, ==, value); \ } G_STMT_END @@ -2013,7 +2013,7 @@ test_netns_general (gpointer fixture, gconstpointer test_data) else path = "/proc/sys/net/ipv6/conf/dummy2b/disable_ipv6"; } - g_assert (nm_platform_sysctl_set (pl, path, nm_sprintf_buf (sbuf, "%d", j))); + g_assert (nm_platform_sysctl_set (pl, NMP_SYSCTL_PATHID_ABSOLUTE (path), nm_sprintf_buf (sbuf, "%d", j))); _sysctl_assert_eq (pl, path, nm_sprintf_buf (sbuf, "%d", j)); } @@ -2187,7 +2187,7 @@ test_netns_push (gpointer fixture, gconstpointer test_data) _ADD_DUMMY (pl[i].platform, pl[i].device_name); - g_assert (nm_platform_sysctl_set (pl[i].platform, pl[i].sysctl_path, pl[i].sysctl_value)); + g_assert (nm_platform_sysctl_set (pl[i].platform, NMP_SYSCTL_PATHID_ABSOLUTE (pl[i].sysctl_path), pl[i].sysctl_value)); tmp = _get_current_namespace_id (CLONE_NEWNET); g_ptr_array_add (device_names, tmp); From c85418746c3e0d80d666586b7347a0626dee4bf2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Dec 2016 15:12:52 +0100 Subject: [PATCH 12/27] platform: implement sysctl access via relative path to sysctl_open_netdir() --- src/platform/nm-linux-platform.c | 97 +++++++++++++++++++------------- src/platform/nm-platform.c | 28 +++++++++ src/platform/nm-platform.h | 11 ++++ 3 files changed, 98 insertions(+), 38 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 6e7f4ccf67..82c644870e 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -2510,32 +2510,32 @@ ASSERT_NETNS_CURRENT (NMPlatform *platform) } G_STMT_END static void -_log_dbg_sysctl_set_impl (NMPlatform *platform, const char *path, const char *value) +_log_dbg_sysctl_set_impl (NMPlatform *platform, const char *pathid, int dirfd, const char *path, const char *value) { GError *error = NULL; char *contents, *contents_escaped; char *value_escaped = g_strescape (value, NULL); - if (!g_file_get_contents (path, &contents, NULL, &error)) { - _LOGD ("sysctl: setting '%s' to '%s' (current value cannot be read: %s)", path, value_escaped, error->message); + if (nm_utils_file_get_contents (dirfd, path, 1*1024*1024, &contents, NULL, &error) < 0) { + _LOGD ("sysctl: setting '%s' to '%s' (current value cannot be read: %s)", pathid, value_escaped, error->message); g_clear_error (&error); } else { g_strstrip (contents); contents_escaped = g_strescape (contents, NULL); if (strcmp (contents, value) == 0) - _LOGD ("sysctl: setting '%s' to '%s' (current value is identical)", path, value_escaped); + _LOGD ("sysctl: setting '%s' to '%s' (current value is identical)", pathid, value_escaped); else - _LOGD ("sysctl: setting '%s' to '%s' (current value is '%s')", path, value_escaped, contents_escaped); + _LOGD ("sysctl: setting '%s' to '%s' (current value is '%s')", pathid, value_escaped, contents_escaped); g_free (contents); g_free (contents_escaped); } g_free (value_escaped); } -#define _log_dbg_sysctl_set(platform, path, value) \ +#define _log_dbg_sysctl_set(platform, pathid, dirfd, path, value) \ G_STMT_START { \ if (_LOGD_ENABLED ()) { \ - _log_dbg_sysctl_set_impl (platform, path, value); \ + _log_dbg_sysctl_set_impl (platform, pathid, dirfd, path, value); \ } \ } G_STMT_END @@ -2555,26 +2555,44 @@ sysctl_set (NMPlatform *platform, const char *pathid, int dirfd, const char *pat ASSERT_SYSCTL_ARGS (pathid, dirfd, path); - if (!nm_platform_netns_push (platform, &netns)) { - errno = ENETDOWN; - return FALSE; - } - - fd = open (path, O_WRONLY | O_TRUNC); - if (fd == -1) { - errsv = errno; - if (errsv == ENOENT) { - _LOGD ("sysctl: failed to open '%s': (%d) %s", - path, errsv, strerror (errsv)); - } else { - _LOGE ("sysctl: failed to open '%s': (%d) %s", - path, errsv, strerror (errsv)); + if (dirfd < 0) { + if (!nm_platform_netns_push (platform, &netns)) { + errno = ENETDOWN; + return FALSE; + } + + pathid = path; + + fd = open (path, O_WRONLY | O_TRUNC | O_CLOEXEC); + if (fd == -1) { + errsv = errno; + if (errsv == ENOENT) { + _LOGD ("sysctl: failed to open '%s': (%d) %s", + pathid, errsv, strerror (errsv)); + } else { + _LOGE ("sysctl: failed to open '%s': (%d) %s", + pathid, errsv, strerror (errsv)); + } + errno = errsv; + return FALSE; + } + } else { + fd = openat (dirfd, path, O_WRONLY | O_TRUNC | O_CLOEXEC); + if (fd == -1) { + errsv = errno; + if (errsv == ENOENT) { + _LOGD ("sysctl: failed to openat '%s': (%d) %s", + pathid, errsv, strerror (errsv)); + } else { + _LOGE ("sysctl: failed to openat '%s': (%d) %s", + pathid, errsv, strerror (errsv)); + } + errno = errsv; + return FALSE; } - errno = errsv; - return FALSE; } - _log_dbg_sysctl_set (platform, path, value); + _log_dbg_sysctl_set (platform, pathid, dirfd, path, value); /* Most sysfs and sysctl options don't care about a trailing LF, while some * (like infiniband) do. So always add the LF. Also, neither sysfs nor @@ -2647,7 +2665,7 @@ _nm_logging_clear_platform_logging_cache_impl (void) } static void -_log_dbg_sysctl_get_impl (NMPlatform *platform, const char *path, const char *contents) +_log_dbg_sysctl_get_impl (NMPlatform *platform, const char *pathid, const char *contents) { NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); const char *prev_value = NULL; @@ -2657,24 +2675,24 @@ _log_dbg_sysctl_get_impl (NMPlatform *platform, const char *path, const char *co sysctl_clear_cache_list = g_slist_prepend (sysctl_clear_cache_list, platform); priv->sysctl_get_prev_values = g_hash_table_new_full (g_str_hash, g_str_equal, g_free, g_free); } else - prev_value = g_hash_table_lookup (priv->sysctl_get_prev_values, path); + prev_value = g_hash_table_lookup (priv->sysctl_get_prev_values, pathid); if (prev_value) { if (strcmp (prev_value, contents) != 0) { char *contents_escaped = g_strescape (contents, NULL); char *prev_value_escaped = g_strescape (prev_value, NULL); - _LOGD ("sysctl: reading '%s': '%s' (changed from '%s' on last read)", path, contents_escaped, prev_value_escaped); + _LOGD ("sysctl: reading '%s': '%s' (changed from '%s' on last read)", pathid, contents_escaped, prev_value_escaped); g_free (contents_escaped); g_free (prev_value_escaped); - g_hash_table_insert (priv->sysctl_get_prev_values, g_strdup (path), g_strdup (contents)); + g_hash_table_insert (priv->sysctl_get_prev_values, g_strdup (pathid), g_strdup (contents)); } } else { char *contents_escaped = g_strescape (contents, NULL); - _LOGD ("sysctl: reading '%s': '%s'", path, contents_escaped); + _LOGD ("sysctl: reading '%s': '%s'", pathid, contents_escaped); g_free (contents_escaped); - g_hash_table_insert (priv->sysctl_get_prev_values, g_strdup (path), g_strdup (contents)); + g_hash_table_insert (priv->sysctl_get_prev_values, g_strdup (pathid), g_strdup (contents)); } if ( !priv->sysctl_get_warned @@ -2684,10 +2702,10 @@ _log_dbg_sysctl_get_impl (NMPlatform *platform, const char *path, const char *co } } -#define _log_dbg_sysctl_get(platform, path, contents) \ +#define _log_dbg_sysctl_get(platform, pathid, contents) \ G_STMT_START { \ if (_LOGD_ENABLED ()) \ - _log_dbg_sysctl_get_impl (platform, path, contents); \ + _log_dbg_sysctl_get_impl (platform, pathid, contents); \ } G_STMT_END static char * @@ -2699,24 +2717,27 @@ sysctl_get (NMPlatform *platform, const char *pathid, int dirfd, const char *pat ASSERT_SYSCTL_ARGS (pathid, dirfd, path); - if (!nm_platform_netns_push (platform, &netns)) - return NULL; + if (dirfd < 0) { + if (!nm_platform_netns_push (platform, &netns)) + return NULL; + pathid = path; + } - if (!g_file_get_contents (path, &contents, NULL, &error)) { + if (nm_utils_file_get_contents (dirfd, path, 1*1024*1024, &contents, NULL, &error) < 0) { /* We assume FAILED means EOPNOTSUP */ if ( g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NOENT) || g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_NODEV) || g_error_matches (error, G_FILE_ERROR, G_FILE_ERROR_FAILED)) - _LOGD ("error reading %s: %s", path, error->message); + _LOGD ("error reading %s: %s", pathid, error->message); else - _LOGE ("error reading %s: %s", path, error->message); + _LOGE ("error reading %s: %s", pathid, error->message); g_clear_error (&error); return NULL; } g_strstrip (contents); - _log_dbg_sysctl_get (platform, path, contents); + _log_dbg_sysctl_get (platform, pathid, contents); return contents; } diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 57f69c9884..99a7f14657 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -265,6 +265,34 @@ nm_platform_process_events (NMPlatform *self) /*****************************************************************************/ +/** + * nm_platform_sysctl_open_netdir: + * @self: platform instance + * @ifindex: the ifindex for which to open /sys/class/net/%s + * @out_ifname: optional output argument of the found ifname. + * + * Wraps nmp_utils_sysctl_open_netdir() by first changing into the right + * network-namespace. + * + * Returns: on success, the open file descriptor to the /sys/class/net/%s + * directory. + */ +int +nm_platform_sysctl_open_netdir (NMPlatform *self, int ifindex, char *out_ifname) +{ + const char*ifname_guess; + _CHECK_SELF_NETNS (self, klass, netns, -1); + + g_return_val_if_fail (ifindex > 0, -1); + + /* we don't have an @ifname_guess argument to make the API nicer. + * But still do a cache-lookup first. Chances are good that we have + * the right ifname cached and save if_indextoname() */ + ifname_guess = nm_platform_link_get_name (self, ifindex); + + return nmp_utils_sysctl_open_netdir (ifindex, ifname_guess, out_ifname); +} + /** * nm_platform_sysctl_set: * @self: platform instance diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 5ed978e469..b2afce3345 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -720,6 +720,17 @@ const char *_nm_platform_error_to_string (NMPlatformError error); #define NMP_SYSCTL_PATHID_ABSOLUTE(path) \ ((const char *) NULL), -1, (path) +#define NMP_SYSCTL_PATHID_NETDIR_unsafe(dirfd, ifname, path) \ + nm_sprintf_bufa (NM_STRLEN ("net:/sys/class/net//\0") + IFNAMSIZ + strlen (path), \ + "net:/sys/class/net/%s/%s", (ifname), (path)), \ + (dirfd), (path) + +#define NMP_SYSCTL_PATHID_NETDIR(dirfd, ifname, path) \ + nm_sprintf_bufa (NM_STRLEN ("net:/sys/class/net//"path"/\0") + IFNAMSIZ, \ + "net:/sys/class/net/%s/%s", (ifname), path), \ + (dirfd), (""path"") + +int nm_platform_sysctl_open_netdir (NMPlatform *self, int ifindex, char *out_ifname); gboolean nm_platform_sysctl_set (NMPlatform *self, const char *pathid, int dirfd, const char *path, const char *value); char *nm_platform_sysctl_get (NMPlatform *self, const char *pathid, int dirfd, const char *path); gint32 nm_platform_sysctl_get_int32 (NMPlatform *self, const char *pathid, int dirfd, const char *path, gint32 fallback); From e933a2ff873df0c4a46255d015a6bd2bdad8c9c3 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Dec 2016 15:05:15 +0100 Subject: [PATCH 13/27] core: use nmp_utils_sysctl_open_netdir() to read infiniband sysctl --- src/platform/nm-platform.c | 25 +++++++++---------------- 1 file changed, 9 insertions(+), 16 deletions(-) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 99a7f14657..726fe5190b 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -2013,10 +2013,11 @@ nm_platform_link_infiniband_get_properties (NMPlatform *self, int *out_p_key, const char **out_mode) { + nm_auto_close int dirfd = -1; + char ifname_verified[IFNAMSIZ]; const NMPlatformLnkInfiniband *plnk; const NMPlatformLink *plink; - const char *iface; - char *path, *contents; + char *contents; const char *mode; int p_key = 0; @@ -2040,15 +2041,13 @@ nm_platform_link_infiniband_get_properties (NMPlatform *self, /* Could not get the link information via netlink. To support older kernels, * fallback to reading sysfs. */ - iface = NM_ASSERT_VALID_PATH_COMPONENT (plink->name); - - /* Fall back to reading sysfs */ - path = g_strdup_printf ("/sys/class/net/%s/mode", iface); - contents = nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)); - g_free (path); - if (!contents) + dirfd = nm_platform_sysctl_open_netdir (self, ifindex, ifname_verified); + if (dirfd < 0) return FALSE; + contents = nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname_verified, "mode")); + if (!contents) + return FALSE; if (strstr (contents, "datagram")) mode = "datagram"; else if (strstr (contents, "connected")) @@ -2057,13 +2056,7 @@ nm_platform_link_infiniband_get_properties (NMPlatform *self, mode = NULL; g_free (contents); - path = g_strdup_printf ("/sys/class/net/%s/pkey", iface); - contents = nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)); - g_free (path); - if (!contents) - return FALSE; - p_key = (int) _nm_utils_ascii_str_to_int64 (contents, 16, 0, 0xFFFF, -1); - g_free (contents); + p_key = nm_platform_sysctl_get_int_checked (self, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname_verified, "pkey"), 16, 0, 0xFFFF, -1); if (p_key < 0) return FALSE; From 89a2169b98ef6508aacb63df516c15165f830951 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Thu, 8 Dec 2016 15:05:15 +0100 Subject: [PATCH 14/27] core: use nmp_utils_sysctl_open_netdir() to set infiniband sysctl --- src/devices/nm-device-infiniband.c | 13 +++++-------- 1 file changed, 5 insertions(+), 8 deletions(-) diff --git a/src/devices/nm-device-infiniband.c b/src/devices/nm-device-infiniband.c index adcd073047..b0871b0a95 100644 --- a/src/devices/nm-device-infiniband.c +++ b/src/devices/nm-device-infiniband.c @@ -77,10 +77,11 @@ get_generic_capabilities (NMDevice *device) static NMActStageReturn act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *reason) { + nm_auto_close int dirfd = -1; NMActStageReturn ret; NMSettingInfiniband *s_infiniband; + char ifname_verified[IFNAMSIZ]; const char *transport_mode; - char *mode_path; gboolean ok, no_firmware = FALSE; g_return_val_if_fail (reason != NULL, NM_ACT_STAGE_RETURN_FAILURE); @@ -94,11 +95,8 @@ act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *reason) transport_mode = nm_setting_infiniband_get_transport_mode (s_infiniband); - mode_path = g_strdup_printf ("/sys/class/net/%s/mode", - NM_ASSERT_VALID_PATH_COMPONENT (nm_device_get_iface (dev))); - if (!g_file_test (mode_path, G_FILE_TEST_EXISTS)) { - g_free (mode_path); - + dirfd = nm_platform_sysctl_open_netdir (NM_PLATFORM_GET, nm_device_get_ifindex (dev), ifname_verified); + if (dirfd < 0) { if (!strcmp (transport_mode, "datagram")) return NM_ACT_STAGE_RETURN_SUCCESS; else { @@ -109,8 +107,7 @@ act_stage1_prepare (NMDevice *dev, NMDeviceStateReason *reason) /* With some drivers the interface must be down to set transport mode */ nm_device_take_down (dev, TRUE); - ok = nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_ABSOLUTE (mode_path), transport_mode); - g_free (mode_path); + ok = nm_platform_sysctl_set (NM_PLATFORM_GET, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname_verified, "mode"), transport_mode); nm_device_bring_up (dev, TRUE, &no_firmware); if (!ok) { From d3af925b91ba1cb7797e6501f95e35954d23c43f Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 9 Dec 2016 10:43:06 +0100 Subject: [PATCH 15/27] core: use nmp_utils_sysctl_open_netdir() to read tun/tap sysctl --- src/devices/nm-device-tun.c | 4 +- src/platform/nm-linux-platform.c | 2 +- src/platform/nm-platform.c | 77 +++++++++++++------------------- src/platform/nm-platform.h | 4 +- 4 files changed, 35 insertions(+), 52 deletions(-) diff --git a/src/devices/nm-device-tun.c b/src/devices/nm-device-tun.c index 15bcacb0bc..ea70f1a539 100644 --- a/src/devices/nm-device-tun.c +++ b/src/devices/nm-device-tun.c @@ -80,7 +80,7 @@ reload_tun_properties (NMDeviceTun *self) ifindex = nm_device_get_ifindex (NM_DEVICE (self)); if (ifindex > 0) { - if (!nm_platform_link_tun_get_properties (NM_PLATFORM_GET, ifindex, &props)) { + if (!nm_platform_link_tun_get_properties (NM_PLATFORM_GET, ifindex, NULL, &props)) { _LOGD (LOGD_DEVICE, "tun-properties: cannot loading tun properties from platform for ifindex %d", ifindex); ifindex = 0; } else if (g_strcmp0 (priv->mode, props.mode) != 0) { @@ -181,7 +181,7 @@ update_connection (NMDevice *device, NMConnection *connection) nm_connection_add_setting (connection, (NMSetting *) s_tun); } - if (!nm_platform_link_tun_get_properties (NM_PLATFORM_GET, nm_device_get_ifindex (device), &props)) { + if (!nm_platform_link_tun_get_properties (NM_PLATFORM_GET, nm_device_get_ifindex (device), NULL, &props)) { _LOGW (LOGD_PLATFORM, "failed to get TUN interface info while updating connection."); return; } diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 82c644870e..497f34e875 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -656,7 +656,7 @@ _linktype_get_type (NMPlatform *platform, NMPlatformTunProperties props; if ( platform - && nm_platform_link_tun_get_properties_ifname (platform, ifname, &props)) { + && nm_platform_link_tun_get_properties (platform, ifindex, ifname ?: "", &props)) { if (!g_strcmp0 (props.mode, "tap")) return NM_LINK_TYPE_TAP; if (!g_strcmp0 (props.mode, "tun")) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 726fe5190b..7cc675d3a9 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -2259,74 +2259,59 @@ nm_platform_link_veth_get_properties (NMPlatform *self, int ifindex, int *out_pe } gboolean -nm_platform_link_tun_get_properties_ifname (NMPlatform *self, const char *ifname, NMPlatformTunProperties *props) +nm_platform_link_tun_get_properties (NMPlatform *self, int ifindex, const char *ifname_guess, NMPlatformTunProperties *props) { - char path[256]; - char *val; + nm_auto_close int dirfd = -1; + const char *ifname; + char ifname_verified[IFNAMSIZ]; + gint64 flags; gboolean success = TRUE; - _CHECK_SELF (self, klass, FALSE); + g_return_val_if_fail (ifindex > 0, FALSE); g_return_val_if_fail (props, FALSE); + /* ifname_guess is an optional argument to find a guess for the ifname corresponding to + * ifindex. */ + if (!ifname_guess) { + /* if NULL, obtain the guess from the platform cache. */ + ifname = nm_platform_link_get_name (self, ifindex); + } else if (!ifname_guess[0]) { + /* if empty, don't use a guess. That means to use if_indextoname(). */ + ifname = NULL; + } else + ifname = ifname_guess; + memset (props, 0, sizeof (*props)); props->owner = -1; props->group = -1; - if (!ifname || !nm_utils_iface_valid_name (ifname)) + dirfd = nm_platform_sysctl_open_netdir (self, ifindex, ifname_verified); + if (dirfd < 0) return FALSE; - nm_sprintf_buf (path, "/sys/class/net/%s/owner", ifname); - val = nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)); - if (val) { - props->owner = _nm_utils_ascii_str_to_int64 (val, 10, -1, G_MAXINT64, -1); - if (errno) - success = FALSE; - g_free (val); - } else + ifname = ifname_verified; + + props->owner = nm_platform_sysctl_get_int_checked (self, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname, "owner"), 10, -1, G_MAXINT64, -1); + if (errno) success = FALSE; - nm_sprintf_buf (path, "/sys/class/net/%s/group", ifname); - val = nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)); - if (val) { - props->group = _nm_utils_ascii_str_to_int64 (val, 10, -1, G_MAXINT64, -1); - if (errno) - success = FALSE; - g_free (val); - } else + props->group = nm_platform_sysctl_get_int_checked (self, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname, "group"), 10, -1, G_MAXINT64, -1); + if (errno) success = FALSE; - nm_sprintf_buf (path, "/sys/class/net/%s/tun_flags", ifname); - val = nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)); - if (val) { - gint64 flags; - - flags = _nm_utils_ascii_str_to_int64 (val, 16, 0, G_MAXINT64, 0); - if (!errno) { - props->mode = ((flags & (IFF_TUN | IFF_TAP)) == IFF_TUN) ? "tun" : "tap"; - props->no_pi = !!(flags & IFF_NO_PI); - props->vnet_hdr = !!(flags & IFF_VNET_HDR); - props->multi_queue = !!(flags & NM_IFF_MULTI_QUEUE); - } else - success = FALSE; - g_free (val); + flags = nm_platform_sysctl_get_int_checked (self, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname, "tun_flags"), 16, 0, G_MAXINT64, -1); + if (flags >= 0) { + props->mode = ((flags & (IFF_TUN | IFF_TAP)) == IFF_TUN) ? "tun" : "tap"; + props->no_pi = !!(flags & IFF_NO_PI); + props->vnet_hdr = !!(flags & IFF_VNET_HDR); + props->multi_queue = !!(flags & NM_IFF_MULTI_QUEUE); } else success = FALSE; return success; } -gboolean -nm_platform_link_tun_get_properties (NMPlatform *self, int ifindex, NMPlatformTunProperties *props) -{ - _CHECK_SELF (self, klass, FALSE); - - g_return_val_if_fail (ifindex > 0, FALSE); - g_return_val_if_fail (props != NULL, FALSE); - - return nm_platform_link_tun_get_properties_ifname (self, nm_platform_link_get_name (self, ifindex), props); -} - gboolean nm_platform_wifi_get_capabilities (NMPlatform *self, int ifindex, NMDeviceWifiCapabilities *caps) { diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index b2afce3345..2e9dbcc844 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -865,9 +865,7 @@ NMPlatformError nm_platform_link_infiniband_delete (NMPlatform *self, gboolean nm_platform_link_infiniband_get_properties (NMPlatform *self, int ifindex, int *parent, int *p_key, const char **mode); gboolean nm_platform_link_veth_get_properties (NMPlatform *self, int ifindex, int *out_peer_ifindex); -gboolean nm_platform_link_tun_get_properties (NMPlatform *self, int ifindex, NMPlatformTunProperties *properties); - -gboolean nm_platform_link_tun_get_properties_ifname (NMPlatform *platform, const char *ifname, NMPlatformTunProperties *props); +gboolean nm_platform_link_tun_get_properties (NMPlatform *self, int ifindex, const char *ifname_guess, NMPlatformTunProperties *properties); gboolean nm_platform_wifi_get_capabilities (NMPlatform *self, int ifindex, NMDeviceWifiCapabilities *caps); gboolean nm_platform_wifi_get_bssid (NMPlatform *self, int ifindex, guint8 *bssid); From 7fc3eace31962da35747e096b7261e0c1b39b5fc Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 9 Dec 2016 10:43:06 +0100 Subject: [PATCH 16/27] core: use nmp_utils_sysctl_open_netdir() to detect link-type --- src/platform/nm-linux-platform.c | 53 +++++++++++++++----------------- src/platform/wifi/wifi-utils.c | 26 ++-------------- src/platform/wifi/wifi-utils.h | 2 +- 3 files changed, 28 insertions(+), 53 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 497f34e875..103cdfa2e3 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -573,18 +573,14 @@ _lookup_cached_link (const NMPCache *cache, int ifindex, gboolean *completed_fro #define DEVTYPE_PREFIX "DEVTYPE=" static char * -_linktype_read_devtype (const char *ifname) +_linktype_read_devtype (int dirfd) { - char uevent[NM_STRLEN ("/sys/class/net/123456789012345/uevent\0") + 100 /*safety*/]; char *contents = NULL; char *cont, *end; - nm_sprintf_buf (uevent, - "/sys/class/net/%s/uevent", - NM_ASSERT_VALID_PATH_COMPONENT (ifname)); - nm_assert (strlen (uevent) < sizeof (uevent) - 1); + nm_assert (dirfd >= 0); - if (!g_file_get_contents (uevent, &contents, NULL, NULL)) + if (nm_utils_file_get_contents (dirfd, "uevent", 1*1024*1024, &contents, NULL, NULL) < 0) return NULL; for (cont = contents; cont; cont = end) { end = strpbrk (cont, "\r\n"); @@ -680,9 +676,10 @@ _linktype_get_type (NMPlatform *platform, return NM_LINK_TYPE_IP6TNL; if (ifname) { - char anycast_mask[NM_STRLEN ("/sys/class/net/123456789012345/anycast_mask\0") + 100 /*safety*/]; + nm_auto_close int dirfd = -1; gs_free char *driver = NULL; gs_free char *devtype = NULL; + char ifname_verified[IFNAMSIZ]; /* Fallback OVS detection for kernel <= 3.16 */ if (nmp_utils_ethtool_get_driver_info (ifname, &driver, NULL, NULL)) { @@ -698,31 +695,29 @@ _linktype_get_type (NMPlatform *platform, } } - nm_sprintf_buf (anycast_mask, - "/sys/class/net/%s/anycast_mask", - NM_ASSERT_VALID_PATH_COMPONENT (ifname)); - nm_assert (strlen (anycast_mask) < sizeof (anycast_mask) - 1); + dirfd = nmp_utils_sysctl_open_netdir (ifindex, ifname, ifname_verified); + if (dirfd >= 0) { + if (faccessat (dirfd, "anycast_mask", F_OK, 0) == 0) + return NM_LINK_TYPE_OLPC_MESH; - if (g_file_test (anycast_mask, G_FILE_TEST_EXISTS)) - return NM_LINK_TYPE_OLPC_MESH; - - devtype = _linktype_read_devtype (ifname); - for (i = 0; devtype && i < G_N_ELEMENTS (linktypes); i++) { - if (g_strcmp0 (devtype, linktypes[i].devtype) == 0) { - if (linktypes[i].nm_type == NM_LINK_TYPE_BNEP) { - /* Both BNEP and 6lowpan use DEVTYPE=bluetooth, so we must - * use arptype to distinguish between them. - */ - if (arptype != ARPHRD_ETHER) - continue; + devtype = _linktype_read_devtype (dirfd); + for (i = 0; devtype && i < G_N_ELEMENTS (linktypes); i++) { + if (g_strcmp0 (devtype, linktypes[i].devtype) == 0) { + if (linktypes[i].nm_type == NM_LINK_TYPE_BNEP) { + /* Both BNEP and 6lowpan use DEVTYPE=bluetooth, so we must + * use arptype to distinguish between them. + */ + if (arptype != ARPHRD_ETHER) + continue; + } + return linktypes[i].nm_type; } - return linktypes[i].nm_type; } - } - /* Fallback for drivers that don't call SET_NETDEV_DEVTYPE() */ - if (wifi_utils_is_wifi (ifindex, ifname)) - return NM_LINK_TYPE_WIFI; + /* Fallback for drivers that don't call SET_NETDEV_DEVTYPE() */ + if (wifi_utils_is_wifi (dirfd, ifname_verified)) + return NM_LINK_TYPE_WIFI; + } if (arptype == ARPHRD_ETHER) { /* Misc non-upstream WWAN drivers. rmnet is Qualcomm's proprietary diff --git a/src/platform/wifi/wifi-utils.c b/src/platform/wifi/wifi-utils.c index 32210e0099..b8da02c15e 100644 --- a/src/platform/wifi/wifi-utils.c +++ b/src/platform/wifi/wifi-utils.c @@ -183,39 +183,19 @@ wifi_utils_deinit (WifiData *data) } gboolean -wifi_utils_is_wifi (int ifindex, const char *ifname) +wifi_utils_is_wifi (int dirfd, const char *ifname) { - int fd_sysnet; - int fd_phy80211; - char ifname_verified[IFNAMSIZ]; + g_return_val_if_fail (dirfd >= 0, FALSE); - g_return_val_if_fail (ifindex > 0, FALSE); - - fd_sysnet = nmp_utils_sysctl_open_netdir (ifindex, ifname, ifname_verified); - if (fd_sysnet < 0) - return FALSE; - - /* there might have been a race and ifname might be wrong. Below for checking - * wext, use the possibly improved name that we just verified. */ - ifname = ifname_verified; - - fd_phy80211 = openat (fd_sysnet, "phy80211", O_CLOEXEC); - close (fd_sysnet); - - if (fd_phy80211 >= 0) { - close (fd_phy80211); + if (faccessat (dirfd, "phy80211", F_OK, 0) == 0) return TRUE; - } - #if HAVE_WEXT if (wifi_wext_is_wifi (ifname)) return TRUE; #endif - return FALSE; } - /* OLPC Mesh-only functions */ guint32 diff --git a/src/platform/wifi/wifi-utils.h b/src/platform/wifi/wifi-utils.h index 3dca2ac1d5..4fd5a80bbe 100644 --- a/src/platform/wifi/wifi-utils.h +++ b/src/platform/wifi/wifi-utils.h @@ -28,7 +28,7 @@ typedef struct WifiData WifiData; -gboolean wifi_utils_is_wifi (int ifindex, const char *ifname); +gboolean wifi_utils_is_wifi (int dirfd, const char *ifname); WifiData *wifi_utils_init (const char *iface, int ifindex, gboolean check_scan); From a9a41edcbddc13cc1dc9ce1b6b8ac335b30713e5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 9 Dec 2016 12:47:17 +0100 Subject: [PATCH 17/27] core: use nmp_utils_sysctl_open_netdir() in platform link operations --- src/platform/nm-linux-platform.c | 63 +++++++++++--------------------- 1 file changed, 22 insertions(+), 41 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 103cdfa2e3..c4e53965a8 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -4578,44 +4578,27 @@ nla_put_failure: static char * link_get_physical_port_id (NMPlatform *platform, int ifindex) { - const char *ifname; - char *path, *id; + nm_auto_close int dirfd = -1; + char ifname_verified[IFNAMSIZ]; - ifname = nm_platform_link_get_name (platform, ifindex); - if (!ifname) + dirfd = nm_platform_sysctl_open_netdir (platform, ifindex, ifname_verified); + if (dirfd < 0) return NULL; - - ifname = NM_ASSERT_VALID_PATH_COMPONENT (ifname); - - path = g_strdup_printf ("/sys/class/net/%s/phys_port_id", ifname); - id = sysctl_get (platform, NMP_SYSCTL_PATHID_ABSOLUTE (path)); - g_free (path); - - return id; + return sysctl_get (platform, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname_verified, "phys_port_id")); } static guint link_get_dev_id (NMPlatform *platform, int ifindex) { - const char *ifname; - gs_free char *path = NULL, *id = NULL; - gint64 int_val; + nm_auto_close int dirfd = -1; + char ifname_verified[IFNAMSIZ]; - ifname = nm_platform_link_get_name (platform, ifindex); - if (!ifname) + dirfd = nm_platform_sysctl_open_netdir (platform, ifindex, ifname_verified); + if (dirfd < 0) return 0; - - ifname = NM_ASSERT_VALID_PATH_COMPONENT (ifname); - - path = g_strdup_printf ("/sys/class/net/%s/dev_id", ifname); - id = sysctl_get (platform, NMP_SYSCTL_PATHID_ABSOLUTE (path)); - if (!id || !*id) - return 0; - - /* Value is reported as hex */ - int_val = _nm_utils_ascii_str_to_int64 (id, 16, 0, G_MAXUINT16, 0); - - return errno ? 0 : (int) int_val; + return nm_platform_sysctl_get_int_checked (platform, + NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname_verified, "dev_id"), + 16, 0, G_MAXUINT16, 0); } static int @@ -5273,9 +5256,9 @@ _infiniband_partition_action (NMPlatform *platform, const NMPlatformLink **out_link) { NMLinuxPlatformPrivate *priv = NM_LINUX_PLATFORM_GET_PRIVATE (platform); - const NMPObject *obj_parent; + nm_auto_close int dirfd = -1; + char ifname_parent[IFNAMSIZ]; const NMPObject *obj; - char path[NM_STRLEN ("/sys/class/net/%s/%s") + IFNAMSIZ + 100]; char id[20]; char name[IFNAMSIZ]; gboolean success; @@ -5283,20 +5266,18 @@ _infiniband_partition_action (NMPlatform *platform, 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); - if (!obj_parent || !obj_parent->link.name[0]) { + dirfd = nm_platform_sysctl_open_netdir (platform, parent, ifname_parent); + if (dirfd < 0) { errno = ENOENT; return FALSE; } - nm_sprintf_buf (path, - "/sys/class/net/%s/%s", - NM_ASSERT_VALID_PATH_COMPONENT (obj_parent->link.name), - (action == INFINIBAND_ACTION_CREATE_CHILD - ? "create_child" - : "delete_child")); nm_sprintf_buf (id, "0x%04x", p_key); - success = nm_platform_sysctl_set (platform, NMP_SYSCTL_PATHID_ABSOLUTE (path), id); + if (action == INFINIBAND_ACTION_CREATE_CHILD) + success = nm_platform_sysctl_set (platform, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname_parent, "create_child"), id); + else + success = nm_platform_sysctl_set (platform, NMP_SYSCTL_PATHID_NETDIR (dirfd, ifname_parent, "delete_child"), id); + if (!success) { if ( action == INFINIBAND_ACTION_DELETE_CHILD && errno == ENODEV) @@ -5304,7 +5285,7 @@ _infiniband_partition_action (NMPlatform *platform, return FALSE; } - nm_utils_new_infiniband_name (name, obj_parent->link.name, p_key); + nm_utils_new_infiniband_name (name, ifname_parent, p_key); do_request_link (platform, 0, name); if (action == INFINIBAND_ACTION_DELETE_CHILD) From 22be2ae865d761c6e91bd979db28f6ac9c3796af Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 9 Dec 2016 13:03:43 +0100 Subject: [PATCH 18/27] core: use nmp_utils_sysctl_open_netdir() for platform master/slave options --- src/platform/nm-platform.c | 56 ++++++++++++++++++++-------------- src/platform/tests/test-link.c | 40 ++++++++++++------------ 2 files changed, 54 insertions(+), 42 deletions(-) diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 7cc675d3a9..015848d878 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -1708,34 +1708,44 @@ nm_platform_link_tun_add (NMPlatform *self, /*****************************************************************************/ -static char * -link_option_path (NMPlatform *self, int master, const char *category, const char *option) +static gboolean +link_set_option (NMPlatform *self, int ifindex, const char *category, const char *option, const char *value) { - const char *name = nm_platform_link_get_name (self, master); + nm_auto_close int dirfd = -1; + char ifname_verified[IFNAMSIZ]; + const char *path; - if (!name || !category || !option) + if (!category || !option) + return FALSE; + + dirfd = nm_platform_sysctl_open_netdir (self, ifindex, ifname_verified); + if (dirfd < 0) + return FALSE; + + path = nm_sprintf_bufa (strlen (category) + strlen (option) + 2, + "%s/%s", + category, option); + return nm_platform_sysctl_set (self, NMP_SYSCTL_PATHID_NETDIR_unsafe (dirfd, ifname_verified, path), value); +} + +static char * +link_get_option (NMPlatform *self, int ifindex, const char *category, const char *option) +{ + nm_auto_close int dirfd = -1; + char ifname_verified[IFNAMSIZ]; + const char *path; + + if (!category || !option) return NULL; - return g_strdup_printf ("/sys/class/net/%s/%s/%s", - NM_ASSERT_VALID_PATH_COMPONENT (name), - NM_ASSERT_VALID_PATH_COMPONENT (category), - NM_ASSERT_VALID_PATH_COMPONENT (option)); -} + dirfd = nm_platform_sysctl_open_netdir (self, ifindex, ifname_verified); + if (dirfd < 0) + return NULL; -static gboolean -link_set_option (NMPlatform *self, int master, const char *category, const char *option, const char *value) -{ - gs_free char *path = link_option_path (self, master, category, option); - - return path && nm_platform_sysctl_set (self, NMP_SYSCTL_PATHID_ABSOLUTE (path), value); -} - -static char * -link_get_option (NMPlatform *self, int master, const char *category, const char *option) -{ - gs_free char *path = link_option_path (self, master, category, option); - - return path ? nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_ABSOLUTE (path)) : NULL; + path = nm_sprintf_bufa (strlen (category) + strlen (option) + 2, + "%s/%s", + category, option); + return nm_platform_sysctl_get (self, NMP_SYSCTL_PATHID_NETDIR_unsafe (dirfd, ifname_verified, path)); } static const char * diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index ebeea1b30a..292f8b9021 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -398,26 +398,28 @@ test_software (NMLinkType link_type, const char *link_typename) accept_signal (link_changed); /* Set master option */ - switch (link_type) { - case NM_LINK_TYPE_BRIDGE: - if (nmtstp_is_sysfs_writable ()) { - g_assert (nm_platform_sysctl_master_set_option (NM_PLATFORM_GET, ifindex, "forward_delay", "628")); - value = nm_platform_sysctl_master_get_option (NM_PLATFORM_GET, ifindex, "forward_delay"); - g_assert_cmpstr (value, ==, "628"); - g_free (value); + if (nmtstp_is_root_test ()) { + switch (link_type) { + case NM_LINK_TYPE_BRIDGE: + if (nmtstp_is_sysfs_writable ()) { + g_assert (nm_platform_sysctl_master_set_option (NM_PLATFORM_GET, ifindex, "forward_delay", "628")); + value = nm_platform_sysctl_master_get_option (NM_PLATFORM_GET, ifindex, "forward_delay"); + g_assert_cmpstr (value, ==, "628"); + g_free (value); + } + break; + case NM_LINK_TYPE_BOND: + if (nmtstp_is_sysfs_writable ()) { + g_assert (nm_platform_sysctl_master_set_option (NM_PLATFORM_GET, ifindex, "mode", "active-backup")); + value = nm_platform_sysctl_master_get_option (NM_PLATFORM_GET, ifindex, "mode"); + /* When reading back, the output looks slightly different. */ + g_assert (g_str_has_prefix (value, "active-backup")); + g_free (value); + } + break; + default: + break; } - break; - case NM_LINK_TYPE_BOND: - if (nmtstp_is_sysfs_writable ()) { - g_assert (nm_platform_sysctl_master_set_option (NM_PLATFORM_GET, ifindex, "mode", "active-backup")); - value = nm_platform_sysctl_master_get_option (NM_PLATFORM_GET, ifindex, "mode"); - /* When reading back, the output looks slightly different. */ - g_assert (g_str_has_prefix (value, "active-backup")); - g_free (value); - } - break; - default: - break; } /* Enslave and release */ From 5eb4dfe7ab777dc5ab07aa9f66e61097725826ae Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 10 Dec 2016 14:53:14 +0100 Subject: [PATCH 19/27] platform/tests: add nmtstp_netns_select_random() util --- src/platform/tests/test-common.c | 19 +++++++++++++++++++ src/platform/tests/test-common.h | 4 ++++ 2 files changed, 23 insertions(+) diff --git a/src/platform/tests/test-common.c b/src/platform/tests/test-common.c index f23cec4606..860e6b7caa 100644 --- a/src/platform/tests/test-common.c +++ b/src/platform/tests/test-common.c @@ -1554,6 +1554,25 @@ nmtstp_namespace_get_fd_for_process (pid_t pid, const char *ns_name) /*****************************************************************************/ +void +nmtstp_netns_select_random (NMPlatform **platforms, gsize n_platforms, NMPNetns **netns) +{ + int i; + + g_assert (platforms); + g_assert (n_platforms && n_platforms <= G_MAXINT32); + g_assert (netns && !*netns); + for (i = 0; i < n_platforms; i++) + g_assert (NM_IS_PLATFORM (platforms[i])); + + i = nmtst_get_rand_int () % (n_platforms + 1); + if (i == 0) + return; + g_assert (nm_platform_netns_push (platforms[i - 1], netns)); +} + +/*****************************************************************************/ + NMTST_DEFINE(); static gboolean diff --git a/src/platform/tests/test-common.h b/src/platform/tests/test-common.h index a9f4959091..48a41de63a 100644 --- a/src/platform/tests/test-common.h +++ b/src/platform/tests/test-common.h @@ -53,6 +53,10 @@ int nmtstp_namespace_get_fd_for_process (pid_t pid, const char *ns_name); /*****************************************************************************/ +void nmtstp_netns_select_random (NMPlatform **platforms, gsize n_platforms, NMPNetns **netns); + +/*****************************************************************************/ + typedef struct { gulong handler_id; const char *name; From 6b132a2bd0f6566e70f28671fe0880f3f935cb75 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 10 Dec 2016 14:53:23 +0100 Subject: [PATCH 20/27] platform/tests: use nmtstp_netns_select_random() util --- src/platform/tests/test-link.c | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index 292f8b9021..0e1f0cbbc7 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -2058,7 +2058,6 @@ test_netns_set_netns (gpointer fixture, gconstpointer test_data) gs_unref_object NMPlatform *platform_1 = NULL; gs_unref_object NMPlatform *platform_2 = NULL; nm_auto_pop_netns NMPNetns *netns_pop = NULL; - int i; if (_test_netns_check_skip ()) return; @@ -2067,9 +2066,7 @@ test_netns_set_netns (gpointer fixture, gconstpointer test_data) platforms[1] = platform_1 = _test_netns_create_platform (); platforms[2] = platform_2 = _test_netns_create_platform (); - i = nmtst_get_rand_int () % 4; - if (i != 3) - g_assert (nm_platform_netns_push (platforms[i], &netns_pop)); + nmtstp_netns_select_random (platforms, G_N_ELEMENTS (platforms), &netns_pop); #define LINK_MOVE_NAME "link-move" g_assert (!nm_platform_link_get_by_ifname (platform_1, LINK_MOVE_NAME)); @@ -2296,9 +2293,7 @@ test_netns_bind_to_path (gpointer fixture, gconstpointer test_data) platforms[1] = platform_1 = _test_netns_create_platform (); platforms[2] = platform_2 = _test_netns_create_platform (); - i = nmtst_get_rand_int () % 4; - if (i != 3) - g_assert (nm_platform_netns_push (platforms[i], &netns_pop)); + nmtstp_netns_select_random (platforms, G_N_ELEMENTS (platforms), &netns_pop); g_assert_cmpint (mount ("tmpfs", P_VAR_RUN, "tmpfs", MS_NOATIME | MS_NODEV | MS_NOSUID, "mode=0755,size=32K"), ==, 0); g_assert_cmpint (mkdir (P_VAR_RUN_NETNS, 755), ==, 0); From e332c278245abc2c0d6e63ca9f36aa77961b15d5 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Fri, 9 Dec 2016 16:30:35 +0100 Subject: [PATCH 21/27] platform/tests: add tests for nmp_utils_sysctl_open_netdir() --- src/platform/tests/test-link.c | 170 +++++++++++++++++++++++++++++++++ 1 file changed, 170 insertions(+) diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index 0e1f0cbbc7..e9c6ccf92d 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -2322,6 +2322,173 @@ test_netns_bind_to_path (gpointer fixture, gconstpointer test_data) /*****************************************************************************/ +static void +test_sysctl_rename (void) +{ + NMPlatform *const PL = NM_PLATFORM_GET; + const char *const IFNAME[3] = { + "nm-dummy-0", + "nm-dummy-1", + "nm-dummy-2", + }; + int ifindex[G_N_ELEMENTS (IFNAME)] = { 0 }; + nm_auto_close int dirfd = -1; + int i; + char ifname_buf[IFNAMSIZ]; + char *s; + const NMPlatformLink *pllink; + + ifindex[0] = nmtstp_link_dummy_add (PL, -1, IFNAME[0])->ifindex; + ifindex[1] = nmtstp_link_dummy_add (PL, -1, IFNAME[1])->ifindex; + + s = (nmtst_get_rand_int () % 2) ? NULL : ifname_buf; + + if (nmtst_get_rand_int () % 2) { + /* bring the platform cache out of sync */ + nmtstp_run_command_check ("ip link set %s name %s", IFNAME[0], IFNAME[2]); + nm_platform_process_events (PL); + nmtstp_run_command_check ("ip link set %s name %s", IFNAME[2], IFNAME[0]); + + pllink = nm_platform_link_get_by_ifname (PL, IFNAME[2]); + g_assert (pllink && pllink->ifindex == ifindex[0]); + pllink = nm_platform_link_get_by_ifname (PL, IFNAME[0]); + g_assert (!pllink); + } + + /* open dirfd for IFNAME[0] */ + i = nmtst_get_rand_int () % (2 + G_N_ELEMENTS (IFNAME)); + if (i == 0) { + dirfd = nm_platform_sysctl_open_netdir (PL, + ifindex[0], + s); + } else { + const char *ifname_guess; + + /* provide a wrong or no guess. */ + ifname_guess = i == 1 ? NULL : IFNAME[i - 2]; + dirfd = nmp_utils_sysctl_open_netdir (ifindex[0], + ifname_guess, + s); + } + g_assert (dirfd >= 0); + if (s) + g_assert_cmpstr (s, ==, IFNAME[0]); + + /* possibly rename the interfaces. */ + switch (nmtst_get_rand_int () % 4) { + case 0: + break; + case 1: + nmtstp_run_command_check ("ip link set %s name %s", IFNAME[0], IFNAME[2]); + break; + case 2: + nmtstp_run_command_check ("ip link set %s name %s", IFNAME[0], IFNAME[2]); + nmtstp_run_command_check ("ip link set %s name %s", IFNAME[1], IFNAME[0]); + break; + } + + /* possibly, resync platform cache (should make no difference). */ + if (nmtst_get_rand_int () % 2) + nm_platform_process_events (PL); + + /* check that we still read the same file. */ + switch (nmtst_get_rand_int () % 2) { + case 0: { + gs_free char *c = NULL; + + if (nm_utils_file_get_contents (dirfd, "ifindex", 1*1024*1024, &c, NULL, NULL) < 0) + g_assert_not_reached(); + g_assert_cmpint (ifindex[0], ==, (int) _nm_utils_ascii_str_to_int64 (c, 10, 0, G_MAXINT, -1)); + break; + } + case 1: { + g_assert_cmpint (ifindex[0], ==, (gint32) nm_platform_sysctl_get_int32 (PL, NMP_SYSCTL_PATHID_NETDIR (dirfd, s ?: "", "ifindex"), -1)); + break; + } + default: + g_assert_not_reached (); + } + + nm_platform_process_events (PL); + nmtstp_link_del (PL, -1, ifindex[0], NULL); + nmtstp_link_del (PL, -1, ifindex[1], NULL); +} + +/*****************************************************************************/ + +static void +test_sysctl_netns_switch (void) +{ + const char *const IFNAME = "nm-dummy-0"; + int ifindex, ifindex_tmp; + nm_auto_close int dirfd = -1; + char ifname_buf[IFNAMSIZ]; + char *s; + gs_unref_object NMPlatform *platform_0 = NULL; + gs_unref_object NMPlatform *platform_1 = NULL; + gs_unref_object NMPlatform *platform_2 = NULL; + nm_auto_pop_netns NMPNetns *netns_pop_1 = NULL; + nm_auto_pop_netns NMPNetns *netns_pop_2 = NULL; + nm_auto_pop_netns NMPNetns *netns_pop_3 = NULL; + NMPlatform *PL; + NMPlatform *platforms[3]; + + if (_test_netns_check_skip ()) + return; + + platforms[0] = platform_0 = nm_linux_platform_new (TRUE); + platforms[1] = platform_1 = _test_netns_create_platform (); + platforms[2] = platform_2 = _test_netns_create_platform (); + PL = platforms[nmtst_get_rand_int () % 3]; + + nmtstp_netns_select_random (platforms, G_N_ELEMENTS (platforms), &netns_pop_1); + + ifindex = nmtstp_link_dummy_add (PL, FALSE, IFNAME)->ifindex; + + nmtstp_netns_select_random (platforms, G_N_ELEMENTS (platforms), &netns_pop_2); + + s = (nmtst_get_rand_int () % 2) ? NULL : ifname_buf; + dirfd = nm_platform_sysctl_open_netdir (PL, + ifindex, + s); + g_assert (dirfd >= 0); + if (s) + g_assert_cmpstr (s, ==, IFNAME); + + nmtstp_netns_select_random (platforms, G_N_ELEMENTS (platforms), &netns_pop_3); + + /* even if we switch to other namespaces, we can still lookup the path correctly, + * either using dirfd or via the platform instance (which switches namespace as needed). */ + { + gs_free char *c = NULL; + + if (nm_utils_file_get_contents (dirfd, "ifindex", 0, &c, NULL, NULL) < 0) + g_assert_not_reached(); + g_assert_cmpint (ifindex, ==, (int) _nm_utils_ascii_str_to_int64 (c, 10, 0, G_MAXINT, -1)); + } + g_assert_cmpint (ifindex, ==, (gint32) nm_platform_sysctl_get_int32 (PL, NMP_SYSCTL_PATHID_NETDIR (dirfd, s ?: "", "ifindex"), -1)); + g_assert_cmpint (ifindex, ==, (gint32) nm_platform_sysctl_get_int32 (PL, NMP_SYSCTL_PATHID_ABSOLUTE (nm_sprintf_bufa (100, "/sys/class/net/%s/ifindex", IFNAME)), -1)); + + /* accessing the path directly, only succeeds iff the current namespace happens to be the namespace + * in which we created the link. */ + { + gs_free char *c = NULL; + + if (nm_utils_file_get_contents (-1, nm_sprintf_bufa (100, "/sys/class/net/%s/ifindex", IFNAME), 0, &c, NULL, NULL) < 0) + ifindex_tmp = -1; + else + ifindex_tmp = _nm_utils_ascii_str_to_int64 (c, 10, 0, G_MAXINT, -2); + } + if (nmp_netns_get_current () == nm_platform_netns_get (PL)) + g_assert_cmpint (ifindex_tmp, ==, ifindex); + else + g_assert_cmpint (ifindex_tmp, ==, -1); + + nmtstp_link_del (PL, FALSE, ifindex, NULL); +} + +/*****************************************************************************/ + NMTstpSetupFunc const _nmtstp_setup_platform_func = SETUP; void @@ -2375,5 +2542,8 @@ _nmtstp_setup_tests (void) g_test_add_vtable ("/general/netns/set-netns", 0, NULL, _test_netns_setup, test_netns_set_netns, _test_netns_teardown); g_test_add_vtable ("/general/netns/push", 0, NULL, _test_netns_setup, test_netns_push, _test_netns_teardown); g_test_add_vtable ("/general/netns/bind-to-path", 0, NULL, _test_netns_setup, test_netns_bind_to_path, _test_netns_teardown); + + g_test_add_func ("/general/sysctl/rename", test_sysctl_rename); + g_test_add_func ("/general/sysctl/netns-switch", test_sysctl_netns_switch); } } From 4bdee37771ae741f4f9548b52c1db53ddf080fe8 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 10 Dec 2016 15:28:15 +0100 Subject: [PATCH 22/27] all: use O_CLOEXEC for file descriptors --- src/devices/adsl/nm-device-adsl.c | 4 ++-- src/devices/bluetooth/nm-bluez5-dun.c | 4 ++-- src/devices/tests/test-lldp.c | 4 ++-- src/devices/wwan/nm-modem.c | 2 +- src/dns/nm-dns-manager.c | 4 ++-- src/main-utils.c | 2 +- src/nm-core-utils.c | 2 +- src/nm-manager.c | 2 +- src/platform/nm-linux-platform.c | 2 +- src/platform/nm-platform-utils.c | 4 ++-- src/platform/nmp-netns.c | 6 +++--- src/platform/tests/test-common.c | 12 ++++++------ src/platform/wifi/wifi-utils-wext.c | 4 ++-- src/ppp/nm-ppp-manager.c | 4 ++-- src/settings/nm-inotify-helper.c | 2 +- src/settings/plugins/ifcfg-rh/shvar.c | 6 +++--- .../plugins/ifupdown/nms-ifupdown-interface-parser.c | 2 +- src/tests/test-general-with-expect.c | 3 ++- 18 files changed, 35 insertions(+), 34 deletions(-) diff --git a/src/devices/adsl/nm-device-adsl.c b/src/devices/adsl/nm-device-adsl.c index 8f8037c25f..9af6e69035 100644 --- a/src/devices/adsl/nm-device-adsl.c +++ b/src/devices/adsl/nm-device-adsl.c @@ -158,7 +158,7 @@ br2684_assign_vcc (NMDeviceAdsl *self, NMSettingAdsl *s_adsl) g_return_val_if_fail (priv->brfd == -1, FALSE); g_return_val_if_fail (priv->nas_ifname != NULL, FALSE); - priv->brfd = socket (PF_ATMPVC, SOCK_DGRAM, ATM_AAL5); + priv->brfd = socket (PF_ATMPVC, SOCK_DGRAM | SOCK_CLOEXEC, ATM_AAL5); if (priv->brfd < 0) { errsv = errno; _LOGE (LOGD_ADSL, "failed to open ATM control socket (%d)", errsv); @@ -344,7 +344,7 @@ br2684_create_iface (NMDeviceAdsl *self, nm_clear_g_source (&priv->nas_update_id); } - fd = socket (PF_ATMPVC, SOCK_DGRAM, ATM_AAL5); + fd = socket (PF_ATMPVC, SOCK_DGRAM | SOCK_CLOEXEC, ATM_AAL5); if (fd < 0) { errsv = errno; _LOGE (LOGD_ADSL, "failed to open ATM control socket (%d)", errsv); diff --git a/src/devices/bluetooth/nm-bluez5-dun.c b/src/devices/bluetooth/nm-bluez5-dun.c index 4c93feba60..aba3a0dd97 100644 --- a/src/devices/bluetooth/nm-bluez5-dun.c +++ b/src/devices/bluetooth/nm-bluez5-dun.c @@ -64,7 +64,7 @@ dun_connect (NMBluez5DunContext *context) .channel = context->rfcomm_channel }; - context->rfcomm_fd = socket (AF_BLUETOOTH, SOCK_STREAM, BTPROTO_RFCOMM); + context->rfcomm_fd = socket (AF_BLUETOOTH, SOCK_STREAM | SOCK_CLOEXEC, BTPROTO_RFCOMM); if (context->rfcomm_fd < 0) { int errsv = errno; error = g_error_new (NM_BT_ERROR, NM_BT_ERROR_DUN_CONNECT_FAILED, @@ -112,7 +112,7 @@ dun_connect (NMBluez5DunContext *context) context->rfcomm_id = devid; snprintf (tty, ttylen, "/dev/rfcomm%d", devid); - while ((context->rfcomm_tty_fd = open (tty, O_RDONLY | O_NOCTTY)) < 0 && try--) { + while ((context->rfcomm_tty_fd = open (tty, O_RDONLY | O_NOCTTY | O_CLOEXEC)) < 0 && try--) { if (try) { g_usleep (100 * 1000); continue; diff --git a/src/devices/tests/test-lldp.c b/src/devices/tests/test-lldp.c index 5d28d461df..4d25f9a681 100644 --- a/src/devices/tests/test-lldp.c +++ b/src/devices/tests/test-lldp.c @@ -350,7 +350,7 @@ _test_recv_fixture_setup (TestRecvFixture *fixture, gconstpointer user_data) struct ifreq ifr = { }; int fd, s; - fd = open ("/dev/net/tun", O_RDWR); + fd = open ("/dev/net/tun", O_RDWR | O_CLOEXEC); g_assert (fd >= 0); ifr.ifr_flags = IFF_TAP | IFF_NO_PI; @@ -358,7 +358,7 @@ _test_recv_fixture_setup (TestRecvFixture *fixture, gconstpointer user_data) g_assert (ioctl (fd, TUNSETIFF, &ifr) >= 0); /* Bring the interface up */ - s = socket (AF_INET, SOCK_DGRAM, 0); + s = socket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); g_assert (s >= 0); ifr.ifr_flags |= IFF_UP; g_assert (ioctl (s, SIOCSIFFLAGS, &ifr) >= 0); diff --git a/src/devices/wwan/nm-modem.c b/src/devices/wwan/nm-modem.c index 1c72c70e04..635f20ea74 100644 --- a/src/devices/wwan/nm-modem.c +++ b/src/devices/wwan/nm-modem.c @@ -500,7 +500,7 @@ port_speed_is_zero (const char *port) struct termios options; nm_auto_close int fd = -1; - fd = open (port, O_RDWR | O_NONBLOCK | O_NOCTTY); + fd = open (port, O_RDWR | O_NONBLOCK | O_NOCTTY | O_CLOEXEC); if (fd < 0) return FALSE; diff --git a/src/dns/nm-dns-manager.c b/src/dns/nm-dns-manager.c index c8ebe23a4c..c52777cff7 100644 --- a/src/dns/nm-dns-manager.c +++ b/src/dns/nm-dns-manager.c @@ -719,7 +719,7 @@ update_resolv_conf (NMDnsManager *self, } } - if ((f = fopen (MY_RESOLV_CONF_TMP, "w")) == NULL) { + if ((f = fopen (MY_RESOLV_CONF_TMP, "we")) == NULL) { errsv = errno; g_set_error (error, NM_MANAGER_ERROR, @@ -1594,7 +1594,7 @@ _check_resconf_immutable (NMDnsManagerResolvConfManager rc_manager) } } - fd = open (_PATH_RESCONF, O_RDONLY); + fd = open (_PATH_RESCONF, O_RDONLY | O_CLOEXEC); if (fd != -1) { if (ioctl (fd, FS_IOC_GETFLAGS, &flags) != -1) immutable = NM_FLAGS_HAS (flags, FS_IMMUTABLE_FL); diff --git a/src/main-utils.c b/src/main-utils.c index bad3141abf..9e3aa7bd55 100644 --- a/src/main-utils.c +++ b/src/main-utils.c @@ -95,7 +95,7 @@ nm_main_utils_write_pidfile (const char *pidfile) int fd; gboolean success = FALSE; - if ((fd = open (pidfile, O_CREAT|O_WRONLY|O_TRUNC, 00644)) < 0) { + if ((fd = open (pidfile, O_CREAT | O_WRONLY | O_TRUNC | O_CLOEXEC, 00644)) < 0) { fprintf (stderr, _("Opening %s failed: %s\n"), pidfile, strerror (errno)); return FALSE; } diff --git a/src/nm-core-utils.c b/src/nm-core-utils.c index 2577330647..70af933835 100644 --- a/src/nm-core-utils.c +++ b/src/nm-core-utils.c @@ -3050,7 +3050,7 @@ nm_utils_read_urandom (void *p, size_t nbytes) int r; again: - fd = open ("/dev/urandom", O_RDONLY|O_CLOEXEC|O_NOCTTY); + fd = open ("/dev/urandom", O_RDONLY | O_CLOEXEC | O_NOCTTY); if (fd < 0) { r = errno; if (r == EINTR) diff --git a/src/nm-manager.c b/src/nm-manager.c index 8414f18802..58a60b197f 100644 --- a/src/nm-manager.c +++ b/src/nm-manager.c @@ -5450,7 +5450,7 @@ rfkill_change (NMManager *self, const char *desc, RfKillType rtype, gboolean ena g_return_if_fail (rtype == RFKILL_TYPE_WLAN || rtype == RFKILL_TYPE_WWAN); errno = 0; - fd = open ("/dev/rfkill", O_RDWR); + fd = open ("/dev/rfkill", O_RDWR | O_CLOEXEC); if (fd < 0) { if (errno == EACCES) _LOGW (LOGD_RFKILL, "(%s): failed to open killswitch device", desc); diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index c4e53965a8..c6431912a3 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -5167,7 +5167,7 @@ tun_add (NMPlatform *platform, const char *name, gboolean tap, _LOGD ("link: add %s '%s' owner %" G_GINT64_FORMAT " group %" G_GINT64_FORMAT, tap ? "tap" : "tun", name, owner, group); - fd = open ("/dev/net/tun", O_RDWR); + fd = open ("/dev/net/tun", O_RDWR | O_CLOEXEC); if (fd < 0) return FALSE; diff --git a/src/platform/nm-platform-utils.c b/src/platform/nm-platform-utils.c index 78da4365fe..8f85884847 100644 --- a/src/platform/nm-platform-utils.c +++ b/src/platform/nm-platform-utils.c @@ -70,7 +70,7 @@ ethtool_get (const char *name, gpointer edata) nm_utils_ifname_cpy (ifr.ifr_name, name); ifr.ifr_data = edata; - fd = socket (PF_INET, SOCK_DGRAM, 0); + fd = socket (PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); if (fd < 0) { nm_log_err (LOGD_PLATFORM, "ethtool: Could not open socket."); return FALSE; @@ -410,7 +410,7 @@ nmp_utils_mii_supports_carrier_detect (const char *ifname) if (!nmp_utils_device_exists (ifname)) return FALSE; - fd = socket (PF_INET, SOCK_DGRAM, 0); + fd = socket (PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); if (fd < 0) { nm_log_err (LOGD_PLATFORM, "mii: couldn't open control socket (%s)", ifname); return FALSE; diff --git a/src/platform/nmp-netns.c b/src/platform/nmp-netns.c index f3704f0313..232c6efc9e 100644 --- a/src/platform/nmp-netns.c +++ b/src/platform/nmp-netns.c @@ -284,7 +284,7 @@ _netns_new (GError **error) int fd_net, fd_mnt; int errsv; - fd_net = open (PROC_SELF_NS_NET, O_RDONLY); + fd_net = open (PROC_SELF_NS_NET, O_RDONLY | O_CLOEXEC); if (fd_net == -1) { errsv = errno; g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, @@ -293,7 +293,7 @@ _netns_new (GError **error) return NULL; } - fd_mnt = open (PROC_SELF_NS_MNT, O_RDONLY); + fd_mnt = open (PROC_SELF_NS_MNT, O_RDONLY | O_CLOEXEC); if (fd_mnt == -1) { errsv = errno; g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, @@ -631,7 +631,7 @@ nmp_netns_bind_to_path (NMPNetns *self, const char *filename, int *out_fd) } if (out_fd) { - if ((fd = open (filename, O_RDONLY)) == -1) { + if ((fd = open (filename, O_RDONLY | O_CLOEXEC)) == -1) { errsv = errno; _LOGE (self, "bind: failed to open %s: %s", filename, g_strerror (errsv)); umount2 (filename, MNT_DETACH); diff --git a/src/platform/tests/test-common.c b/src/platform/tests/test-common.c index 860e6b7caa..89cbd188ed 100644 --- a/src/platform/tests/test-common.c +++ b/src/platform/tests/test-common.c @@ -1407,7 +1407,7 @@ nmtstp_namespace_create (int unshare_flags, GError **error) int pipefd_p2c[2]; ssize_t r; - e = pipe (pipefd_c2p); + e = pipe2 (pipefd_c2p, O_CLOEXEC); if (e != 0) { errsv = errno; g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, @@ -1415,7 +1415,7 @@ nmtstp_namespace_create (int unshare_flags, GError **error) return FALSE; } - e = pipe (pipefd_p2c); + e = pipe2 (pipefd_p2c, O_CLOEXEC); if (e != 0) { errsv = errno; g_set_error (error, NM_UTILS_ERROR, NM_UTILS_ERROR_UNKNOWN, @@ -1549,7 +1549,7 @@ nmtstp_namespace_get_fd_for_process (pid_t pid, const char *ns_name) nm_sprintf_buf (p, "/proc/%lu/ns/%s", (long unsigned) pid, ns_name); - return open(p, O_RDONLY); + return open(p, O_RDONLY | O_CLOEXEC); } /*****************************************************************************/ @@ -1592,21 +1592,21 @@ unshare_user (void) /* Since Linux 3.19 we have to disable setgroups() in order to map users. * Just proceed if the file is not there. */ - f = fopen ("/proc/self/setgroups", "w"); + f = fopen ("/proc/self/setgroups", "we"); if (f) { fprintf (f, "deny"); fclose (f); } /* Map current UID to root in NS to be created. */ - f = fopen ("/proc/self/uid_map", "w"); + f = fopen ("/proc/self/uid_map", "we"); if (!f) return FALSE; fprintf (f, "0 %d 1", uid); fclose (f); /* Map current GID to root in NS to be created. */ - f = fopen ("/proc/self/gid_map", "w"); + f = fopen ("/proc/self/gid_map", "we"); if (!f) return FALSE; fprintf (f, "0 %d 1", gid); diff --git a/src/platform/wifi/wifi-utils-wext.c b/src/platform/wifi/wifi-utils-wext.c index 3936bbe37c..398f3c558f 100644 --- a/src/platform/wifi/wifi-utils-wext.c +++ b/src/platform/wifi/wifi-utils-wext.c @@ -577,7 +577,7 @@ wifi_wext_init (const char *iface, int ifindex, gboolean check_scan) wext->parent.set_mesh_channel = wifi_wext_set_mesh_channel; wext->parent.set_mesh_ssid = wifi_wext_set_mesh_ssid; - wext->fd = socket (PF_INET, SOCK_DGRAM, 0); + wext->fd = socket (PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); if (wext->fd < 0) goto error; @@ -665,7 +665,7 @@ wifi_wext_is_wifi (const char *iface) if (!nmp_utils_device_exists (iface)) return FALSE; - fd = socket (PF_INET, SOCK_DGRAM, 0); + fd = socket (PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); if (fd >= 0) { nm_utils_ifname_cpy (iwr.ifr_ifrn.ifrn_name, iface); if (ioctl (fd, SIOCGIWNAME, &iwr) == 0) diff --git a/src/ppp/nm-ppp-manager.c b/src/ppp/nm-ppp-manager.c index c8d43299d9..e33d9fe6fc 100644 --- a/src/ppp/nm-ppp-manager.c +++ b/src/ppp/nm-ppp-manager.c @@ -148,7 +148,7 @@ monitor_cb (gpointer user_data) if (errno != ENODEV) _LOGW ("could not read ppp stats: %s", strerror (errno)); } else { - g_signal_emit (manager, signals[STATS], 0, + g_signal_emit (manager, signals[STATS], 0, stats.p.ppp_ibytes, stats.p.ppp_obytes); } @@ -165,7 +165,7 @@ monitor_stats (NMPPPManager *manager) if (priv->monitor_fd >= 0) return; - priv->monitor_fd = socket (AF_INET, SOCK_DGRAM, 0); + priv->monitor_fd = socket (AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); if (priv->monitor_fd >= 0) { g_warn_if_fail (priv->monitor_id == 0); if (priv->monitor_id) diff --git a/src/settings/nm-inotify-helper.c b/src/settings/nm-inotify-helper.c index 44a102b059..a0432a25c0 100644 --- a/src/settings/nm-inotify-helper.c +++ b/src/settings/nm-inotify-helper.c @@ -144,7 +144,7 @@ init_inotify (NMInotifyHelper *self) GIOChannel *channel; guint source_id; - priv->ifd = inotify_init (); + priv->ifd = inotify_init1 (IN_CLOEXEC); if (priv->ifd == -1) { int errsv = errno; diff --git a/src/settings/plugins/ifcfg-rh/shvar.c b/src/settings/plugins/ifcfg-rh/shvar.c index 22c942fce2..28989e1dfc 100644 --- a/src/settings/plugins/ifcfg-rh/shvar.c +++ b/src/settings/plugins/ifcfg-rh/shvar.c @@ -641,11 +641,11 @@ svOpenFileInternal (const char *name, gboolean create, GError **error) s->fd = -1; if (create) - s->fd = open (name, O_RDWR); /* NOT O_CREAT */ + s->fd = open (name, O_RDWR | O_CLOEXEC); /* NOT O_CREAT */ if (!create || s->fd == -1) { /* try read-only */ - s->fd = open (name, O_RDONLY); /* NOT O_CREAT */ + s->fd = open (name, O_RDONLY | O_CLOEXEC); /* NOT O_CREAT */ if (s->fd == -1) errsv = errno; else @@ -1017,7 +1017,7 @@ svWriteFile (shvarFile *s, int mode, GError **error) if (s->modified) { if (s->fd == -1) - s->fd = open (s->fileName, O_WRONLY | O_CREAT, mode); + s->fd = open (s->fileName, O_WRONLY | O_CREAT | O_CLOEXEC, mode); if (s->fd == -1) { int errsv = errno; diff --git a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c index df6d248c03..e86f521647 100644 --- a/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c +++ b/src/settings/plugins/ifupdown/nms-ifupdown-interface-parser.c @@ -117,7 +117,7 @@ _recursive_ifparser (const char *eni_file, int quiet) nm_log_warn (LOGD_SETTINGS, "interfaces file %s doesn't exist\n", eni_file); return; } - inp = fopen (eni_file, "r"); + inp = fopen (eni_file, "re"); if (inp == NULL) { if (!quiet) nm_log_warn (LOGD_SETTINGS, "Can't open %s\n", eni_file); diff --git a/src/tests/test-general-with-expect.c b/src/tests/test-general-with-expect.c index 9338557b41..fbed7799b1 100644 --- a/src/tests/test-general-with-expect.c +++ b/src/tests/test-general-with-expect.c @@ -26,6 +26,7 @@ #include #include #include +#include #include "NetworkManagerUtils.h" #include "nm-multi-index.h" @@ -173,7 +174,7 @@ test_nm_utils_kill_child_create_and_join_pgroup (void) int pipefd[2]; pid_t pgid; - err = pipe (pipefd); + err = pipe2 (pipefd, O_CLOEXEC); g_assert (err == 0); pgid = fork(); From da7b8dd85043d0bd2e33c4166b72bbde969b94a2 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sat, 10 Dec 2016 16:30:42 +0100 Subject: [PATCH 23/27] wifi: remove check for existing device in wifi_wext_is_wifi() See also commit ab41c13b0611c6cc967b055d328637a143b5c59b. --- src/platform/wifi/wifi-utils-wext.c | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/src/platform/wifi/wifi-utils-wext.c b/src/platform/wifi/wifi-utils-wext.c index 398f3c558f..af8cf2dee6 100644 --- a/src/platform/wifi/wifi-utils-wext.c +++ b/src/platform/wifi/wifi-utils-wext.c @@ -662,8 +662,15 @@ wifi_wext_is_wifi (const char *iface) struct iwreq iwr; gboolean is_wifi = FALSE; - if (!nmp_utils_device_exists (iface)) - return FALSE; + /* performing an ioctl on a non-existing name may cause the automatic + * loading of kernel modules, which should be avoided. + * + * Usually, we should thus make sure that an inteface with this name + * exists. + * + * Note that wifi_wext_is_wifi() has only one caller which just verified + * that an interface with this name exists. + */ fd = socket (PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); if (fd >= 0) { From 3641178508bb0d3b9890840390755dc4f2925440 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Sun, 11 Dec 2016 22:46:14 +0100 Subject: [PATCH 24/27] platform: lookup ifname for ethtool/mii ioctl immediately before use The ioctl APIs ethtool/mii require an interface ifname. That is inherrently racy as interfaces can be renamed. This cannot be fixed, we can only minimize the time between verifying the ifname and calling ioctl. We already had problems with that when ethtool would access an interface by name that didn't exists. See commit ab41c13b0611c6cc967b055d328637a143b5c59b . Checking for an existing interface only helps avoiding races when an interface gets deleted. It does not help against renaming. Go one step further, and instead of checking whether such an ifname exists, try to get the ifname based on the ifindex immediately before we need it. This brings an additional overhead for each ethtool access. --- src/devices/nm-device-ethernet.c | 8 +- src/platform/nm-linux-platform.c | 16 +-- src/platform/nm-platform-utils.c | 214 +++++++++++++++++++------------ src/platform/nm-platform-utils.h | 22 ++-- src/platform/nm-platform.c | 20 ++- src/platform/nm-platform.h | 6 +- src/platform/nmp-object.c | 8 +- src/platform/tests/test-link.c | 10 +- 8 files changed, 177 insertions(+), 127 deletions(-) diff --git a/src/devices/nm-device-ethernet.c b/src/devices/nm-device-ethernet.c index 5496bd9a51..2f9a092a0f 100644 --- a/src/devices/nm-device-ethernet.c +++ b/src/devices/nm-device-ethernet.c @@ -827,7 +827,7 @@ link_negotiation_set (NMDevice *device) } } - if (!nm_platform_ethtool_get_link_settings (NM_PLATFORM_GET, nm_device_get_iface (device), + if (!nm_platform_ethtool_get_link_settings (NM_PLATFORM_GET, nm_device_get_ifindex (device), &link_autoneg, &link_speed, &link_duplex)) { _LOGW (LOGD_DEVICE, "set-link: unable to retrieve link negotiation"); return; @@ -852,7 +852,7 @@ link_negotiation_set (NMDevice *device) } if (!nm_platform_ethtool_set_link_settings (NM_PLATFORM_GET, - nm_device_get_iface (device), + nm_device_get_ifindex (device), autoneg, speed, duplex)) { @@ -1243,7 +1243,7 @@ wake_on_lan_enable (NMDevice *device) } wol = NM_SETTING_WIRED_WAKE_ON_LAN_IGNORE; found: - return nm_platform_ethtool_set_wake_on_lan (NM_PLATFORM_GET, nm_device_get_iface (device), wol, password); + return nm_platform_ethtool_set_wake_on_lan (NM_PLATFORM_GET, nm_device_get_ifindex (device), wol, password); } /*****************************************************************************/ @@ -1609,7 +1609,7 @@ get_link_speed (NMDevice *device) NMDeviceEthernetPrivate *priv = NM_DEVICE_ETHERNET_GET_PRIVATE (self); guint32 speed; - if (!nm_platform_ethtool_get_link_settings (NM_PLATFORM_GET, nm_device_get_iface (device), NULL, &speed, NULL)) + if (!nm_platform_ethtool_get_link_settings (NM_PLATFORM_GET, nm_device_get_ifindex (device), NULL, &speed, NULL)) return; if (priv->speed == speed) return; diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index c6431912a3..96e14cb33f 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -682,7 +682,7 @@ _linktype_get_type (NMPlatform *platform, char ifname_verified[IFNAMSIZ]; /* Fallback OVS detection for kernel <= 3.16 */ - if (nmp_utils_ethtool_get_driver_info (ifname, &driver, NULL, NULL)) { + if (nmp_utils_ethtool_get_driver_info (ifindex, &driver, NULL, NULL)) { if (!g_strcmp0 (driver, "openvswitch")) return NM_LINK_TYPE_OPENVSWITCH; @@ -4451,10 +4451,6 @@ static gboolean link_supports_carrier_detect (NMPlatform *platform, int ifindex) { nm_auto_pop_netns NMPNetns *netns = NULL; - const char *name = nm_platform_link_get_name (platform, ifindex); - - if (!name) - return FALSE; if (!nm_platform_netns_push (platform, &netns)) return FALSE; @@ -4463,7 +4459,7 @@ link_supports_carrier_detect (NMPlatform *platform, int ifindex) * us whether the device actually supports carrier detection in the first * place. We assume any device that does implements one of these two APIs. */ - return nmp_utils_ethtool_supports_carrier_detect (name) || nmp_utils_mii_supports_carrier_detect (name); + return nmp_utils_ethtool_supports_carrier_detect (ifindex) || nmp_utils_mii_supports_carrier_detect (ifindex); } static gboolean @@ -4481,7 +4477,7 @@ link_supports_vlans (NMPlatform *platform, int ifindex) if (!nm_platform_netns_push (platform, &netns)) return FALSE; - return nmp_utils_ethtool_supports_vlans (obj->link.name); + return nmp_utils_ethtool_supports_vlans (ifindex); } static NMPlatformError @@ -4549,7 +4545,7 @@ link_get_permanent_address (NMPlatform *platform, if (!nm_platform_netns_push (platform, &netns)) return FALSE; - return nmp_utils_ethtool_get_permanent_address (nm_platform_link_get_name (platform, ifindex), buf, length); + return nmp_utils_ethtool_get_permanent_address (ifindex, buf, length); } static gboolean @@ -5519,7 +5515,7 @@ link_get_wake_on_lan (NMPlatform *platform, int ifindex) return FALSE; if (type == NM_LINK_TYPE_ETHERNET) - return nmp_utils_ethtool_get_wake_on_lan (nm_platform_link_get_name (platform, ifindex)); + return nmp_utils_ethtool_get_wake_on_lan (ifindex); else if (type == NM_LINK_TYPE_WIFI) { WifiData *wifi_data = wifi_get_wifi_data (platform, ifindex); @@ -5543,7 +5539,7 @@ link_get_driver_info (NMPlatform *platform, if (!nm_platform_netns_push (platform, &netns)) return FALSE; - return nmp_utils_ethtool_get_driver_info (nm_platform_link_get_name (platform, ifindex), + return nmp_utils_ethtool_get_driver_info (ifindex, out_driver_name, out_driver_version, out_fw_version); diff --git a/src/platform/nm-platform-utils.c b/src/platform/nm-platform-utils.c index 8f85884847..4e81325c90 100644 --- a/src/platform/nm-platform-utils.c +++ b/src/platform/nm-platform-utils.c @@ -44,6 +44,26 @@ extern char *if_indextoname (unsigned int __ifindex, char *__ifname); * ethtool ******************************************************************/ +NM_UTILS_ENUM2STR_DEFINE_STATIC (_ethtool_cmd_to_string, guint32, + NM_UTILS_ENUM2STR (ETHTOOL_GDRVINFO, "ETHTOOL_GDRVINFO"), + NM_UTILS_ENUM2STR (ETHTOOL_GFEATURES, "ETHTOOL_GFEATURES"), + NM_UTILS_ENUM2STR (ETHTOOL_GLINK, "ETHTOOL_GLINK"), + NM_UTILS_ENUM2STR (ETHTOOL_GPERMADDR, "ETHTOOL_GPERMADDR"), + NM_UTILS_ENUM2STR (ETHTOOL_GSET, "ETHTOOL_GSET"), + NM_UTILS_ENUM2STR (ETHTOOL_GSSET_INFO, "ETHTOOL_GSSET_INFO"), + NM_UTILS_ENUM2STR (ETHTOOL_GSTATS, "ETHTOOL_GSTATS"), + NM_UTILS_ENUM2STR (ETHTOOL_GSTRINGS, "ETHTOOL_GSTRINGS"), + NM_UTILS_ENUM2STR (ETHTOOL_GWOL, "ETHTOOL_GWOL"), + NM_UTILS_ENUM2STR (ETHTOOL_SSET, "ETHTOOL_SSET"), + NM_UTILS_ENUM2STR (ETHTOOL_SWOL, "ETHTOOL_SWOL"), +); + +static const char * +_ethtool_data_to_string (gconstpointer edata, char *buf, gsize len) +{ + return _ethtool_cmd_to_string (*((guint32 *) edata), buf, len); +} + #if LINUX_VERSION_CODE < KERNEL_VERSION(2,6,27) #define ethtool_cmd_speed(pedata) ((pedata)->speed) @@ -52,53 +72,83 @@ extern char *if_indextoname (unsigned int __ifindex, char *__ifname); #endif static gboolean -ethtool_get (const char *name, gpointer edata) +ethtool_get (int ifindex, gpointer edata) { - struct ifreq ifr; - int fd; + char ifname[IFNAMSIZ]; + char sbuf[50]; - if (!name || !*name) - return FALSE; + nm_assert (ifindex > 0); - if (!nmp_utils_device_exists (name)) - return FALSE; + /* ethtool ioctl API uses the ifname to refer to an interface. That is racy + * as interfaces can be renamed *sigh*. + * + * Note that we anyway have to verify whether the interface exists, before + * calling ioctl for a non-existing ifname. This is to prevent autoloading + * of kernel modules *sigh*. + * Thus, as we anyway verify the existence of ifname before doing the call, + * go one step further and lookup the ifname everytime anew. + * + * This does not solve the renaming race, but it minimizes the time for + * the race to happen as much as possible. */ - /* nmp_utils_device_exists() already errors out if @name is invalid. */ - nm_assert (strlen (name) < IFNAMSIZ); - - memset (&ifr, 0, sizeof (ifr)); - nm_utils_ifname_cpy (ifr.ifr_name, name); - ifr.ifr_data = edata; - - fd = socket (PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); - if (fd < 0) { - nm_log_err (LOGD_PLATFORM, "ethtool: Could not open socket."); + if (!if_indextoname (ifindex, ifname)) { + nm_log_trace (LOGD_PLATFORM, "ethtool[%d]: %s: request fails resolving ifindex: %s", + ifindex, + _ethtool_data_to_string (edata, sbuf, sizeof (sbuf)), + g_strerror (errno)); return FALSE; } - if (ioctl (fd, SIOCETHTOOL, &ifr) < 0) { - nm_log_dbg (LOGD_PLATFORM, "ethtool: Request failed: %s", strerror (errno)); - close (fd); - return FALSE; - } + { + nm_auto_close int fd = -1; + struct ifreq ifr = { + .ifr_data = edata, + }; - close (fd); - return TRUE; + memcpy (ifr.ifr_name, ifname, sizeof (ifname)); + + fd = socket (PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); + if (fd < 0) { + nm_log_trace (LOGD_PLATFORM, "ethtool[%d]: %s, %s: failed creating socket for ioctl: %s", + ifindex, + _ethtool_data_to_string (edata, sbuf, sizeof (sbuf)), + ifname, + g_strerror (errno)); + return FALSE; + } + + if (ioctl (fd, SIOCETHTOOL, &ifr) < 0) { + nm_log_trace (LOGD_PLATFORM, "ethtool[%d]: %s, %s: failed: %s", + ifindex, + _ethtool_data_to_string (edata, sbuf, sizeof (sbuf)), + ifname, + strerror (errno)); + return FALSE; + } + + nm_log_trace (LOGD_PLATFORM, "ethtool[%d]: %s, %s: success", + ifindex, + _ethtool_data_to_string (edata, sbuf, sizeof (sbuf)), + ifname); + return TRUE; + } } static int -ethtool_get_stringset_index (const char *ifname, int stringset_id, const char *string) +ethtool_get_stringset_index (int ifindex, int stringset_id, const char *string) { gs_free struct ethtool_sset_info *info = NULL; gs_free struct ethtool_gstrings *strings = NULL; guint32 len, i; + g_return_val_if_fail (ifindex > 0, -1); + info = g_malloc0 (sizeof (*info) + sizeof (guint32)); info->cmd = ETHTOOL_GSSET_INFO; info->reserved = 0; info->sset_mask = 1ULL << stringset_id; - if (!ethtool_get (ifname, info)) + if (!ethtool_get (ifindex, info)) return -1; if (!info->sset_mask) return -1; @@ -109,7 +159,7 @@ ethtool_get_stringset_index (const char *ifname, int stringset_id, const char *s strings->cmd = ETHTOOL_GSTRINGS; strings->string_set = stringset_id; strings->len = len; - if (!ethtool_get (ifname, strings)) + if (!ethtool_get (ifindex, strings)) return -1; for (i = 0; i < len; i++) { @@ -121,18 +171,17 @@ ethtool_get_stringset_index (const char *ifname, int stringset_id, const char *s } gboolean -nmp_utils_ethtool_get_driver_info (const char *ifname, +nmp_utils_ethtool_get_driver_info (int ifindex, char **out_driver_name, char **out_driver_version, char **out_fw_version) { struct ethtool_drvinfo drvinfo = { 0 }; - if (!ifname) - return FALSE; + g_return_val_if_fail (ifindex > 0, FALSE); drvinfo.cmd = ETHTOOL_GDRVINFO; - if (!ethtool_get (ifname, &drvinfo)) + if (!ethtool_get (ifindex, &drvinfo)) return FALSE; if (out_driver_name) @@ -146,7 +195,7 @@ nmp_utils_ethtool_get_driver_info (const char *ifname, } gboolean -nmp_utils_ethtool_get_permanent_address (const char *ifname, +nmp_utils_ethtool_get_permanent_address (int ifindex, guint8 *buf, size_t *length) { @@ -156,14 +205,13 @@ nmp_utils_ethtool_get_permanent_address (const char *ifname, } edata; guint i; - if (!ifname) - return FALSE; + g_return_val_if_fail (ifindex > 0, FALSE); memset (&edata, 0, sizeof (edata)); edata.e.cmd = ETHTOOL_GPERMADDR; edata.e.size = NM_UTILS_HWADDR_LEN_MAX; - if (!ethtool_get (ifname, &edata.e)) + if (!ethtool_get (ifindex, &edata.e)) return FALSE; if (edata.e.size > NM_UTILS_HWADDR_LEN_MAX) @@ -190,29 +238,30 @@ not_all_0or1: } gboolean -nmp_utils_ethtool_supports_carrier_detect (const char *ifname) +nmp_utils_ethtool_supports_carrier_detect (int ifindex) { struct ethtool_cmd edata = { .cmd = ETHTOOL_GLINK }; + g_return_val_if_fail (ifindex > 0, FALSE); + /* We ignore the result. If the ETHTOOL_GLINK call succeeded, then we * assume the device supports carrier-detect, otherwise we assume it * doesn't. */ - return ethtool_get (ifname, &edata); + return ethtool_get (ifindex, &edata); } gboolean -nmp_utils_ethtool_supports_vlans (const char *ifname) +nmp_utils_ethtool_supports_vlans (int ifindex) { gs_free struct ethtool_gfeatures *features = NULL; int idx, block, bit, size; - if (!ifname) - return FALSE; + g_return_val_if_fail (ifindex > 0, FALSE); - idx = ethtool_get_stringset_index (ifname, ETH_SS_FEATURES, "vlan-challenged"); + idx = ethtool_get_stringset_index (ifindex, ETH_SS_FEATURES, "vlan-challenged"); if (idx == -1) { - nm_log_dbg (LOGD_PLATFORM, "ethtool: vlan-challenged ethtool feature does not exist for %s?", ifname); + nm_log_dbg (LOGD_PLATFORM, "ethtool: vlan-challenged ethtool feature does not exist for %d?", ifindex); return FALSE; } @@ -224,54 +273,52 @@ nmp_utils_ethtool_supports_vlans (const char *ifname) features->cmd = ETHTOOL_GFEATURES; features->size = size; - if (!ethtool_get (ifname, features)) + if (!ethtool_get (ifindex, features)) return FALSE; return !(features->features[block].active & (1 << bit)); } int -nmp_utils_ethtool_get_peer_ifindex (const char *ifname) +nmp_utils_ethtool_get_peer_ifindex (int ifindex) { gs_free struct ethtool_stats *stats = NULL; int peer_ifindex_stat; - if (!ifname) - return 0; + g_return_val_if_fail (ifindex > 0, 0); - peer_ifindex_stat = ethtool_get_stringset_index (ifname, ETH_SS_STATS, "peer_ifindex"); + peer_ifindex_stat = ethtool_get_stringset_index (ifindex, ETH_SS_STATS, "peer_ifindex"); if (peer_ifindex_stat == -1) { - nm_log_dbg (LOGD_PLATFORM, "ethtool: peer_ifindex stat for %s does not exist?", ifname); + nm_log_dbg (LOGD_PLATFORM, "ethtool: peer_ifindex stat for %d does not exist?", ifindex); return FALSE; } stats = g_malloc0 (sizeof (*stats) + (peer_ifindex_stat + 1) * sizeof (guint64)); stats->cmd = ETHTOOL_GSTATS; stats->n_stats = peer_ifindex_stat + 1; - if (!ethtool_get (ifname, stats)) + if (!ethtool_get (ifindex, stats)) return 0; return stats->data[peer_ifindex_stat]; } gboolean -nmp_utils_ethtool_get_wake_on_lan (const char *ifname) +nmp_utils_ethtool_get_wake_on_lan (int ifindex) { struct ethtool_wolinfo wol; - if (!ifname) - return FALSE; + g_return_val_if_fail (ifindex > 0, FALSE); memset (&wol, 0, sizeof (wol)); wol.cmd = ETHTOOL_GWOL; - if (!ethtool_get (ifname, &wol)) + if (!ethtool_get (ifindex, &wol)) return FALSE; return wol.wolopts != 0; } gboolean -nmp_utils_ethtool_get_link_settings (const char *ifname, +nmp_utils_ethtool_get_link_settings (int ifindex, gboolean *out_autoneg, guint32 *out_speed, NMPlatformLinkDuplexType *out_duplex) @@ -280,7 +327,9 @@ nmp_utils_ethtool_get_link_settings (const char *ifname, .cmd = ETHTOOL_GSET, }; - if (!ethtool_get (ifname, &edata)) + g_return_val_if_fail (ifindex > 0, FALSE); + + if (!ethtool_get (ifindex, &edata)) return FALSE; if (out_autoneg) @@ -314,14 +363,19 @@ nmp_utils_ethtool_get_link_settings (const char *ifname, } gboolean -nmp_utils_ethtool_set_link_settings (const char *ifname, gboolean autoneg, guint32 speed, NMPlatformLinkDuplexType duplex) +nmp_utils_ethtool_set_link_settings (int ifindex, + gboolean autoneg, + guint32 speed, + NMPlatformLinkDuplexType duplex) { struct ethtool_cmd edata = { .cmd = ETHTOOL_GSET, }; + g_return_val_if_fail (ifindex > 0, FALSE); + /* retrieve first current settings */ - if (!ethtool_get (ifname, &edata)) + if (!ethtool_get (ifindex, &edata)) return FALSE; /* then change the needed ones */ @@ -349,16 +403,18 @@ nmp_utils_ethtool_set_link_settings (const char *ifname, gboolean autoneg, guint } } - return ethtool_get (ifname, &edata); + return ethtool_get (ifindex, &edata); } gboolean -nmp_utils_ethtool_set_wake_on_lan (const char *ifname, +nmp_utils_ethtool_set_wake_on_lan (int ifindex, NMSettingWiredWakeOnLan wol, const char *wol_password) { struct ethtool_wolinfo wol_info = { }; + g_return_val_if_fail (ifindex > 0, FALSE); + if (wol == NM_SETTING_WIRED_WAKE_ON_LAN_IGNORE) return TRUE; @@ -389,7 +445,7 @@ nmp_utils_ethtool_set_wake_on_lan (const char *ifname, wol_info.wolopts |= WAKE_MAGICSECURE; } - return ethtool_get (ifname, &wol_info); + return ethtool_get (ifindex, &wol_info); } /****************************************************************** @@ -397,51 +453,45 @@ nmp_utils_ethtool_set_wake_on_lan (const char *ifname, ******************************************************************/ gboolean -nmp_utils_mii_supports_carrier_detect (const char *ifname) +nmp_utils_mii_supports_carrier_detect (int ifindex) { - int fd, errsv; + char ifname[IFNAMSIZ]; + nm_auto_close int fd = -1; struct ifreq ifr; struct mii_ioctl_data *mii; - gboolean supports_mii = FALSE; - if (!ifname) - return FALSE; + g_return_val_if_fail (ifindex > 0, FALSE); - if (!nmp_utils_device_exists (ifname)) + if (!if_indextoname (ifindex, ifname)) { + nm_log_trace (LOGD_PLATFORM, "mii[%d]: carrier-detect no: request fails resolving ifindex: %s", ifindex, g_strerror (errno)); return FALSE; + } fd = socket (PF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); if (fd < 0) { - nm_log_err (LOGD_PLATFORM, "mii: couldn't open control socket (%s)", ifname); + nm_log_trace (LOGD_PLATFORM, "mii[%d,%s]: carrier-detect no: couldn't open control socket: %s", ifindex, ifname, g_strerror (errno)); return FALSE; } memset (&ifr, 0, sizeof (struct ifreq)); - nm_utils_ifname_cpy (ifr.ifr_name, ifname); + memcpy (ifr.ifr_name, ifname, IFNAMSIZ); - errno = 0; if (ioctl (fd, SIOCGMIIPHY, &ifr) < 0) { - errsv = errno; - nm_log_dbg (LOGD_PLATFORM, "mii: SIOCGMIIPHY failed: %s (%d) (%s)", strerror (errsv), errsv, ifname); - goto out; + nm_log_trace (LOGD_PLATFORM, "mii[%d,%s]: carrier-detect no: SIOCGMIIPHY failed: %s", ifindex, ifname, strerror (errno)); + return FALSE; } /* If we can read the BMSR register, we assume that the card supports MII link detection */ mii = (struct mii_ioctl_data *) &ifr.ifr_ifru; mii->reg_num = MII_BMSR; - if (ioctl (fd, SIOCGMIIREG, &ifr) == 0) { - nm_log_dbg (LOGD_PLATFORM, "mii: SIOCGMIIREG result 0x%X (%s)", mii->val_out, ifname); - supports_mii = TRUE; - } else { - errsv = errno; - nm_log_dbg (LOGD_PLATFORM, "mii: SIOCGMIIREG failed: %s (%d) (%s)", strerror (errsv), errsv, ifname); + if (ioctl (fd, SIOCGMIIREG, &ifr) != 0) { + nm_log_trace (LOGD_PLATFORM, "mii[%d,%s]: carrier-detect no: SIOCGMIIREG failed: %s", ifindex, ifname, strerror (errno)); + return FALSE; } -out: - close (fd); - nm_log_dbg (LOGD_PLATFORM, "mii: MII %s supported (%s)", supports_mii ? "is" : "not", ifname); - return supports_mii; + nm_log_trace (LOGD_PLATFORM, "mii[%d,%s]: carrier-detect yes: SIOCGMIIREG result 0x%X", ifindex, ifname, mii->val_out); + return TRUE; } /****************************************************************** diff --git a/src/platform/nm-platform-utils.h b/src/platform/nm-platform-utils.h index c99babf86f..e5a19c5ab6 100644 --- a/src/platform/nm-platform-utils.h +++ b/src/platform/nm-platform-utils.h @@ -27,28 +27,28 @@ #include "nm-setting-wired.h" -const char *nmp_utils_ethtool_get_driver (const char *ifname); -gboolean nmp_utils_ethtool_supports_carrier_detect (const char *ifname); -gboolean nmp_utils_ethtool_supports_vlans (const char *ifname); -int nmp_utils_ethtool_get_peer_ifindex (const char *ifname); -gboolean nmp_utils_ethtool_get_wake_on_lan (const char *ifname); -gboolean nmp_utils_ethtool_set_wake_on_lan (const char *ifname, NMSettingWiredWakeOnLan wol, +const char *nmp_utils_ethtool_get_driver (int ifindex); +gboolean nmp_utils_ethtool_supports_carrier_detect (int ifindex); +gboolean nmp_utils_ethtool_supports_vlans (int ifindex); +int nmp_utils_ethtool_get_peer_ifindex (int ifindex); +gboolean nmp_utils_ethtool_get_wake_on_lan (int ifindex); +gboolean nmp_utils_ethtool_set_wake_on_lan (int ifindex, NMSettingWiredWakeOnLan wol, const char *wol_password); -gboolean nmp_utils_ethtool_get_link_settings (const char *ifname, gboolean *out_autoneg, guint32 *out_speed, NMPlatformLinkDuplexType *out_duplex); -gboolean nmp_utils_ethtool_set_link_settings (const char *ifname, gboolean autoneg, guint32 speed, NMPlatformLinkDuplexType duplex); +gboolean nmp_utils_ethtool_get_link_settings (int ifindex, gboolean *out_autoneg, guint32 *out_speed, NMPlatformLinkDuplexType *out_duplex); +gboolean nmp_utils_ethtool_set_link_settings (int ifindex, gboolean autoneg, guint32 speed, NMPlatformLinkDuplexType duplex); -gboolean nmp_utils_ethtool_get_driver_info (const char *ifname, +gboolean nmp_utils_ethtool_get_driver_info (int ifindex, char **out_driver_name, char **out_driver_version, char **out_fw_version); -gboolean nmp_utils_ethtool_get_permanent_address (const char *ifname, +gboolean nmp_utils_ethtool_get_permanent_address (int ifindex, guint8 *buf, size_t *length); -gboolean nmp_utils_mii_supports_carrier_detect (const char *ifname); +gboolean nmp_utils_mii_supports_carrier_detect (int ifindex); const char *nmp_utils_udev_get_driver (GUdevDevice *device); diff --git a/src/platform/nm-platform.c b/src/platform/nm-platform.c index 015848d878..d0b65adf08 100644 --- a/src/platform/nm-platform.c +++ b/src/platform/nm-platform.c @@ -2259,7 +2259,7 @@ nm_platform_link_veth_get_properties (NMPlatform *self, int ifindex, int *out_pe if (!nm_platform_netns_push (self, &netns)) return FALSE; - peer_ifindex = nmp_utils_ethtool_get_peer_ifindex (plink->name); + peer_ifindex = nmp_utils_ethtool_get_peer_ifindex (plink->ifindex); if (peer_ifindex <= 0) return FALSE; @@ -2507,27 +2507,33 @@ _to_string_ifa_flags (guint32 ifa_flags, char *buf, gsize size) /*****************************************************************************/ gboolean -nm_platform_ethtool_set_wake_on_lan (NMPlatform *self, const char *ifname, NMSettingWiredWakeOnLan wol, const char *wol_password) +nm_platform_ethtool_set_wake_on_lan (NMPlatform *self, int ifindex, NMSettingWiredWakeOnLan wol, const char *wol_password) { _CHECK_SELF_NETNS (self, klass, netns, FALSE); - return nmp_utils_ethtool_set_wake_on_lan (ifname, wol, wol_password); + g_return_val_if_fail (ifindex > 0, FALSE); + + return nmp_utils_ethtool_set_wake_on_lan (ifindex, wol, wol_password); } gboolean -nm_platform_ethtool_set_link_settings (NMPlatform *self, const char *ifname, gboolean autoneg, guint32 speed, NMPlatformLinkDuplexType duplex) +nm_platform_ethtool_set_link_settings (NMPlatform *self, int ifindex, gboolean autoneg, guint32 speed, NMPlatformLinkDuplexType duplex) { _CHECK_SELF_NETNS (self, klass, netns, FALSE); - return nmp_utils_ethtool_set_link_settings (ifname, autoneg, speed, duplex); + g_return_val_if_fail (ifindex > 0, FALSE); + + return nmp_utils_ethtool_set_link_settings (ifindex, autoneg, speed, duplex); } gboolean -nm_platform_ethtool_get_link_settings (NMPlatform *self, const char *ifname, gboolean *out_autoneg, guint32 *out_speed, NMPlatformLinkDuplexType *out_duplex) +nm_platform_ethtool_get_link_settings (NMPlatform *self, int ifindex, gboolean *out_autoneg, guint32 *out_speed, NMPlatformLinkDuplexType *out_duplex) { _CHECK_SELF_NETNS (self, klass, netns, FALSE); - return nmp_utils_ethtool_get_link_settings (ifname, out_autoneg, out_speed, out_duplex); + g_return_val_if_fail (ifindex > 0, FALSE); + + return nmp_utils_ethtool_get_link_settings (ifindex, out_autoneg, out_speed, out_duplex); } /*****************************************************************************/ diff --git a/src/platform/nm-platform.h b/src/platform/nm-platform.h index 2e9dbcc844..55d7a6f20e 100644 --- a/src/platform/nm-platform.h +++ b/src/platform/nm-platform.h @@ -992,8 +992,8 @@ const char *nm_platform_route_scope2str (int scope, char *buf, gsize len); int nm_platform_ip_address_cmp_expiry (const NMPlatformIPAddress *a, const NMPlatformIPAddress *b); -gboolean nm_platform_ethtool_set_wake_on_lan (NMPlatform *self, const char *ifname, NMSettingWiredWakeOnLan wol, const char *wol_password); -gboolean nm_platform_ethtool_set_link_settings (NMPlatform *self, const char *ifname, gboolean autoneg, guint32 speed, NMPlatformLinkDuplexType duplex); -gboolean nm_platform_ethtool_get_link_settings (NMPlatform *self, const char *ifname, gboolean *out_autoneg, guint32 *out_speed, NMPlatformLinkDuplexType *out_duplex); +gboolean nm_platform_ethtool_set_wake_on_lan (NMPlatform *self, int ifindex, NMSettingWiredWakeOnLan wol, const char *wol_password); +gboolean nm_platform_ethtool_set_link_settings (NMPlatform *self, int ifindex, gboolean autoneg, guint32 speed, NMPlatformLinkDuplexType duplex); +gboolean nm_platform_ethtool_get_link_settings (NMPlatform *self, int ifindex, gboolean *out_autoneg, guint32 *out_speed, NMPlatformLinkDuplexType *out_duplex); #endif /* __NETWORKMANAGER_PLATFORM_H__ */ diff --git a/src/platform/nmp-object.c b/src/platform/nmp-object.c index 936c41f270..613f6f309a 100644 --- a/src/platform/nmp-object.c +++ b/src/platform/nmp-object.c @@ -125,7 +125,7 @@ _vlan_xgress_qos_mappings_cpy (guint *dst_n_map, /*****************************************************************************/ static const char * -_link_get_driver (GUdevDevice *udev_device, const char *kind, const char *ifname) +_link_get_driver (GUdevDevice *udev_device, const char *kind, int ifindex) { const char *driver = NULL; @@ -140,10 +140,10 @@ _link_get_driver (GUdevDevice *udev_device, const char *kind, const char *ifname if (kind) return kind; - if (ifname) { + if (ifindex > 0) { char *d; - if (nmp_utils_ethtool_get_driver_info (ifname, &d, NULL, NULL)) { + if (nmp_utils_ethtool_get_driver_info (ifindex, &d, NULL, NULL)) { driver = d && d[0] ? g_intern_string (d) : NULL; g_free (d); if (driver) @@ -169,7 +169,7 @@ _nmp_object_fixup_link_udev_fields (NMPObject *obj, gboolean use_udev) if (obj->_link.netlink.is_in_netlink) { driver = _link_get_driver (obj->_link.udev.device, obj->link.kind, - obj->link.name); + obj->link.ifindex); if (obj->_link.udev.device) initialized = TRUE; else if (!use_udev) { diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index e9c6ccf92d..8c50b95c76 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -2026,9 +2026,8 @@ test_netns_general (gpointer fixture, gconstpointer test_data) * skip asserts that are known to fail. */ ethtool_support = nmtstp_run_command ("ethtool -i dummy1_ > /dev/null") == 0; if (ethtool_support) { - g_assert ( nmp_utils_ethtool_get_driver_info ("dummy1_", NULL, NULL, NULL)); - g_assert ( nmp_utils_ethtool_get_driver_info ("dummy2a", NULL, NULL, NULL)); - g_assert (!nmp_utils_ethtool_get_driver_info ("dummy2b", NULL, NULL, NULL)); + g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_1, 0, "dummy1_", NM_LINK_TYPE_DUMMY)->ifindex, NULL, NULL, NULL)); + g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_1, 0, "dummy2a", NM_LINK_TYPE_DUMMY)->ifindex, NULL, NULL, NULL)); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy1_ > /dev/null"), ==, 0); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy2a > /dev/null"), ==, 0); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy2b 2> /dev/null"), !=, 0); @@ -2037,9 +2036,8 @@ test_netns_general (gpointer fixture, gconstpointer test_data) g_assert (nm_platform_netns_push (platform_2, &netns_tmp)); if (ethtool_support) { - g_assert ( nmp_utils_ethtool_get_driver_info ("dummy1_", NULL, NULL, NULL)); - g_assert (!nmp_utils_ethtool_get_driver_info ("dummy2a", NULL, NULL, NULL)); - g_assert ( nmp_utils_ethtool_get_driver_info ("dummy2b", NULL, NULL, NULL)); + g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_2, 0, "dummy1_", NM_LINK_TYPE_DUMMY)->ifindex, NULL, NULL, NULL)); + g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_2, 0, "dummy2b", NM_LINK_TYPE_DUMMY)->ifindex, NULL, NULL, NULL)); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy1_ > /dev/null"), ==, 0); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy2a 2> /dev/null"), !=, 0); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy2b > /dev/null"), ==, 0); From 16ad046c875c7d8e6b606d8348a3b1f89a5cf876 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Dec 2016 14:41:10 +0100 Subject: [PATCH 25/27] platform: remove unused nmp_utils_device_exists() util --- src/platform/nm-platform-utils.c | 16 ---------------- src/platform/nm-platform-utils.h | 2 -- 2 files changed, 18 deletions(-) diff --git a/src/platform/nm-platform-utils.c b/src/platform/nm-platform-utils.c index 4e81325c90..219119ee42 100644 --- a/src/platform/nm-platform-utils.c +++ b/src/platform/nm-platform-utils.c @@ -539,22 +539,6 @@ out: * utils *****************************************************************************/ -gboolean -nmp_utils_device_exists (const char *name) -{ -#define SYS_CLASS_NET "/sys/class/net/" - char sysdir[NM_STRLEN (SYS_CLASS_NET) + IFNAMSIZ]; - - if ( !name - || strlen (name) >= IFNAMSIZ - || !nm_utils_is_valid_path_component (name)) - g_return_val_if_reached (FALSE); - - memcpy (sysdir, SYS_CLASS_NET, NM_STRLEN (SYS_CLASS_NET)); - nm_utils_ifname_cpy (&sysdir[NM_STRLEN (SYS_CLASS_NET)], name); - return g_file_test (sysdir, G_FILE_TEST_EXISTS); -} - NMIPConfigSource nmp_utils_ip_config_source_from_rtprot (guint8 rtprot) { diff --git a/src/platform/nm-platform-utils.h b/src/platform/nm-platform-utils.h index e5a19c5ab6..592dbb7aa1 100644 --- a/src/platform/nm-platform-utils.h +++ b/src/platform/nm-platform-utils.h @@ -53,8 +53,6 @@ gboolean nmp_utils_mii_supports_carrier_detect (int ifindex); const char *nmp_utils_udev_get_driver (GUdevDevice *device); -gboolean nmp_utils_device_exists (const char *name); - NMIPConfigSource nmp_utils_ip_config_source_from_rtprot (guint8 rtprot) _nm_const; guint8 nmp_utils_ip_config_source_coerce_to_rtprot (NMIPConfigSource source) _nm_const; NMIPConfigSource nmp_utils_ip_config_source_coerce_from_rtprot (NMIPConfigSource source) _nm_const; From d32fb8158bd64add544f49d5bb870a1281b32488 Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Dec 2016 13:47:52 +0100 Subject: [PATCH 26/27] platform: avoid copying arguments for nmp_utils_ethtool_get_driver_info() We call nmp_utils_ethtool_get_driver_info() twice when receiving a netlink message, but we don't need a clone of the string values. Instead, expose a data structure that should be stack allocated by the caller. --- src/platform/nm-linux-platform.c | 19 +++++++++++-------- src/platform/nm-platform-utils.c | 29 ++++++++++++++--------------- src/platform/nm-platform-utils.h | 21 ++++++++++++++++++--- src/platform/nmp-object.c | 10 ++++------ src/platform/tests/test-link.c | 9 +++++---- 5 files changed, 52 insertions(+), 36 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index 96e14cb33f..e491a85639 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -676,21 +676,21 @@ _linktype_get_type (NMPlatform *platform, return NM_LINK_TYPE_IP6TNL; if (ifname) { + NMPUtilsEthtoolDriverInfo driver_info; nm_auto_close int dirfd = -1; - gs_free char *driver = NULL; gs_free char *devtype = NULL; char ifname_verified[IFNAMSIZ]; /* Fallback OVS detection for kernel <= 3.16 */ - if (nmp_utils_ethtool_get_driver_info (ifindex, &driver, NULL, NULL)) { - if (!g_strcmp0 (driver, "openvswitch")) + if (nmp_utils_ethtool_get_driver_info (ifindex, &driver_info)) { + if (nm_streq (driver_info.driver, "openvswitch")) return NM_LINK_TYPE_OPENVSWITCH; if (arptype == 256) { /* Some s390 CTC-type devices report 256 for the encapsulation type * for some reason, but we need to call them Ethernet. */ - if (!g_strcmp0 (driver, "ctcm")) + if (nm_streq (driver_info.driver, "ctcm")) return NM_LINK_TYPE_ETHERNET; } } @@ -5535,14 +5535,17 @@ link_get_driver_info (NMPlatform *platform, char **out_fw_version) { nm_auto_pop_netns NMPNetns *netns = NULL; + NMPUtilsEthtoolDriverInfo driver_info; if (!nm_platform_netns_push (platform, &netns)) return FALSE; - return nmp_utils_ethtool_get_driver_info (ifindex, - out_driver_name, - out_driver_version, - out_fw_version); + if (!nmp_utils_ethtool_get_driver_info (ifindex, &driver_info)) + return FALSE; + NM_SET_OUT (out_driver_name, g_strdup (driver_info.driver)); + NM_SET_OUT (out_driver_version, g_strdup (driver_info.version)); + NM_SET_OUT (out_fw_version, g_strdup (driver_info.fw_version)); + return TRUE; } /*****************************************************************************/ diff --git a/src/platform/nm-platform-utils.c b/src/platform/nm-platform-utils.c index 219119ee42..8b6994e20e 100644 --- a/src/platform/nm-platform-utils.c +++ b/src/platform/nm-platform-utils.c @@ -172,26 +172,25 @@ ethtool_get_stringset_index (int ifindex, int stringset_id, const char *string) gboolean nmp_utils_ethtool_get_driver_info (int ifindex, - char **out_driver_name, - char **out_driver_version, - char **out_fw_version) + NMPUtilsEthtoolDriverInfo *data) { - struct ethtool_drvinfo drvinfo = { 0 }; + struct ethtool_drvinfo *drvinfo; + G_STATIC_ASSERT (sizeof (*data) == sizeof (*drvinfo)); + G_STATIC_ASSERT (offsetof (NMPUtilsEthtoolDriverInfo, driver) == offsetof (struct ethtool_drvinfo, driver)); + G_STATIC_ASSERT (offsetof (NMPUtilsEthtoolDriverInfo, version) == offsetof (struct ethtool_drvinfo, version)); + G_STATIC_ASSERT (offsetof (NMPUtilsEthtoolDriverInfo, fw_version) == offsetof (struct ethtool_drvinfo, fw_version)); + G_STATIC_ASSERT (sizeof (data->driver) == sizeof (drvinfo->driver)); + G_STATIC_ASSERT (sizeof (data->version) == sizeof (drvinfo->version)); + G_STATIC_ASSERT (sizeof (data->fw_version) == sizeof (drvinfo->fw_version)); g_return_val_if_fail (ifindex > 0, FALSE); + g_return_val_if_fail (data, FALSE); - drvinfo.cmd = ETHTOOL_GDRVINFO; - if (!ethtool_get (ifindex, &drvinfo)) - return FALSE; + drvinfo = (struct ethtool_drvinfo *) data; - if (out_driver_name) - *out_driver_name = g_strdup (drvinfo.driver); - if (out_driver_version) - *out_driver_version = g_strdup (drvinfo.version); - if (out_fw_version) - *out_fw_version = g_strdup (drvinfo.fw_version); - - return TRUE; + memset (drvinfo, 0, sizeof (*drvinfo)); + drvinfo->cmd = ETHTOOL_GDRVINFO; + return ethtool_get (ifindex, drvinfo); } gboolean diff --git a/src/platform/nm-platform-utils.h b/src/platform/nm-platform-utils.h index 592dbb7aa1..ea9607e6a3 100644 --- a/src/platform/nm-platform-utils.h +++ b/src/platform/nm-platform-utils.h @@ -38,10 +38,25 @@ gboolean nmp_utils_ethtool_set_wake_on_lan (int ifindex, NMSettingWiredWakeOnLan gboolean nmp_utils_ethtool_get_link_settings (int ifindex, gboolean *out_autoneg, guint32 *out_speed, NMPlatformLinkDuplexType *out_duplex); gboolean nmp_utils_ethtool_set_link_settings (int ifindex, gboolean autoneg, guint32 speed, NMPlatformLinkDuplexType duplex); +typedef struct { + /* We don't want to include in header files, + * thus create a ABI compatible version of struct ethtool_drvinfo.*/ + guint32 _private_cmd; + char driver[32]; + char version[32]; + char fw_version[32]; + char _private_bus_info[32]; + char _private_erom_version[32]; + char _private_reserved2[12]; + guint32 _private_n_priv_flags; + guint32 _private_n_stats; + guint32 _private_testinfo_len; + guint32 _private_eedump_len; + guint32 _private_regdump_len; +} NMPUtilsEthtoolDriverInfo; + gboolean nmp_utils_ethtool_get_driver_info (int ifindex, - char **out_driver_name, - char **out_driver_version, - char **out_fw_version); + NMPUtilsEthtoolDriverInfo *data); gboolean nmp_utils_ethtool_get_permanent_address (int ifindex, guint8 *buf, diff --git a/src/platform/nmp-object.c b/src/platform/nmp-object.c index 613f6f309a..c0380ec567 100644 --- a/src/platform/nmp-object.c +++ b/src/platform/nmp-object.c @@ -141,13 +141,11 @@ _link_get_driver (GUdevDevice *udev_device, const char *kind, int ifindex) return kind; if (ifindex > 0) { - char *d; + NMPUtilsEthtoolDriverInfo driver_info; - if (nmp_utils_ethtool_get_driver_info (ifindex, &d, NULL, NULL)) { - driver = d && d[0] ? g_intern_string (d) : NULL; - g_free (d); - if (driver) - return driver; + if (nmp_utils_ethtool_get_driver_info (ifindex, &driver_info)) { + if (driver_info.driver[0]) + return g_intern_string (driver_info.driver); } } diff --git a/src/platform/tests/test-link.c b/src/platform/tests/test-link.c index 8c50b95c76..c2b3de1391 100644 --- a/src/platform/tests/test-link.c +++ b/src/platform/tests/test-link.c @@ -1956,6 +1956,7 @@ test_netns_general (gpointer fixture, gconstpointer test_data) char sbuf[100]; int i, j, k; gboolean ethtool_support; + NMPUtilsEthtoolDriverInfo driver_info; if (_test_netns_check_skip ()) return; @@ -2026,8 +2027,8 @@ test_netns_general (gpointer fixture, gconstpointer test_data) * skip asserts that are known to fail. */ ethtool_support = nmtstp_run_command ("ethtool -i dummy1_ > /dev/null") == 0; if (ethtool_support) { - g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_1, 0, "dummy1_", NM_LINK_TYPE_DUMMY)->ifindex, NULL, NULL, NULL)); - g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_1, 0, "dummy2a", NM_LINK_TYPE_DUMMY)->ifindex, NULL, NULL, NULL)); + g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_1, 0, "dummy1_", NM_LINK_TYPE_DUMMY)->ifindex, &driver_info)); + g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_1, 0, "dummy2a", NM_LINK_TYPE_DUMMY)->ifindex, &driver_info)); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy1_ > /dev/null"), ==, 0); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy2a > /dev/null"), ==, 0); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy2b 2> /dev/null"), !=, 0); @@ -2036,8 +2037,8 @@ test_netns_general (gpointer fixture, gconstpointer test_data) g_assert (nm_platform_netns_push (platform_2, &netns_tmp)); if (ethtool_support) { - g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_2, 0, "dummy1_", NM_LINK_TYPE_DUMMY)->ifindex, NULL, NULL, NULL)); - g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_2, 0, "dummy2b", NM_LINK_TYPE_DUMMY)->ifindex, NULL, NULL, NULL)); + g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_2, 0, "dummy1_", NM_LINK_TYPE_DUMMY)->ifindex, &driver_info)); + g_assert (nmp_utils_ethtool_get_driver_info (nmtstp_link_get_typed (platform_2, 0, "dummy2b", NM_LINK_TYPE_DUMMY)->ifindex, &driver_info)); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy1_ > /dev/null"), ==, 0); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy2a 2> /dev/null"), !=, 0); g_assert_cmpint (nmtstp_run_command ("ethtool -i dummy2b > /dev/null"), ==, 0); From 396d90e744348b9b2c432e9a60eb3f6f245c8c5c Mon Sep 17 00:00:00 2001 From: Thomas Haller Date: Mon, 12 Dec 2016 14:06:44 +0100 Subject: [PATCH 27/27] platform: assume ifname is present in _linktype_get_type() _linktype_get_type() only has one caller, and ifname is *never* NULL. --- src/platform/nm-linux-platform.c | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/platform/nm-linux-platform.c b/src/platform/nm-linux-platform.c index e491a85639..7a7947c6d7 100644 --- a/src/platform/nm-linux-platform.c +++ b/src/platform/nm-linux-platform.c @@ -611,6 +611,7 @@ _linktype_get_type (NMPlatform *platform, guint i; ASSERT_NETNS_CURRENT (platform); + nm_assert (ifname); if (completed_from_cache) { const NMPObject *obj; @@ -631,7 +632,7 @@ _linktype_get_type (NMPlatform *platform, * of messing stuff up. */ if ( obj && !NM_IN_SET (obj->link.type, NM_LINK_TYPE_UNKNOWN, NM_LINK_TYPE_NONE) - && !g_strcmp0 (ifname, obj->link.name) + && nm_streq (ifname, obj->link.name) && ( !kind || !g_strcmp0 (kind, obj->link.kind))) { nm_assert (obj->link.kind == g_intern_string (obj->link.kind)); @@ -652,7 +653,7 @@ _linktype_get_type (NMPlatform *platform, NMPlatformTunProperties props; if ( platform - && nm_platform_link_tun_get_properties (platform, ifindex, ifname ?: "", &props)) { + && nm_platform_link_tun_get_properties (platform, ifindex, ifname, &props)) { if (!g_strcmp0 (props.mode, "tap")) return NM_LINK_TYPE_TAP; if (!g_strcmp0 (props.mode, "tun")) @@ -675,11 +676,8 @@ _linktype_get_type (NMPlatform *platform, else if (arptype == ARPHRD_TUNNEL6) return NM_LINK_TYPE_IP6TNL; - if (ifname) { + { NMPUtilsEthtoolDriverInfo driver_info; - nm_auto_close int dirfd = -1; - gs_free char *devtype = NULL; - char ifname_verified[IFNAMSIZ]; /* Fallback OVS detection for kernel <= 3.16 */ if (nmp_utils_ethtool_get_driver_info (ifindex, &driver_info)) { @@ -694,6 +692,12 @@ _linktype_get_type (NMPlatform *platform, return NM_LINK_TYPE_ETHERNET; } } + } + + { + nm_auto_close int dirfd = -1; + gs_free char *devtype = NULL; + char ifname_verified[IFNAMSIZ]; dirfd = nmp_utils_sysctl_open_netdir (ifindex, ifname, ifname_verified); if (dirfd >= 0) {