Stop using signalfd and audit signal handling code.

There's really no advantage to using signalfd in ndhc, particularly
since the normal POSIX signal API is now used for handling SIGCHLD in
ndhc-master.  So just use the tried and true volatile sig_atomic_t set
and check approach.

The only intended behavior change is in the dhcp RELEASE state --
before there would be a spurious attempt at renewing a nonexistent
lease when the RENEW signal was received.

README.md

@@ -1,5 +1,5 @@
 # ndhc
-Copyright 2004-2018 Nicholas J. Kain.
+Copyright 2004-2020 Nicholas J. Kain.
 
 See LICENSE for licensing information.  In short: Two-clause / New BSD.
 

src/ifchd.c

@@ -1,6 +1,6 @@
 /* ifchd.c - interface change daemon
  *
- * Copyright 2004-2018 Nicholas J. Kain <njkain at gmail dot com>
+ * Copyright 2004-2020 Nicholas J. Kain <njkain at gmail dot com>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -53,8 +53,8 @@
 
 struct ifchd_client cl;
 
-static int epollfd, signalFd;
+static int epollfd;
-/* Slots are for signalFd and the ndhc -> ifchd socket. */
+/* Slots are for the ndhc -> ifchd socket. */
 static struct epoll_event events[2];
 
 static int resolv_conf_fd = -1;
@@ -352,10 +352,9 @@ static void do_ifch_work(void)
 
     epoll_add(epollfd, ifchSock[1]);
     epoll_add(epollfd, ifchStream[1]);
-    epoll_add(epollfd, signalFd);
 
     for (;;) {
-        int r = epoll_wait(epollfd, events, 3, -1);
+        int r = epoll_wait(epollfd, events, 2, -1);
         if (r < 0) {
             if (errno == EINTR)
                 continue;
@@ -370,9 +369,6 @@ static void do_ifch_work(void)
             } else if (fd == ifchStream[1]) {
                 if (events[i].events & (EPOLLHUP|EPOLLERR|EPOLLRDHUP))
                     exit(EXIT_SUCCESS);
-            } else if (fd == signalFd) {
-                if (events[i].events & EPOLLIN)
-                    signal_dispatch_subprocess(signalFd, "ifch");
             } else
                 suicide("ifch: unexpected fd while performing epoll");
         }
@@ -414,7 +410,7 @@ void ifch_main(void)
 {
     prctl(PR_SET_NAME, "ndhc: ifch");
     umask(077);
-    signalFd = setup_signals_subprocess();
+    setup_signals_subprocess();
     setup_resolv_conf();
 
     nk_set_chroot(chroot_dir);

src/ndhc.c

@@ -1,6 +1,6 @@
 /* ndhc.c - DHCP client
  *
- * Copyright 2004-2018 Nicholas J. Kain <njkain at gmail dot com>
+ * Copyright 2004-2020 Nicholas J. Kain <njkain at gmail dot com>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -41,7 +41,6 @@
 #include <netinet/in.h>
 #include <arpa/inet.h>
 #include <sys/epoll.h>
-#include <sys/signalfd.h>
 #include <sys/prctl.h>
 #include <net/if.h>
 #include <errno.h>
@@ -75,7 +74,6 @@
 struct client_state_t cs = {
     .program_init = true,
     .epollFd = -1,
-    .signalFd = -1,
     .listenFd = -1,
     .arpFd = -1,
     .nlFd = -1,
@@ -93,12 +91,33 @@ struct client_config_t client_config = {
     .metric = 0,
 };
 
+static volatile sig_atomic_t l_signal_exit;
+static volatile sig_atomic_t l_signal_renew;
+static volatile sig_atomic_t l_signal_release;
+// Intended to be called in a loop until SIGNAL_NONE is returned.
+int signals_flagged(void)
+{
+    if (l_signal_exit) {
+        l_signal_exit = 0;
+        return SIGNAL_EXIT;
+    }
+    if (l_signal_renew) {
+        l_signal_renew = 0;
+        return SIGNAL_RENEW;
+    }
+    if (l_signal_release) {
+        l_signal_release = 0;
+        return SIGNAL_RELEASE;
+    }
+    return SIGNAL_NONE;
+}
+
 void set_client_addr(const char v[static 1]) { cs.clientAddr = inet_addr(v); }
 
 void print_version(void)
 {
     printf("ndhc %s, dhcp client.\n", NDHC_VERSION);
-    printf("Copyright 2004-2018 Nicholas J. Kain\n"
+    printf("Copyright 2004-2020 Nicholas J. Kain\n"
            "All rights reserved.\n\n"
            "Redistribution and use in source and binary forms, with or without\n"
            "modification, are permitted provided that the following conditions are met:\n\n"
@@ -125,7 +144,7 @@ void show_usage(void)
 {
     printf(
 "ndhc " NDHC_VERSION ", dhcp client.  Licensed under 2-clause BSD.\n"
-"Copyright 2004-2018 Nicholas J. Kain\n"
+"Copyright 2004-2020 Nicholas J. Kain\n"
 "Usage: ndhc [OPTIONS]\n\n"
 "  -c, --config=FILE               Path to ndhc configuration file\n"
 "  -I, --clientid=CLIENTID         Client identifier\n"
@@ -166,70 +185,46 @@ static void signal_handler(int signo)
         safe_write(STDOUT_FILENO, errstr, sizeof errstr - 1);
         exit(EXIT_FAILURE);
     }
-    default:
+    case SIGINT:
-        break;
+    case SIGTERM: l_signal_exit = 1; break;
+    case SIGUSR1: l_signal_renew = 1; break;
+    case SIGUSR2: l_signal_release = 1; break;
+    default: break;
     }
 }
 
+void signal_exit(int status)
+{
+    log_line("Received terminal signal. Exiting.");
+    exit(status);
+}
+
 static void setup_signals_ndhc(void)
 {
+    static const int ss[] = {
+        SIGCHLD, SIGINT, SIGTERM, SIGUSR1, SIGUSR2, SIGKILL
+    };
     sigset_t mask;
-    sigemptyset(&mask);
+
-    sigaddset(&mask, SIGUSR1);
+    if (sigprocmask(0, 0, &mask) < 0)
-    sigaddset(&mask, SIGUSR2);
+        suicide("sigprocmask failed");
-    sigaddset(&mask, SIGTERM);
+    for (int i = 0; ss[i] != SIGKILL; ++i)
-    sigaddset(&mask, SIGINT);
+        if (sigdelset(&mask, ss[i]))
-    if (sigprocmask(SIG_BLOCK, &mask, (sigset_t *)0) < 0)
+            suicide("sigdelset failed");
+    if (sigaddset(&mask, SIGPIPE))
+        suicide("sigaddset failed");
+    if (sigprocmask(SIG_SETMASK, &mask, (sigset_t *)0) < 0)
         suicide("sigprocmask failed");
 
-    sigemptyset(&mask);
-    sigaddset(&mask, SIGCHLD);
-    if (sigprocmask(SIG_UNBLOCK, &mask, (sigset_t *)0) < 0)
-        suicide("sigprocmask failed");
     struct sigaction sa = {
         .sa_handler = signal_handler,
         .sa_flags = SA_RESTART,
     };
-    sigemptyset(&sa.sa_mask);
+    if (sigemptyset(&sa.sa_mask))
-    if (sigaction(SIGCHLD, &sa, NULL))
+        suicide("sigemptyset failed");
-        suicide("sigaction failed");
+    for (int i = 0; ss[i] != SIGKILL; ++i)
-
+        if (sigaction(ss[i], &sa, NULL))
-    if (cs.signalFd >= 0) {
+            suicide("sigaction failed");
-        epoll_del(cs.epollFd, cs.signalFd);
-        close(cs.signalFd);
-    }
-    cs.signalFd = signalfd(-1, &mask, SFD_NONBLOCK);
-    if (cs.signalFd < 0)
-        suicide("signalfd failed");
-    epoll_add(cs.epollFd, cs.signalFd);
-}
-
-static int signal_dispatch(void)
-{
-    struct signalfd_siginfo si;
-    memset(&si, 0, sizeof si);
-    ssize_t r = safe_read(cs.signalFd, (char *)&si, sizeof si);
-    if (r < 0) {
-        log_error("%s: ndhc: error reading from signalfd: %s",
-                  client_config.interface, strerror(errno));
-        return SIGNAL_NONE;
-    }
-    if ((size_t)r < sizeof si) {
-        log_error("%s: ndhc: short read from signalfd: %zd < %zu",
-                  client_config.interface, r, sizeof si);
-        return SIGNAL_NONE;
-    }
-    switch (si.ssi_signo) {
-        case SIGUSR1: return SIGNAL_RENEW;
-        case SIGUSR2: return SIGNAL_RELEASE;
-        case SIGTERM:
-            log_line("Received SIGTERM.  Exiting gracefully.");
-            exit(EXIT_SUCCESS);
-        case SIGINT:
-            log_line("Received SIGINT.  Exiting gracefully.");
-            exit(EXIT_SUCCESS);
-        default: return SIGNAL_NONE;
-    }
 }
 
 static int is_string_hwaddr(const char str[static 1], size_t slen)
@@ -317,16 +312,11 @@ static void do_ndhc_work(void)
         bool sev_arp = false;
         int sev_nl = IFS_NONE;
         int sev_rfk = RFK_NONE;
-        int sev_signal = SIGNAL_NONE;
         bool force_fingerprint = false;
         for (int i = 0; i < maxi; ++i) {
             had_event = true;
             int fd = events[i].data.fd;
-            if (fd == cs.signalFd) {
+            if (fd == cs.listenFd) {
-                if (!(events[i].events & EPOLLIN))
-                    suicide("signalfd closed unexpectedly");
-                sev_signal = signal_dispatch();
-            } else if (fd == cs.listenFd) {
                 if (!(events[i].events & EPOLLIN))
                     suicide("listenfd closed unexpectedly");
                 sev_dhcp = dhcp_packet_get(&cs, &dhcp_packet, &dhcp_msgtype,
@@ -387,7 +377,7 @@ static void do_ndhc_work(void)
                                   dhcp_msgtype, dhcp_srcaddr,
                                   sev_arp, force_fingerprint,
                                   cs.dhcp_wake_ts <= nowts,
-                                  arp_wake_ts <= nowts, sev_signal);
+                                  arp_wake_ts <= nowts);
 
         if (dhcp_ok == COR_ERROR) {
             timeout = 2000 + (int)(nk_random_u32(&cs.rnd_state) % 3000);

src/ndhc.h

@@ -1,6 +1,6 @@
 /* ndhc.h - DHCP client
  *
- * Copyright 2014-2018 Nicholas J. Kain <njkain at gmail dot com>
+ * Copyright 2014-2020 Nicholas J. Kain <njkain at gmail dot com>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -45,7 +45,7 @@ struct client_state_t {
     long long leaseStartTime, renewTime, rebindTime;
     long long dhcp_wake_ts;
     int ifDeconfig; // Set if the interface has already been deconfigured.
-    int epollFd, signalFd, listenFd, arpFd, nlFd, rfkillFd;
+    int epollFd, listenFd, arpFd, nlFd, rfkillFd;
     int server_arp_sent, router_arp_sent;
     uint32_t nlPortId;
     unsigned int num_dhcp_requests, num_dhcp_renews;
@@ -75,6 +75,13 @@ struct client_config_t {
     bool enable_rfkill;          // Listen for rfkill events
 };
 
+enum {
+    SIGNAL_NONE = 0,
+    SIGNAL_EXIT,
+    SIGNAL_RENEW,
+    SIGNAL_RELEASE
+};
+
 extern struct client_config_t client_config;
 
 extern int ifchSock[2];
@@ -89,8 +96,10 @@ extern uid_t ndhc_uid;
 extern gid_t ndhc_gid;
 extern bool write_pid_enabled;
 
+int signals_flagged(void);
 void set_client_addr(const char v[static 1]);
 void show_usage(void);
+void signal_exit(int status);
 int get_clientid_string(const char str[static 1], size_t slen);
 void background(void);
 void print_version(void);

src/sockd.c

@@ -1,6 +1,6 @@
 /* sockd.c - privsep socket creation daemon
  *
- * Copyright 2014-2018 Nicholas J. Kain <njkain at gmail dot com>
+ * Copyright 2014-2020 Nicholas J. Kain <njkain at gmail dot com>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -58,8 +58,8 @@
 #include "dhcp.h"
 #include "sys.h"
 
-static int epollfd, signalFd;
+static int epollfd;
-/* Slots are for signalFd and the ndhc -> ifchd socket. */
+/* Slots are for the ndhc -> ifchd socket. */
 static struct epoll_event events[2];
 
 uid_t sockd_uid = 0;
@@ -556,10 +556,9 @@ static void do_sockd_work(void)
 
     epoll_add(epollfd, sockdSock[1]);
     epoll_add(epollfd, sockdStream[1]);
-    epoll_add(epollfd, signalFd);
 
     for (;;) {
-        int r = epoll_wait(epollfd, events, 3, -1);
+        int r = epoll_wait(epollfd, events, 2, -1);
         if (r < 0) {
             if (errno == EINTR)
                 continue;
@@ -574,9 +573,6 @@ static void do_sockd_work(void)
             } else if (fd == sockdStream[1]) {
                 if (events[i].events & (EPOLLHUP|EPOLLERR|EPOLLRDHUP))
                     exit(EXIT_SUCCESS);
-            } else if (fd == signalFd) {
-                if (events[i].events & EPOLLIN)
-                    signal_dispatch_subprocess(signalFd, "sockd");
             } else
                 suicide("sockd: unexpected fd while performing epoll");
         }
@@ -587,7 +583,7 @@ void sockd_main(void)
 {
     prctl(PR_SET_NAME, "ndhc: sockd");
     umask(077);
-    signalFd = setup_signals_subprocess();
+    setup_signals_subprocess();
     nk_set_chroot(chroot_dir);
     memset(chroot_dir, 0, sizeof chroot_dir);
     unsigned char keepcaps[] = { CAP_NET_BIND_SERVICE, CAP_NET_BROADCAST,

src/state.c

@@ -1,6 +1,6 @@
 /* state.c - high level DHCP state machine
  *
- * Copyright 2011-2018 Nicholas J. Kain <njkain at gmail dot com>
+ * Copyright 2011-2020 Nicholas J. Kain <njkain at gmail dot com>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -431,24 +431,78 @@ no_fingerprint:
     return IFUP_NEWLEASE;
 }
 
+// If ret == 0: do nothing
+//    ret == 1: ret = COR_ERROR; scrReturn(ret); continue
+//    ret == 2: goto skip_to_released
+//    ret == 3: break
+static int signal_check_nolease(struct client_state_t cs[static 1])
+{
+    for (;;) {
+        int s = signals_flagged();
+        if (s == SIGNAL_NONE) break;
+        if (s == SIGNAL_EXIT) signal_exit(EXIT_SUCCESS);
+        if (s == SIGNAL_RELEASE) {
+            print_release(cs);
+            return 2;
+        }
+    }
+    return 0;
+}
+static int signal_check_havelease(struct client_state_t cs[static 1])
+{
+    for (;;) {
+        int s = signals_flagged();
+        if (s == SIGNAL_NONE) break;
+        if (s == SIGNAL_EXIT) signal_exit(EXIT_SUCCESS);
+        if (s == SIGNAL_RELEASE) {
+            int r = xmit_release(cs);
+            if (r) return 1;
+            return 2;
+        }
+        if (s == SIGNAL_RENEW) {
+            int r = frenew(cs, true);
+            if (r) return 1;
+        }
+    }
+    return 0;
+}
+static int signal_check_released(struct client_state_t cs[static 1])
+{
+    (void)cs;
+    for (;;) {
+        int s = signals_flagged();
+        if (s == SIGNAL_NONE) break;
+        if (s == SIGNAL_EXIT) signal_exit(EXIT_SUCCESS);
+        if (s == SIGNAL_RENEW) return 3;
+    }
+    return 0;
+}
+#define SIGNAL_CHECK(NAME) \
+{ \
+    int tt = signal_check_ ## NAME(cs); \
+    if (tt == 1) { \
+        ret = COR_ERROR; \
+        scrReturn(ret); \
+        continue; \
+    } \
+    if (tt == 2) goto skip_to_released; \
+    if (tt == 3) break; \
+}
+
 #define BAD_STATE() suicide("%s(%d): bad state", __func__, __LINE__)
 
 // XXX: Should be re-entrant so as to handle multiple servers.
 int dhcp_handle(struct client_state_t cs[static 1], long long nowts,
                 bool sev_dhcp, struct dhcpmsg dhcp_packet[static 1],
                 uint8_t dhcp_msgtype, uint32_t dhcp_srcaddr, bool sev_arp,
-                bool force_fingerprint, bool dhcp_timeout, bool arp_timeout,
+                bool force_fingerprint, bool dhcp_timeout, bool arp_timeout)
-                int sev_signal)
 {
     scrBegin;
 reinit:
     // We're in the SELECTING state here.
     for (;;) {
         int ret = COR_SUCCESS;
-        if (sev_signal == SIGNAL_RELEASE) {
+        SIGNAL_CHECK(nolease);
-            print_release(cs);
-            goto skip_to_released;
-        }
         if (sev_dhcp) {
             int r = selecting_packet(cs, dhcp_packet, dhcp_msgtype,
                                      dhcp_srcaddr, false);
@@ -473,10 +527,7 @@ reinit:
         int ret;
 skip_to_requesting:
         ret = COR_SUCCESS;
-        if (sev_signal == SIGNAL_RELEASE) {
+        SIGNAL_CHECK(nolease);
-            print_release(cs);
-            goto skip_to_released;
-        }
         if (sev_dhcp) {
             int r = selecting_packet(cs, dhcp_packet, dhcp_msgtype,
                                      dhcp_srcaddr, true);
@@ -513,10 +564,7 @@ skip_to_requesting:
     for (;;) {
         int ret;
         ret = COR_SUCCESS;
-        if (sev_signal == SIGNAL_RELEASE) {
+        SIGNAL_CHECK(nolease);
-            print_release(cs);
-            goto skip_to_released;
-        }
         if (sev_dhcp) {
             // XXX: Maybe I can think of something to do here.  Would
             //      be more relevant if we tracked multiple dhcp servers.
@@ -568,25 +616,7 @@ skip_to_requesting:
     // We're in the BOUND, RENEWING, or REBINDING states here.
     for (;;) {
         int ret = COR_SUCCESS;
-        if (sev_signal) {
+        SIGNAL_CHECK(havelease);
-            if (sev_signal == SIGNAL_RELEASE) {
-                int r = xmit_release(cs);
-                if (r) {
-                    ret = COR_ERROR;
-                    scrReturn(ret);
-                    continue;
-                }
-                goto skip_to_released;
-            }
-            if (sev_signal == SIGNAL_RENEW) {
-                int r = frenew(cs, true);
-                if (r) {
-                    ret = COR_ERROR;
-                    scrReturn(ret);
-                    continue;
-                }
-            }
-        }
         if (sev_dhcp && cs->sent_renew_or_rebind) {
             int r = extend_packet(cs, dhcp_packet, dhcp_msgtype, dhcp_srcaddr);
             if (r == ANP_SUCCESS || r == ANP_IGNORE) {
@@ -712,15 +742,7 @@ skip_to_requesting:
         int ret;
 skip_to_released:
         ret = COR_SUCCESS;
-        if (sev_signal == SIGNAL_RENEW) {
+        SIGNAL_CHECK(released);
-            int r = frenew(cs, false);
-            if (r) {
-                ret = COR_ERROR;
-                scrReturn(ret);
-                continue;
-            }
-            break;
-        }
         scrReturn(ret);
     }
     sev_dhcp = false;

src/state.h

@@ -1,6 +1,6 @@
 /* state.h - high level DHCP state machine
  *
- * Copyright 2011-2018 Nicholas J. Kain <njkain at gmail dot com>
+ * Copyright 2011-2020 Nicholas J. Kain <njkain at gmail dot com>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -34,17 +34,10 @@
 #define COR_SUCCESS 0
 #define COR_ERROR -1
 
-enum {
-    SIGNAL_NONE = 0,
-    SIGNAL_RENEW,
-    SIGNAL_RELEASE
-};
-
 int dhcp_handle(struct client_state_t cs[static 1], long long nowts,
                 bool sev_dhcp, struct dhcpmsg dhcp_packet[static 1],
                 uint8_t dhcp_msgtype, uint32_t dhcp_srcaddr, bool sev_arp,
-                bool force_fingerprint, bool dhcp_timeout, bool arp_timeout,
+                bool force_fingerprint, bool dhcp_timeout, bool arp_timeout);
-                int sev_signal);
 
 #endif
 

src/sys.c

@@ -1,6 +1,6 @@
 /* sys.c - linux-specific signal and epoll functions
  *
- * Copyright 2010-2018 Nicholas J. Kain <njkain at gmail dot com>
+ * Copyright 2010-2020 Nicholas J. Kain <njkain at gmail dot com>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -32,7 +32,6 @@
 #include <errno.h>
 #include <signal.h>
 #include <sys/epoll.h>
-#include <sys/signalfd.h>
 #include "nk/log.h"
 #include "nk/io.h"
 #include "ndhc.h"
@@ -70,41 +69,14 @@ void epoll_del(int epfd, int fd)
         suicide("epoll_del failed %s", strerror(errno));
 }
 
-int setup_signals_subprocess(void)
+void setup_signals_subprocess(void)
 {
     sigset_t mask;
-    sigemptyset(&mask);
+    if (sigprocmask(0, 0, &mask) < 0)
-    sigaddset(&mask, SIGHUP);
+        suicide("sigprocmask failed");
-    sigaddset(&mask, SIGINT);
+    if (sigaddset(&mask, SIGPIPE))
-    sigaddset(&mask, SIGTERM);
+        suicide("sigaddset failed");
-    if (sigprocmask(SIG_BLOCK, &mask, (sigset_t *)0) < 0)
+    if (sigprocmask(SIG_SETMASK, &mask, (sigset_t *)0) < 0)
         suicide("sigprocmask failed");
-    int sfd = signalfd(-1, &mask, SFD_NONBLOCK);
-    if (sfd < 0)
-        suicide("signalfd failed");
-    return sfd;
-}
-
-void signal_dispatch_subprocess(int sfd, const char pname[static 1])
-{
-    struct signalfd_siginfo si;
-    memset(&si, 0, sizeof si);
-    ssize_t r = safe_read(sfd, (char *)&si, sizeof si);
-    if (r < 0) {
-        log_error("%s: %s: error reading from signalfd: %s",
-                  client_config.interface, pname, strerror(errno));
-        return;
-    }
-    if ((size_t)r < sizeof si) {
-        log_error("%s: %s: short read from signalfd: %zd < %zu",
-                  client_config.interface, pname, r, sizeof si);
-        return;
-    }
-    switch (si.ssi_signo) {
-        case SIGINT:
-        case SIGTERM:
-        case SIGHUP: exit(EXIT_SUCCESS); break;
-        default: break;
-    }
 }
 

src/sys.h

@@ -1,6 +1,6 @@
 /* sys.h - linux-specific signal and epoll functions
  *
- * Copyright 2010-2018 Nicholas J. Kain <njkain at gmail dot com>
+ * Copyright 2010-2020 Nicholas J. Kain <njkain at gmail dot com>
  * All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
@@ -41,7 +41,7 @@ long long IMPL_curms(const char *parent_function);
 void epoll_add(int epfd, int fd);
 void epoll_del(int epfd, int fd);
 
-int setup_signals_subprocess(void);
+void setup_signals_subprocess(void);
 void signal_dispatch_subprocess(int sfd, const char pname[static 1]);
 
 #endif /* SYS_H_ */