router: improve error checking

Improve error checking fixing resource leak detected by Coverity in CID
1430880.
Further fix unchecked return value reported by Coverity in CIDs 1430872,
1430839, 1430831 and 1412382

Signed-off-by: Hans Dedecker <dedeckeh@gmail.com>

diff --git a/src/odhcpd.c b/src/odhcpd.c
index 8aa4571ea642dc15409f00f235ad8b16900e92c2..2c6ae4631cf9e1895e0be13396ce64b31794f01e 100644 (file)
--- a/src/odhcpd.c
+++ b/src/odhcpd.c
@@ -200,20 +200,27 @@ ssize_t odhcpd_send(int socket, struct sockaddr_in6 *dest,
 
 static int odhcpd_get_linklocal_interface_address(int ifindex, struct in6_addr *lladdr)
 {
-       int status = -1;
-       struct sockaddr_in6 addr = {AF_INET6, 0, 0, ALL_IPV6_ROUTERS, ifindex};
+       int ret = -1;
+       struct sockaddr_in6 addr;
        socklen_t alen = sizeof(addr);
        int sock = socket(AF_INET6, SOCK_RAW, IPPROTO_ICMPV6);
 
+       if (sock < 0)
+               return -1;
+
+       memset(&addr, 0, sizeof(addr));
+       addr.sin6_family = AF_INET6;
+       inet_pton(AF_INET6, ALL_IPV6_ROUTERS, &addr.sin6_addr);
+       addr.sin6_scope_id = ifindex;
+
        if (!connect(sock, (struct sockaddr*)&addr, sizeof(addr)) &&
                        !getsockname(sock, (struct sockaddr*)&addr, &alen)) {
                *lladdr = addr.sin6_addr;
-               status = 0;
+               ret = 0;
        }
 
        close(sock);
-
-       return status;
+       return ret;
 }
 
 /*

diff --git a/src/odhcpd.h b/src/odhcpd.h
index 9a27708a2c408c33dbf277dc8ef5a968b49ee80f..91fdcbfe27ead984cebd2c69e3dbd44a7ec2a06c 100644 (file)
--- a/src/odhcpd.h
+++ b/src/odhcpd.h
@@ -45,12 +45,8 @@
 #define _unused __attribute__((unused))
 #define _packed __attribute__((packed))
 
-
-#define ALL_IPV6_NODES {{{0xff, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\
-               0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x01}}}
-
-#define ALL_IPV6_ROUTERS {{{0xff, 0x02, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00,\
-               0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02}}}
+#define ALL_IPV6_NODES "ff02::1"
+#define ALL_IPV6_ROUTERS "ff02::2"
 
 #define IN6_IS_ADDR_ULA(a) (((a)->s6_addr32[0] & htonl(0xfe000000)) == htonl(0xfc000000))
 

diff --git a/src/router.c b/src/router.c
index 79b688a556e1d662e6f00269388010f7bf328497..e52fffd9f61f1c4c6691b24f27f8a13a53376ad4 100644 (file)
--- a/src/router.c
+++ b/src/router.c
@@ -45,89 +45,154 @@ static FILE *fp_route = NULL;
 
 int router_init(void)
 {
+       struct icmp6_filter filt;
+       int ret = 0;
+
        // Open ICMPv6 socket
-       int sock = socket(AF_INET6, SOCK_RAW | SOCK_CLOEXEC, IPPROTO_ICMPV6);
-       if (sock < 0 && errno != EAFNOSUPPORT) {
-               syslog(LOG_ERR, "Failed to open RAW-socket: %m");
-               return -1;
+       router_event.uloop.fd = socket(AF_INET6, SOCK_RAW | SOCK_CLOEXEC, IPPROTO_ICMPV6);
+       if (router_event.uloop.fd < 0) {
+               syslog(LOG_ERR, "socket(AF_INET6): %m");
+               ret = -1;
+               goto out;
        }
 
        // Let the kernel compute our checksums
        int val = 2;
-       setsockopt(sock, IPPROTO_RAW, IPV6_CHECKSUM, &val, sizeof(val));
+       if (setsockopt(router_event.uloop.fd, IPPROTO_RAW, IPV6_CHECKSUM,
+                               &val, sizeof(val)) < 0) {
+               syslog(LOG_ERR, "setsockopt(IPV6_CHECKSUM): %m");
+               ret = -1;
+               goto out;
+       }
 
        // This is required by RFC 4861
        val = 255;
-       setsockopt(sock, IPPROTO_IPV6, IPV6_MULTICAST_HOPS, &val, sizeof(val));
-       setsockopt(sock, IPPROTO_IPV6, IPV6_UNICAST_HOPS, &val, sizeof(val));
+       if (setsockopt(router_event.uloop.fd, IPPROTO_IPV6, IPV6_MULTICAST_HOPS,
+                               &val, sizeof(val)) < 0) {
+               syslog(LOG_ERR, "setsockopt(IPV6_MULTICAST_HOPS): %m");
+               ret = -1;
+               goto out;
+       }
+
+       if (setsockopt(router_event.uloop.fd, IPPROTO_IPV6, IPV6_UNICAST_HOPS,
+                               &val, sizeof(val)) < 0) {
+               syslog(LOG_ERR, "setsockopt(IPV6_UNICAST_HOPS): %m");
+               ret = -1;
+               goto out;
+       }
 
        // We need to know the source interface
        val = 1;
-       setsockopt(sock, IPPROTO_IPV6, IPV6_RECVPKTINFO, &val, sizeof(val));
-       setsockopt(sock, IPPROTO_IPV6, IPV6_RECVHOPLIMIT, &val, sizeof(val));
+       if (setsockopt(router_event.uloop.fd, IPPROTO_IPV6, IPV6_RECVPKTINFO,
+                               &val, sizeof(val)) < 0) {
+               syslog(LOG_ERR, "setsockopt(IPV6_RECVPKTINFO): %m");
+               ret = -1;
+               goto out;
+       }
+
+       if (setsockopt(router_event.uloop.fd, IPPROTO_IPV6, IPV6_RECVHOPLIMIT,
+                               &val, sizeof(val)) < 0) {
+               syslog(LOG_ERR, "setsockopt(IPV6_RECVHOPLIMIT): %m");
+               ret = -1;
+               goto out;
+       }
 
        // Don't loop back
        val = 0;
-       setsockopt(sock, IPPROTO_IPV6, IPV6_MULTICAST_LOOP, &val, sizeof(val));
+       if (setsockopt(router_event.uloop.fd, IPPROTO_IPV6, IPV6_MULTICAST_LOOP,
+                               &val, sizeof(val)) < 0) {
+               syslog(LOG_ERR, "setsockopt(IPV6_MULTICAST_LOOP): %m");
+               ret = -1;
+               goto out;
+       }
 
        // Filter ICMPv6 package types
-       struct icmp6_filter filt;
        ICMP6_FILTER_SETBLOCKALL(&filt);
        ICMP6_FILTER_SETPASS(ND_ROUTER_ADVERT, &filt);
        ICMP6_FILTER_SETPASS(ND_ROUTER_SOLICIT, &filt);
-       setsockopt(sock, IPPROTO_ICMPV6, ICMP6_FILTER, &filt, sizeof(filt));
+       if (setsockopt(router_event.uloop.fd, IPPROTO_ICMPV6, ICMP6_FILTER,
+                               &filt, sizeof(filt)) < 0) {
+               syslog(LOG_ERR, "setsockopt(ICMP6_FILTER): %m");
+               ret = -1;
+               goto out;
+       }
+
+       if (!(fp_route = fopen("/proc/net/ipv6_route", "r"))) {
+               syslog(LOG_ERR, "fopen(/proc/net/ipv6_route): %m");
+               ret = -1;
+               goto out;
+       }
+
+       if (netlink_add_netevent_handler(&router_netevent_handler) < 0) {
+               syslog(LOG_ERR, "Failed to add netevent handler");
+               ret = -1;
+               goto out;
+       }
 
        // Register socket
-       router_event.uloop.fd = sock;
        odhcpd_register(&router_event);
+out:
+       if (ret < 0) {
+               if (router_event.uloop.fd > 0) {
+                       close(router_event.uloop.fd);
+                       router_event.uloop.fd = -1;
+               }
 
-       if (!(fp_route = fopen("/proc/net/ipv6_route", "r")))
-               syslog(LOG_ERR, "Failed to open routing table: %m");
-
-       netlink_add_netevent_handler(&router_netevent_handler);
+               if (fp_route) {
+                       fclose(fp_route);
+                       fp_route = NULL;
+               }
+       }
 
-       return 0;
+       return ret;
 }
 
 
 int router_setup_interface(struct interface *iface, bool enable)
 {
-       if (!fp_route || router_event.uloop.fd < 0)
-               return -1;
+       struct ipv6_mreq mreq;
+       int ret = 0;
 
-       struct ipv6_mreq all_nodes = {ALL_IPV6_NODES, iface->ifindex};
-       struct ipv6_mreq all_routers = {ALL_IPV6_ROUTERS, iface->ifindex};
+       if (!fp_route || router_event.uloop.fd < 0) {
+               ret = -1;
+               goto out;
+       }
 
        uloop_timeout_cancel(&iface->timer_rs);
        iface->timer_rs.cb = NULL;
 
-       if (iface->ifindex <= 0)
-               return -1;
-
+       memset(&mreq, 0, sizeof(mreq));
+       mreq.ipv6mr_interface = iface->ifindex;
+       inet_pton(AF_INET6, ALL_IPV6_NODES, &mreq.ipv6mr_multiaddr);
        setsockopt(router_event.uloop.fd, IPPROTO_IPV6, IPV6_DROP_MEMBERSHIP,
-                       &all_nodes, sizeof(all_nodes));
+                       &mreq, sizeof(mreq));
+
+       inet_pton(AF_INET6, ALL_IPV6_ROUTERS, &mreq.ipv6mr_multiaddr);
        setsockopt(router_event.uloop.fd, IPPROTO_IPV6, IPV6_DROP_MEMBERSHIP,
-                       &all_routers, sizeof(all_routers));
+                       &mreq, sizeof(mreq));
 
        if (!enable) {
                if (iface->ra)
                        trigger_router_advert(&iface->timer_rs);
        } else {
-               void *mreq = &all_routers;
-
                if (iface->ra == MODE_RELAY && iface->master) {
-                       mreq = &all_nodes;
+                       inet_pton(AF_INET6, ALL_IPV6_NODES, &mreq.ipv6mr_multiaddr);
                        forward_router_solicitation(iface);
                } else if (iface->ra == MODE_SERVER && !iface->master) {
                        iface->timer_rs.cb = trigger_router_advert;
                        uloop_timeout_set(&iface->timer_rs, 1000);
                }
 
-               if (iface->ra == MODE_RELAY || (iface->ra == MODE_SERVER && !iface->master))
-                       setsockopt(router_event.uloop.fd, IPPROTO_IPV6,
-                                       IPV6_ADD_MEMBERSHIP, mreq, sizeof(all_nodes));
+               if (iface->ra == MODE_RELAY || (iface->ra == MODE_SERVER && !iface->master)) {
+                       if (setsockopt(router_event.uloop.fd, IPPROTO_IPV6,
+                                       IPV6_ADD_MEMBERSHIP, &mreq, sizeof(mreq)) < 0) {
+                               ret = -1;
+                               syslog(LOG_ERR, "setsockopt(IPV6_ADD_MEMBERSHIP): %m");
+                       }
+               }
        }
-       return 0;
+out:
+       return ret;
 }
 
 
@@ -573,10 +638,15 @@ static uint64_t send_router_advert(struct interface *iface, const struct in6_add
                        [IOV_RA_DNS_ADDR] = {dns_addr, dns_cnt * sizeof(*dns_addr)},
                        [IOV_RA_SEARCH] = {search, search->len * 8},
                        [IOV_RA_ADV_INTERVAL] = {&adv_interval, adv_interval.len * 8}};
-       struct sockaddr_in6 dest = {AF_INET6, 0, 0, ALL_IPV6_NODES, 0};
+       struct sockaddr_in6 dest;
+
+       memset(&dest, 0, sizeof(dest));
+       dest.sin6_family = AF_INET6;
 
        if (from && !IN6_IS_ADDR_UNSPECIFIED(from))
                dest.sin6_addr = *from;
+       else
+               inet_pton(AF_INET6, ALL_IPV6_NODES, &dest.sin6_addr);
 
        odhcpd_send(router_event.uloop.fd,
                        &dest, iov, ARRAY_SIZE(iov), iface);
@@ -624,13 +694,17 @@ static void handle_icmpv6(void *addr, void *data, size_t len,
 // Forward router solicitation
 static void forward_router_solicitation(const struct interface *iface)
 {
+       struct icmp6_hdr rs = {ND_ROUTER_SOLICIT, 0, 0, {{0}}};
+       struct iovec iov = {&rs, sizeof(rs)};
+       struct sockaddr_in6 all_routers;
+
        if (!iface)
                return;
 
-       struct icmp6_hdr rs = {ND_ROUTER_SOLICIT, 0, 0, {{0}}};
-       struct iovec iov = {&rs, sizeof(rs)};
-       struct sockaddr_in6 all_routers =
-               {AF_INET6, 0, 0, ALL_IPV6_ROUTERS, iface->ifindex};
+       memset(&all_routers, 0, sizeof(all_routers));
+       all_routers.sin6_family = AF_INET6;
+       inet_pton(AF_INET6, ALL_IPV6_ROUTERS, &all_routers.sin6_addr);
+       all_routers.sin6_scope_id = iface->ifindex;
 
        syslog(LOG_NOTICE, "Sending RS to %s", iface->ifname);
        odhcpd_send(router_event.uloop.fd, &all_routers, &iov, 1, iface);
@@ -641,6 +715,7 @@ static void forward_router_solicitation(const struct interface *iface)
 static void forward_router_advertisement(uint8_t *data, size_t len)
 {
        struct nd_router_advert *adv = (struct nd_router_advert *)data;
+       struct sockaddr_in6 all_nodes;
 
        // Rewrite options
        uint8_t *end = data + len;
@@ -666,7 +741,10 @@ static void forward_router_advertisement(uint8_t *data, size_t len)
        adv->nd_ra_flags_reserved |= ND_RA_FLAG_PROXY;
 
        // Forward advertisement to all slave interfaces
-       struct sockaddr_in6 all_nodes = {AF_INET6, 0, 0, ALL_IPV6_NODES, 0};
+       memset(&all_nodes, 0, sizeof(all_nodes));
+       all_nodes.sin6_family = AF_INET6;
+       inet_pton(AF_INET6, ALL_IPV6_NODES, &all_nodes.sin6_addr);
+
        struct iovec iov = {data, len};
 
        struct interface *iface;