From: Vincent Bernat Date: Sat, 12 Aug 2017 21:15:56 +0000 (+0200) Subject: priv: don't run ethtool as root X-Git-Tag: 0.9.8~7^2~3 X-Git-Url: http://git.ipfire.org/?a=commitdiff_plain;h=78c9dfdec6b2129b97b3015b5b007d1fa311ab7b;p=thirdparty%2Flldpd.git priv: don't run ethtool as root Kernels older than 2.6.19 won't get any link information. Kernels 4.6, 4.7 and 4.8 won't get information from GLINKSETTINGS (higher speeds) but they are not LTS kernels so we should be fine. This removes a bunch of code. --- diff --git a/NEWS b/NEWS index 11764521..90ed7bdd 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,8 @@ lldpd (0.9.8) + Use ethtool to get permanent address for bonds and teams. This might provide different results than the previous method. Some devices may still use the previous method. + + Don't run ethtool as root. Kernels older than 2.6.19 won't get + link information anymore. * Fixes: + Handle team interfaces like a bond. Real MAC address cannot be retrieved yet. diff --git a/src/daemon/interfaces-linux.c b/src/daemon/interfaces-linux.c index ea7d6221..9032cad2 100644 --- a/src/daemon/interfaces-linux.c +++ b/src/daemon/interfaces-linux.c @@ -19,6 +19,7 @@ #include #include +#include #include #include #if defined(__clang__) @@ -431,6 +432,19 @@ iflinux_get_permanent_mac(struct lldpd *cfg, iflinux_get_permanent_mac_bond(cfg, interfaces, iface); } +#define ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32 (SCHAR_MAX) +#define ETHTOOL_DECLARE_LINK_MODE_MASK(name) \ + uint32_t name[ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32] + +struct ethtool_link_usettings { + struct ethtool_link_settings base; + struct { + ETHTOOL_DECLARE_LINK_MODE_MASK(supported); + ETHTOOL_DECLARE_LINK_MODE_MASK(advertising); + ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising); + } link_modes; +}; + static inline int iflinux_ethtool_link_mode_test_bit(unsigned int nr, const uint32_t *mask) { @@ -458,10 +472,84 @@ iflinux_ethtool_link_mode_is_empty(const uint32_t *mask) return 1; } +static int +iflinux_ethtool_glink(struct lldpd *cfg, const char *ifname, struct ethtool_link_usettings *uset) { + int rc; + + /* Try with ETHTOOL_GLINKSETTINGS first */ + struct { + struct ethtool_link_settings req; + uint32_t link_mode_data[3 * ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32]; + } ecmd; + static int8_t nwords = 0; + struct ifreq ifr = {}; + strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)); + + if (nwords == 0) { + /* Do a handshake first. We assume that this is device-independant. */ + memset(&ecmd, 0, sizeof(ecmd)); + ecmd.req.cmd = ETHTOOL_GLINKSETTINGS; + ifr.ifr_data = (caddr_t)&ecmd; + rc = ioctl(cfg->g_sock, SIOCETHTOOL, &ifr); + if (rc == 0) { + nwords = -ecmd.req.link_mode_masks_nwords; + log_debug("interfaces", "glinksettings nwords is %" PRId8, nwords); + } + } + if (nwords > 0) { + memset(&ecmd, 0, sizeof(ecmd)); + ecmd.req.cmd = ETHTOOL_GLINKSETTINGS; + ecmd.req.link_mode_masks_nwords = nwords; + ifr.ifr_data = (caddr_t)&ecmd; + rc = ioctl(cfg->g_sock, SIOCETHTOOL, &ifr); + if (rc == 0) { + log_debug("interfaces", "got ethtool results for %s with GLINKSETTINGS", + ifname); + memcpy(&uset->base, &ecmd.req, sizeof(uset->base)); + unsigned int u32_offs = 0; + memcpy(uset->link_modes.supported, + &ecmd.link_mode_data[u32_offs], + 4 * ecmd.req.link_mode_masks_nwords); + u32_offs += ecmd.req.link_mode_masks_nwords; + memcpy(uset->link_modes.advertising, + &ecmd.link_mode_data[u32_offs], + 4 * ecmd.req.link_mode_masks_nwords); + u32_offs += ecmd.req.link_mode_masks_nwords; + memcpy(uset->link_modes.lp_advertising, + &ecmd.link_mode_data[u32_offs], + 4 * ecmd.req.link_mode_masks_nwords); + goto end; + } + } + + /* Try with ETHTOOL_GSET */ + struct ethtool_cmd ethc; + memset(ðc, 0, sizeof(ethc)); + ethc.cmd = ETHTOOL_GSET; + ifr.ifr_data = (caddr_t)ðc; + rc = ioctl(cfg->g_sock, SIOCETHTOOL, &ifr); + if (rc == 0) { + /* Do a partial copy (only what we need) */ + log_debug("interfaces", "got ethtool results for %s with GSET", + ifname); + memset(uset, 0, sizeof(*uset)); + uset->base.cmd = ETHTOOL_GSET; + uset->base.link_mode_masks_nwords = 1; + uset->link_modes.supported[0] = ethc.supported; + uset->link_modes.advertising[0] = ethc.advertising; + uset->link_modes.lp_advertising[0] = ethc.lp_advertising; + uset->base.speed = (ethc.speed_hi << 16) | ethc.speed; + uset->base.duplex = ethc.duplex; + uset->base.port = ethc.port; + uset->base.autoneg = ethc.autoneg; + } +end: + return rc; +} /* Fill up MAC/PHY for a given hardware port */ static void -iflinux_macphy(struct lldpd_hardware *hardware) +iflinux_macphy(struct lldpd *cfg, struct lldpd_hardware *hardware) { #ifdef ENABLE_DOT3 struct ethtool_link_usettings uset; @@ -481,7 +569,7 @@ iflinux_macphy(struct lldpd_hardware *hardware) log_debug("interfaces", "ask ethtool for the appropriate MAC/PHY for %s", hardware->h_ifname); - if (priv_ethtool(hardware->h_ifname, &uset) == 0) { + if (iflinux_ethtool_glink(cfg, hardware->h_ifname, &uset) == 0) { port->p_macphy.autoneg_support = iflinux_ethtool_link_mode_test_bit( ETHTOOL_LINK_MODE_Autoneg_BIT, uset.link_modes.supported); port->p_macphy.autoneg_enabled = (uset.base.autoneg == AUTONEG_DISABLE) ? 0 : 1; @@ -932,7 +1020,7 @@ interfaces_update(struct lldpd *cfg) /* Mac/PHY */ TAILQ_FOREACH(hardware, &cfg->g_hardware, h_entries) { if (!hardware->h_flags) continue; - iflinux_macphy(hardware); + iflinux_macphy(cfg, hardware); interfaces_helper_promisc(cfg, hardware); } } diff --git a/src/daemon/lldpd.h b/src/daemon/lldpd.h index 50e00592..5d2924f2 100644 --- a/src/daemon/lldpd.h +++ b/src/daemon/lldpd.h @@ -37,17 +37,6 @@ #include #include -#ifdef HOST_OS_LINUX -# if defined(__clang__) -# pragma clang diagnostic push -# pragma clang diagnostic ignored "-Wdocumentation" -# endif -# include -# if defined(__clang__) -# pragma clang diagnostic pop -# endif -#endif - #if HAVE_VFORK_H # include #endif @@ -212,22 +201,8 @@ void priv_wait(void); void priv_ctl_cleanup(const char *ctlname); char *priv_gethostname(void); #ifdef HOST_OS_LINUX -#define ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32 (SCHAR_MAX) -#define ETHTOOL_DECLARE_LINK_MODE_MASK(name) \ - uint32_t name[ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32] - -struct ethtool_link_usettings { - struct ethtool_link_settings base; - struct { - ETHTOOL_DECLARE_LINK_MODE_MASK(supported); - ETHTOOL_DECLARE_LINK_MODE_MASK(advertising); - ETHTOOL_DECLARE_LINK_MODE_MASK(lp_advertising); - } link_modes; -}; int priv_open(char*); void asroot_open(void); -int priv_ethtool(char*, struct ethtool_link_usettings*); -void asroot_ethtool(void); #endif int priv_iface_init(int, char *); int asroot_iface_init_os(int, char *, int *); @@ -243,7 +218,6 @@ enum priv_cmd { PRIV_DELETE_CTL_SOCKET, PRIV_GET_HOSTNAME, PRIV_OPEN, - PRIV_ETHTOOL, PRIV_IFACE_INIT, PRIV_IFACE_MULTICAST, PRIV_IFACE_DESCRIPTION, diff --git a/src/daemon/priv-linux.c b/src/daemon/priv-linux.c index 20263df1..3e4b89ce 100644 --- a/src/daemon/priv-linux.c +++ b/src/daemon/priv-linux.c @@ -31,8 +31,8 @@ #pragma clang diagnostic ignored "-Wdocumentation" #endif #include /* For BPF filtering */ -#include #include +#include #if defined(__clang__) #pragma clang diagnostic pop #endif @@ -54,106 +54,6 @@ priv_open(char *file) return receive_fd(PRIV_UNPRIVILEGED); } -static int -asroot_ethtool_real(const char *ifname, struct ethtool_link_usettings *uset) { - int rc, sock = -1; - struct ifreq ifr = {}; - if ((rc = sock = socket(AF_INET, SOCK_DGRAM, 0)) == -1) { - return rc; - } - strlcpy(ifr.ifr_name, ifname, sizeof(ifr.ifr_name)); - - /* Try with ETHTOOL_GLINKSETTINGS first */ - struct { - struct ethtool_link_settings req; - uint32_t link_mode_data[3 * ETHTOOL_LINK_MODE_MASK_MAX_KERNEL_NU32]; - } ecmd; - static int8_t nwords = 0; - - if (nwords == 0) { - /* Do a handshake first. We assume that this is device-independant. */ - memset(&ecmd, 0, sizeof(ecmd)); - ecmd.req.cmd = ETHTOOL_GLINKSETTINGS; - ifr.ifr_data = (caddr_t)&ecmd; - rc = ioctl(sock, SIOCETHTOOL, &ifr); - if (rc == 0) { - nwords = -ecmd.req.link_mode_masks_nwords; - log_debug("privsep", "glinksettings nwords is %" PRId8, nwords); - } - } - if (nwords > 0) { - memset(&ecmd, 0, sizeof(ecmd)); - ecmd.req.cmd = ETHTOOL_GLINKSETTINGS; - ecmd.req.link_mode_masks_nwords = nwords; - ifr.ifr_data = (caddr_t)&ecmd; - rc = ioctl(sock, SIOCETHTOOL, &ifr); - if (rc == 0) { - log_debug("privsep", "got ethtool results for %s with GLINKSETTINGS", - ifname); - memcpy(&uset->base, &ecmd.req, sizeof(uset->base)); - unsigned int u32_offs = 0; - memcpy(uset->link_modes.supported, - &ecmd.link_mode_data[u32_offs], - 4 * ecmd.req.link_mode_masks_nwords); - u32_offs += ecmd.req.link_mode_masks_nwords; - memcpy(uset->link_modes.advertising, - &ecmd.link_mode_data[u32_offs], - 4 * ecmd.req.link_mode_masks_nwords); - u32_offs += ecmd.req.link_mode_masks_nwords; - memcpy(uset->link_modes.lp_advertising, - &ecmd.link_mode_data[u32_offs], - 4 * ecmd.req.link_mode_masks_nwords); - goto end; - } - } - - /* Try with ETHTOOL_GSET */ - struct ethtool_cmd ethc; - memset(ðc, 0, sizeof(ethc)); - ethc.cmd = ETHTOOL_GSET; - ifr.ifr_data = (caddr_t)ðc; - rc = ioctl(sock, SIOCETHTOOL, &ifr); - if (rc == 0) { - /* Do a partial copy (only what we need) */ - log_debug("privsep", "got ethtool results for %s with GSET", - ifname); - memset(uset, 0, sizeof(*uset)); - uset->base.cmd = ETHTOOL_GSET; - uset->base.link_mode_masks_nwords = 1; - uset->link_modes.supported[0] = ethc.supported; - uset->link_modes.advertising[0] = ethc.advertising; - uset->link_modes.lp_advertising[0] = ethc.lp_advertising; - uset->base.speed = (ethc.speed_hi << 16) | ethc.speed; - uset->base.duplex = ethc.duplex; - uset->base.port = ethc.port; - uset->base.autoneg = ethc.autoneg; - } -end: - close(sock); - return rc; -} - -/* Proxy for ethtool ioctl (GSET/GLINKSETTINGS only). Not needed since - * 0fdc100bdc4b7ab61ed632962c76dfe539047296 (2.6.37). But needed until - * 8006f6bf5e39f11c697f48df20382b81d2f2f8b8 (4.9). */ -int -priv_ethtool(char *ifname, struct ethtool_link_usettings *uset) -{ - int rc; - int len; - enum priv_cmd cmd = PRIV_ETHTOOL; - must_write(PRIV_UNPRIVILEGED, &cmd, sizeof(enum priv_cmd)); - len = strlen(ifname); - must_write(PRIV_UNPRIVILEGED, &len, sizeof(int)); - must_write(PRIV_UNPRIVILEGED, ifname, len); - priv_wait(); - must_read(PRIV_UNPRIVILEGED, &rc, sizeof(int)); - if (rc != 0) - return rc; - must_read(PRIV_UNPRIVILEGED, uset, sizeof(struct ethtool_link_usettings)); - return rc; -} - void asroot_open() { @@ -214,25 +114,6 @@ asroot_open() close(fd); } -void -asroot_ethtool() -{ - struct ethtool_link_usettings uset; - int len, rc; - char *ifname; - - must_read(PRIV_PRIVILEGED, &len, sizeof(int)); - if ((ifname = (char*)malloc(len + 1)) == NULL) - fatal("privsep", NULL); - must_read(PRIV_PRIVILEGED, ifname, len); - ifname[len] = '\0'; - rc = asroot_ethtool_real(ifname, &uset); - free(ifname); - must_write(PRIV_PRIVILEGED, &rc, sizeof(int)); - if (rc == -1) return; - must_write(PRIV_PRIVILEGED, &uset, sizeof(struct ethtool_link_usettings)); -} - int asroot_iface_init_os(int ifindex, char *name, int *fd) { diff --git a/src/daemon/priv.c b/src/daemon/priv.c index f03c90aa..92c68d87 100644 --- a/src/daemon/priv.c +++ b/src/daemon/priv.c @@ -387,7 +387,6 @@ static struct dispatch_actions actions[] = { {PRIV_GET_HOSTNAME, asroot_gethostname}, #ifdef HOST_OS_LINUX {PRIV_OPEN, asroot_open}, - {PRIV_ETHTOOL, asroot_ethtool}, #endif {PRIV_IFACE_INIT, asroot_iface_init}, {PRIV_IFACE_MULTICAST, asroot_iface_multicast},