]> git.ipfire.org Git - thirdparty/lldpd.git/commitdiff
priv: don't run ethtool as root
authorVincent Bernat <vincent@bernat.im>
Sat, 12 Aug 2017 21:15:56 +0000 (23:15 +0200)
committerVincent Bernat <vincent@bernat.im>
Sat, 12 Aug 2017 21:15:56 +0000 (23:15 +0200)
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.

NEWS
src/daemon/interfaces-linux.c
src/daemon/lldpd.h
src/daemon/priv-linux.c
src/daemon/priv.c

diff --git a/NEWS b/NEWS
index 11764521ea60d433ca85308f9b7186b63ee6c98f..90ed7bddbb27e66022d2b1cbfe126bb240a567c4 100644 (file)
--- 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.
index ea7d6221509774e6b0cc5c7837081f273749304b..9032cad2bcb9851c6370b689c32bc233cdcdcdfd 100644 (file)
@@ -19,6 +19,7 @@
 
 #include <stdio.h>
 #include <unistd.h>
+#include <inttypes.h>
 #include <errno.h>
 #include <sys/ioctl.h>
 #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(&ethc, 0, sizeof(ethc));
+       ethc.cmd = ETHTOOL_GSET;
+       ifr.ifr_data = (caddr_t)&ethc;
+       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);
        }
 }
index 50e005920e575b530c8faf1048b9a55aec776e1a..5d2924f29e66524b2bba1581288f2e80d7d51491 100644 (file)
 #include <netinet/in.h>
 #include <sys/un.h>
 
-#ifdef HOST_OS_LINUX
-# if defined(__clang__)
-#  pragma clang diagnostic push
-#  pragma clang diagnostic ignored "-Wdocumentation"
-# endif
-# include <linux/ethtool.h>
-# if defined(__clang__)
-#  pragma clang diagnostic pop
-# endif
-#endif
-
 #if HAVE_VFORK_H
 # include <vfork.h>
 #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,
index 20263df10984933f8402a9ab0276ae21868548e1..3e4b89ce1cb294bbfa3ecc5e8c028b95bedb03ac 100644 (file)
@@ -31,8 +31,8 @@
 #pragma clang diagnostic ignored "-Wdocumentation"
 #endif
 #include <linux/filter.h>     /* For BPF filtering */
-#include <linux/ethtool.h>
 #include <linux/sockios.h>
+#include <linux/if_ether.h>
 #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(&ethc, 0, sizeof(ethc));
-       ethc.cmd = ETHTOOL_GSET;
-       ifr.ifr_data = (caddr_t)&ethc;
-       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)
 {
index f03c90aa1e30834ff6d03decbd7b940b64df1f23..92c68d870b6df2a98fbbe4e55d2596c57e795b2d 100644 (file)
@@ -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},