If ifchd commands fail, propagate the failure back to ndhc.

src/ifchd-parse.rl

@@ -76,7 +76,7 @@
 
 %% write data;
 
-static void perform_ip4set(const char buf[static 1], size_t len)
+static int perform_ip4set(const char buf[static 1], size_t len)
 {
     char ip4_addr[INET_ADDRSTRLEN];
     char ip4_subnet[INET_ADDRSTRLEN];
@@ -96,21 +96,21 @@ static void perform_ip4set(const char buf[static 1], size_t len)
 
     if (cs < ipv4set_parser_first_final) {
         log_line("%s: received invalid arguments", __func__);
-        return;
+        return -1;
     }
 
     // These should never trigger because of the above check, but be safe...
     if (!have_ip) {
         log_line("%s: No IPv4 address specified.", __func__);
-        return;
+        return -1;
     }
     if (!have_subnet) {
         log_line("%s: No IPv4 subnet specified.", __func__);
-        return;
+        return -1;
     }
 
-    perform_ip_subnet_bcast(ip4_addr, ip4_subnet,
+    return perform_ip_subnet_bcast(ip4_addr, ip4_subnet,
-                            have_bcast ? ip4_bcast : NULL);
+                                   have_bcast ? ip4_bcast : NULL);
 }
 
 %%{
@@ -122,7 +122,7 @@ static void perform_ip4set(const char buf[static 1], size_t len)
         arg_len = p - arg_start;
         if (arg_len > sizeof tb - 1) {
             log_line("command argument would overflow");
-            return -1;
+            return -99;
         }
         memcpy(tb, arg_start, arg_len);
         tb[arg_len] = 0;
@@ -130,20 +130,20 @@ static void perform_ip4set(const char buf[static 1], size_t len)
 
     action Dispatch {
         switch (cl.state) {
-        case STATE_IP4SET: perform_ip4set(tb, arg_len); break;
+        case STATE_IP4SET: cmdf |= perform_ip4set(tb, arg_len); break;
-        case STATE_TIMEZONE: perform_timezone( tb, arg_len); break;
+        case STATE_TIMEZONE: cmdf |= perform_timezone( tb, arg_len); break;
-        case STATE_ROUTER: perform_router(tb, arg_len); break;
+        case STATE_ROUTER: cmdf |= perform_router(tb, arg_len); break;
-        case STATE_DNS: perform_dns(tb, arg_len); break;
+        case STATE_DNS: cmdf |= perform_dns(tb, arg_len); break;
-        case STATE_LPRSVR: perform_lprsvr(tb, arg_len); break;
+        case STATE_LPRSVR: cmdf |= perform_lprsvr(tb, arg_len); break;
-        case STATE_HOSTNAME: perform_hostname(tb, arg_len); break;
+        case STATE_HOSTNAME: cmdf |= perform_hostname(tb, arg_len); break;
-        case STATE_DOMAIN: perform_domain(tb, arg_len); break;
+        case STATE_DOMAIN: cmdf |= perform_domain(tb, arg_len); break;
-        case STATE_IPTTL: perform_ipttl(tb, arg_len); break;
+        case STATE_IPTTL: cmdf |= perform_ipttl(tb, arg_len); break;
-        case STATE_MTU: perform_mtu(tb, arg_len); break;
+        case STATE_MTU: cmdf |= perform_mtu(tb, arg_len); break;
-        case STATE_NTPSVR: perform_ntpsrv(tb, arg_len); break;
+        case STATE_NTPSVR: cmdf |= perform_ntpsrv(tb, arg_len); break;
-        case STATE_WINS: perform_wins(tb, arg_len); break;
+        case STATE_WINS: cmdf |= perform_wins(tb, arg_len); break;
         default:
             log_line("error: invalid state in dispatch_work");
-            return -1;
+            return -99;
         }
     }
 
@@ -178,23 +178,26 @@ static void perform_ip4set(const char buf[static 1], size_t len)
 %% write data;
 
 /*
- * Returns -1 on fatal error; that leads to peer connection being closed.
+ * Returns -99 on fatal error; that leads to peer connection being closed.
+ * Returns -1 if one of the commands failed.
+ * Returns 0 on success.
  */
 int execute_buffer(const char newbuf[static 1])
 {
     char buf[MAX_BUF * 2];
     char tb[MAX_BUF];
+    int cmdf = 0;
 
     ssize_t buflen = snprintf(buf, sizeof buf, "%s%s", cl.ibuf, newbuf);
     if (buflen < 0) {
         log_error("%s: (%s) snprintf1 failed; your system is broken?",
                   client_config.interface, __func__);
-        return -1;
+        return -99;
     }
     if ((size_t)buflen >= sizeof buf) {
         log_error("%s: (%s) input is too long for buffer",
                   client_config.interface, __func__);
-        return -1;
+        return -99;
     }
 
     size_t init_siz = strlen(buf);
@@ -214,22 +217,22 @@ int execute_buffer(const char newbuf[static 1])
         if (ilen < 0) {
             log_error("%s: (%s) snprintf2 failed; your system is broken?",
                       client_config.interface, __func__);
-            return -1;
+            return -99;
         }
         if ((size_t)ilen >= sizeof buf) {
             log_error("%s: (%s) unconsumed input too long for buffer",
                       client_config.interface, __func__);
-            return -1;
+            return -99;
         }
     }
 
     if (cs < ifchd_parser_first_final) {
         log_error("%s: ifch received invalid commands",
                   client_config.interface);
-        return -1;
+        return -99;
     }
     log_line("%s: Commands received and successfully executed.",
              client_config.interface);
-    return 0;
+    return !cmdf ? 0 : -1;
 }
 

src/ifchd.c

@@ -76,22 +76,21 @@ static void writeordie(int fd, const char buf[static 1], size_t len)
 }
 
 /* Writes a new resolv.conf based on the information we have received. */
-static void write_resolve_conf(void)
+static int write_resolve_conf(void)
 {
     static const char ns_str[] = "nameserver ";
     static const char dom_str[] = "domain ";
     static const char srch_str[] = "search ";
-    int r;
     off_t off;
     char buf[MAX_BUF];
 
     if (resolv_conf_fd < 0)
-        return;
+        return 0;
     if (strlen(cl.namesvrs) == 0)
-        return;
+        return -1;
 
     if (lseek(resolv_conf_fd, 0, SEEK_SET) < 0)
-        return;
+        return -1;
 
     char *p = cl.namesvrs;
     while (p && (*p != '\0')) {
@@ -152,106 +151,118 @@ static void write_resolve_conf(void)
     if (off < 0) {
         log_line("write_resolve_conf: lseek returned error: %s",
                 strerror(errno));
-        return;
+        return -1;
     }
   retry:
-    r = ftruncate(resolv_conf_fd, off);
+    if (ftruncate(resolv_conf_fd, off) < 0) {
-    if (r < 0) {
         if (errno == EINTR)
             goto retry;
         log_line("write_resolve_conf: ftruncate returned error: %s",
                  strerror(errno));
-        return;
+        return -1;
     }
-    r = fsync(resolv_conf_fd);
+    if (fsync(resolv_conf_fd) < 0) {
-    if (r < 0) {
         log_line("write_resolve_conf: fsync returned error: %s",
                  strerror(errno));
-        return;
+        return -1;
     }
+    return 0;
 }
 
 /* XXX: addme */
-void perform_timezone(const char str[static 1], size_t len)
+int perform_timezone(const char str[static 1], size_t len)
 {
     (void)len;
     log_line("Timezone setting NYI: '%s'", str);
+    return 0;
 }
 
 /* Add a dns server to the /etc/resolv.conf -- we already have a fd. */
-void perform_dns(const char str[static 1], size_t len)
+int perform_dns(const char str[static 1], size_t len)
 {
     if (resolv_conf_fd < 0)
-        return;
+        return 0;
+    int ret = -1;
     if (len > sizeof cl.namesvrs) {
         log_line("DNS server list is too long: %zu > %zu", len, cl.namesvrs);
-        return;
+        return ret;
     }
     ssize_t sl = snprintf(cl.namesvrs, sizeof cl.namesvrs, "%s", str);
     if (sl < 0 || (size_t)sl >= sizeof cl.namesvrs) {
         log_warning("%s: (%s) snprintf failed",
                     client_config.interface, __func__);
     }
-    write_resolve_conf();
+    ret = write_resolve_conf();
-    log_line("Added DNS server: '%s'", str);
+    if (ret >= 0)
+        log_line("Added DNS server: '%s'", str);
+    return ret;
 }
 
 /* Updates for print daemons are too non-standard to be useful. */
-void perform_lprsvr(const char str[static 1], size_t len)
+int perform_lprsvr(const char str[static 1], size_t len)
 {
     (void)len;
     log_line("Line printer server setting NYI: '%s'", str);
+    return 0;
 }
 
 /* Sets machine hostname. */
-void perform_hostname(const char str[static 1], size_t len)
+int perform_hostname(const char str[static 1], size_t len)
 {
     if (!allow_hostname)
-        return;
+        return 0;
-    if (sethostname(str, len) < 0)
+    if (sethostname(str, len) < 0) {
         log_line("sethostname returned %s", strerror(errno));
-    else
+        return -1;
-        log_line("Set hostname: '%s'", str);
+    }
+    log_line("Set hostname: '%s'", str);
+    return 0;
 }
 
 /* update "domain" and "search" in /etc/resolv.conf */
-void perform_domain(const char str[static 1], size_t len)
+int perform_domain(const char str[static 1], size_t len)
 {
     if (resolv_conf_fd < 0)
-        return;
+        return 0;
+    int ret = -1;
     if (len > sizeof cl.domains) {
         log_line("DNS domain list is too long: %zu > %zu", len, cl.namesvrs);
-        return;
+        return ret;
     }
     ssize_t sl = snprintf(cl.domains, sizeof cl.domains, "%s", str);
     if (sl < 0 || (size_t)sl >= sizeof cl.domains) {
         log_warning("%s: (%s) snprintf failed",
                     client_config.interface, __func__);
     }
-    write_resolve_conf();
+    ret = write_resolve_conf();
-    log_line("Added DNS domain: '%s'", str);
+    if (ret <= 0)
+        log_line("Added DNS domain: '%s'", str);
+    return ret;
 }
 
 /* I don't think this can be done without a netfilter extension
  * that isn't in the mainline kernels. */
-void perform_ipttl(const char str[static 1], size_t len)
+int perform_ipttl(const char str[static 1], size_t len)
 {
     (void)len;
     log_line("TTL setting NYI: '%s'", str);
+    return 0;
 }
 
 /* XXX: addme */
-void perform_ntpsrv(const char str[static 1], size_t len)
+int perform_ntpsrv(const char str[static 1], size_t len)
 {
     (void)len;
     log_line("NTP server setting NYI: '%s'", str);
+    return 0;
 }
 
 /* Maybe Samba cares about this feature?  I don't know. */
-void perform_wins(const char str[static 1], size_t len)
+int perform_wins(const char str[static 1], size_t len)
 {
     (void)str;
     (void)len;
+    return 0;
 }
 
 static void inform_execute(char c)

src/ifchd.h

@@ -36,14 +36,14 @@ extern int allow_hostname;
 extern uid_t ifch_uid;
 extern gid_t ifch_gid;
 
-void perform_timezone(const char str[static 1], size_t len);
+int perform_timezone(const char str[static 1], size_t len);
-void perform_dns(const char str[static 1], size_t len);
+int perform_dns(const char str[static 1], size_t len);
-void perform_lprsvr(const char str[static 1], size_t len);
+int perform_lprsvr(const char str[static 1], size_t len);
-void perform_hostname(const char str[static 1], size_t len);
+int perform_hostname(const char str[static 1], size_t len);
-void perform_domain(const char str[static 1], size_t len);
+int perform_domain(const char str[static 1], size_t len);
-void perform_ipttl(const char str[static 1], size_t len);
+int perform_ipttl(const char str[static 1], size_t len);
-void perform_ntpsrv(const char str[static 1], size_t len);
+int perform_ntpsrv(const char str[static 1], size_t len);
-void perform_wins(const char str[static 1], size_t len);
+int perform_wins(const char str[static 1], size_t len);
 
 void ifch_main(void);
 

src/ifset.c

@@ -303,7 +303,7 @@ static void link_flags_get_do(const struct nlmsghdr *nlh, void *data)
     }
 }
 
-static int link_flags_get(int fd, uint32_t *flags)
+static int link_flags_get(int fd, uint32_t flags[static 1])
 {
     char nlbuf[8192];
     struct link_flag_data ipx = { .fd = fd, .flags = 0, .got_flags = false };
@@ -493,34 +493,24 @@ int perform_ifup(void)
 }
 
 // str_bcast is optional.
-void perform_ip_subnet_bcast(const char *str_ipaddr,
+int perform_ip_subnet_bcast(const char str_ipaddr[static 1],
-                             const char *str_subnet, const char *str_bcast)
+                            const char str_subnet[static 1],
+                            const char *str_bcast)
 {
     struct in_addr ipaddr, subnet, bcast;
-    int fd, r;
+    int fd, r, ret = -1;
     uint8_t prefixlen;
 
-    if (!str_ipaddr) {
-        log_error("%s: (%s) interface ip address is NULL",
-                  client_config.interface, __func__);
-        return;
-    }
-    if (!str_subnet) {
-        log_error("%s: (%s) interface subnet address is NULL",
-                  client_config.interface, __func__);
-        return;
-    }
-
     if (inet_pton(AF_INET, str_ipaddr, &ipaddr) <= 0) {
         log_error("%s: (%s) bad interface ip address: '%s'",
                   client_config.interface, __func__, str_ipaddr);
-        return;
+        goto fail;
     }
 
     if (inet_pton(AF_INET, str_subnet, &subnet) <= 0) {
         log_error("%s: (%s) bad interface subnet address: '%s'",
                   client_config.interface, __func__, str_subnet);
-        return;
+        goto fail;
     }
     prefixlen = subnet4_to_prefixlen(subnet.s_addr);
 
@@ -528,7 +518,7 @@ void perform_ip_subnet_bcast(const char *str_ipaddr,
         if (inet_pton(AF_INET, str_bcast, &bcast) <= 0) {
             log_error("%s: (%s) bad interface broadcast address: '%s'",
                       client_config.interface, __func__, str_bcast);
-            return;
+            goto fail;
         }
     } else {
         // Generate the standard broadcast address if unspecified.
@@ -539,7 +529,7 @@ void perform_ip_subnet_bcast(const char *str_ipaddr,
     if (fd < 0) {
         log_error("%s: (%s) netlink socket open failed: %s",
                   client_config.interface, __func__, strerror(errno));
-        return;
+        goto fail;
     }
 
     r = ipbcpfx_clear_others(fd, ipaddr.s_addr, bcast.s_addr, prefixlen);
@@ -550,18 +540,15 @@ void perform_ip_subnet_bcast(const char *str_ipaddr,
         else if (r == -2)
             log_error("%s: (%s) error receiving link ip address list",
                       client_config.interface, __func__);
-        close(fd);
+        goto fail_fd;
-        return;
     }
 
     if (r < 1) {
         r = rtnl_addr_broadcast_send(fd, RTM_NEWADDR, IFA_F_PERMANENT,
                                      RT_SCOPE_UNIVERSE, &ipaddr.s_addr, &bcast.s_addr,
                                      prefixlen);
-        if (r < 0) {
+        if (r < 0)
-            close(fd);
+            goto fail_fd;
-            return;
-        }
 
         log_line("%s: Interface IP set to: '%s'", client_config.interface,
                  str_ipaddr);
@@ -574,87 +561,101 @@ void perform_ip_subnet_bcast(const char *str_ipaddr,
         log_line("%s: Interface IP, subnet, and broadcast were already OK.",
                  client_config.interface);
 
-    if (link_set_flags(fd, IFF_UP | IFF_RUNNING) < 0)
+    if (link_set_flags(fd, IFF_UP | IFF_RUNNING) < 0) {
         log_error("%s: (%s) Failed to set link to be up and running.",
                   client_config.interface, __func__);
+        goto fail_fd;
+    }
+    ret = 0;
+fail_fd:
     close(fd);
+fail:
+    return ret;
 }
 
 
-void perform_router(const char *str_router, size_t len)
+int perform_router(const char str_router[static 1], size_t len)
 {
-    if (!str_router)
+    int ret = -1;
-        return;
     if (len < 7)
-        return;
+        goto fail;
     struct in_addr router;
     if (inet_pton(AF_INET, str_router, &router) <= 0) {
         log_error("%s: (%s) bad router ip address: '%s'",
                   client_config.interface, __func__, str_router);
-        return;
+        goto fail;
     }
 
     int fd = socket(AF_NETLINK, SOCK_DGRAM | SOCK_NONBLOCK, NETLINK_ROUTE);
     if (fd < 0) {
         log_error("%s: (%s) netlink socket open failed: %s",
                   client_config.interface, __func__, strerror(errno));
-        return;
+        goto fail;
     }
 
-    if (rtnl_set_default_gw_v4(fd, router.s_addr,
+    if (rtnl_set_default_gw_v4(fd, router.s_addr, client_config.metric) < 0) {
-                               client_config.metric) < 0)
         log_error("%s: (%s) failed to set route: %s",
                   client_config.interface, __func__, strerror(errno));
-    else
+        goto fail_fd;
-        log_line("%s: Gateway router set to: '%s'", client_config.interface,
+    }
-                 str_router);
+    log_line("%s: Gateway router set to: '%s'", client_config.interface,
+             str_router);
+    ret = 0;
+fail_fd:
     close(fd);
+fail:
+    return ret;
 }
 
-void perform_mtu(const char *str, size_t len)
+int perform_mtu(const char str[static 1], size_t len)
 {
-    if (!str)
+    unsigned int mtu;
-        return;
+    int fd, ret = -1;
     if (len < 2)
-        return;
+        goto fail;
 
     char *estr;
     long tmtu = strtol(str, &estr, 10);
     if (estr == str) {
         log_error("%s: (%s) provided mtu arg isn't a valid number",
                   client_config.interface, __func__);
-        return;
+        goto fail;
     }
     if ((tmtu == LONG_MAX || tmtu == LONG_MIN) && errno == ERANGE) {
         log_error("%s: (%s) provided mtu arg would overflow a long",
                   client_config.interface, __func__);
-        return;
+        goto fail;
     }
     if (tmtu > INT_MAX) {
         log_error("%s: (%s) provided mtu arg would overflow int",
                   client_config.interface, __func__);
-        return;
+        goto fail;
     }
     // 68 bytes for IPv4.  1280 bytes for IPv6.
     if (tmtu < 68) {
         log_error("%s: (%s) provided mtu arg (%ul) less than minimum MTU (68)",
                   client_config.interface, __func__, tmtu);
-        return;
+        goto fail;
     }
-    unsigned int mtu = (unsigned int)tmtu;
+    mtu = (unsigned int)tmtu;
 
-    int fd = socket(AF_NETLINK, SOCK_DGRAM | SOCK_NONBLOCK, NETLINK_ROUTE);
+    fd = socket(AF_NETLINK, SOCK_DGRAM | SOCK_NONBLOCK, NETLINK_ROUTE);
     if (fd < 0) {
         log_error("%s: (%s) netlink socket open failed: %s",
                   client_config.interface, __func__, strerror(errno));
-        return;
+        goto fail;
     }
 
-    if (rtnl_if_mtu_set(fd, mtu) < 0)
+    if (rtnl_if_mtu_set(fd, mtu) < 0) {
         log_error("%s: (%s) failed to set MTU [%d]",
                   client_config.interface, __func__, mtu);
-    else
+        goto fail_fd;
-        log_line("%s: MTU set to: '%s'", client_config.interface, str);
+    }
+    log_line("%s: MTU set to: '%s'", client_config.interface, str);
+    ret = 0;
+fail_fd:
     close(fd);
+fail:
+    return ret;
 }
 

src/ifset.h

@@ -29,10 +29,10 @@
 #ifndef NJK_IFSET_H_
 #define NJK_IFSET_H_
 int perform_ifup(void);
-void perform_ip_subnet_bcast(const char *str_ipaddr,
+int perform_ip_subnet_bcast(const char str_ipaddr[static 1],
-                             const char *str_subnet,
+                            const char str_subnet[static 1],
-                             const char *str_bcast);
+                            const char *str_bcast);
-void perform_router(const char *str, size_t len);
+int perform_router(const char str[static 1], size_t len);
-void perform_mtu(const char *str, size_t len);
+int perform_mtu(const char *str, size_t len);
 #endif