From e526adce19413c883542f928c7b9b0789278e1ed Mon Sep 17 00:00:00 2001 From: "Nicholas J. Kain" Date: Tue, 15 Apr 2014 20:55:13 -0400 Subject: [PATCH] Make the signal handling code use safe_read() and unify ifchd and sockd signals code. --- src/ifchd.c | 55 +++++----------------------------------------------- src/ndhc.c | 32 +++++++++++++----------------- src/sockd.c | 56 +++++------------------------------------------------ src/sys.c | 44 ++++++++++++++++++++++++++++++++++++++++- src/sys.h | 4 ++++ 5 files changed, 70 insertions(+), 121 deletions(-) diff --git a/src/ifchd.c b/src/ifchd.c index 36d6043..f6637ee 100644 --- a/src/ifchd.c +++ b/src/ifchd.c @@ -35,13 +35,11 @@ #include #include #include -#include #include #include #include #include #include -#include #include "nk/log.h" #include "nk/privilege.h" #include "nk/signals.h" @@ -256,51 +254,6 @@ void perform_wins(const char *str, size_t len) (void)len; } -static void setup_signals_ifch(void) -{ - sigset_t mask; - sigemptyset(&mask); - sigaddset(&mask, SIGUSR1); - sigaddset(&mask, SIGUSR2); - sigaddset(&mask, SIGTSTP); - sigaddset(&mask, SIGTTIN); - sigaddset(&mask, SIGCHLD); - sigaddset(&mask, SIGHUP); - sigaddset(&mask, SIGINT); - sigaddset(&mask, SIGTERM); - if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) - suicide("sigprocmask failed"); - signalFd = signalfd(-1, &mask, SFD_NONBLOCK); - if (signalFd < 0) - suicide("signalfd failed"); -} - -static void signal_dispatch(void) -{ - int t; - size_t off = 0; - struct signalfd_siginfo si = {0}; - again: - t = read(signalFd, (char *)&si + off, sizeof si - off); - if (t < 0) { - if (t == EAGAIN || t == EWOULDBLOCK || t == EINTR) - goto again; - else - suicide("signalfd read error"); - } - if (off + (unsigned)t < sizeof si) - off += t; - switch (si.ssi_signo) { - case SIGINT: - case SIGTERM: - case SIGHUP: - exit(EXIT_SUCCESS); - break; - default: - break; - } -} - static void inform_execute(char c) { ssize_t r = safe_write(ifchSock[1], &c, sizeof c); @@ -366,7 +319,7 @@ static void do_ifch_work(void) if (fd == ifchSock[1]) process_client_socket(); else if (fd == signalFd) - signal_dispatch(); + signal_dispatch_subprocess(signalFd, "ifch"); else suicide("ifch: unexpected fd while performing epoll"); } @@ -376,9 +329,11 @@ static void do_ifch_work(void) void ifch_main(void) { prctl(PR_SET_NAME, "ndhc: ifch"); - prctl(PR_SET_PDEATHSIG, SIGHUP); + if (prctl(PR_SET_PDEATHSIG, SIGHUP) < 0) + suicide("%s: (%s) prctl(PR_SET_PDEATHSIG) failed: %s", + client_config.interface, __func__, strerror(errno)); umask(077); - setup_signals_ifch(); + signalFd = setup_signals_subprocess(); // If we are requested to update resolv.conf, preopen the fd before // we drop root privileges, making sure that if we create diff --git a/src/ndhc.c b/src/ndhc.c index 25b523d..b58f331 100644 --- a/src/ndhc.c +++ b/src/ndhc.c @@ -184,26 +184,21 @@ static void setup_signals_ndhc(void) static void signal_dispatch(void) { - int t; - size_t off = 0; struct signalfd_siginfo si = {0}; - again: - t = read(cs.signalFd, (char *)&si + off, sizeof si - off); - if (t < 0) { - if (t == EAGAIN || t == EWOULDBLOCK || t == EINTR) - goto again; - else - suicide("signalfd read error"); + 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; + } + if ((size_t)r < sizeof si) { + log_error("%s: ndhc: short read from signalfd: %zd < %zu", + client_config.interface, r, sizeof si); + return; } - if (off + (unsigned)t < sizeof si) - off += t; switch (si.ssi_signo) { - case SIGUSR1: - force_renew_action(&cs); - break; - case SIGUSR2: - force_release_action(&cs); - break; + case SIGUSR1: force_renew_action(&cs); break; + case SIGUSR2: force_release_action(&cs); break; case SIGCHLD: suicide("ndhc-master: Subprocess terminated unexpectedly. Exiting."); break; @@ -211,8 +206,7 @@ static void signal_dispatch(void) log_line("Received SIGTERM. Exiting gracefully."); exit(EXIT_SUCCESS); break; - default: - break; + default: break; } } diff --git a/src/sockd.c b/src/sockd.c index 2602791..5446aa8 100644 --- a/src/sockd.c +++ b/src/sockd.c @@ -36,7 +36,6 @@ #include #include #include -#include #include #include #include @@ -434,53 +433,6 @@ static int create_arp_basic_socket(bool *using_bpf) return fd; } -// XXX: Can share with ifch -static void setup_signals_sockd(void) -{ - sigset_t mask; - sigemptyset(&mask); - sigaddset(&mask, SIGUSR1); - sigaddset(&mask, SIGUSR2); - sigaddset(&mask, SIGTSTP); - sigaddset(&mask, SIGTTIN); - sigaddset(&mask, SIGCHLD); - sigaddset(&mask, SIGHUP); - sigaddset(&mask, SIGINT); - sigaddset(&mask, SIGTERM); - if (sigprocmask(SIG_BLOCK, &mask, NULL) < 0) - suicide("sigprocmask failed"); - signalFd = signalfd(-1, &mask, SFD_NONBLOCK); - if (signalFd < 0) - suicide("signalfd failed"); -} - -// XXX: Can share with ifch -static void signal_dispatch(void) -{ - int t; - size_t off = 0; - struct signalfd_siginfo si = {0}; - again: - t = read(signalFd, (char *)&si + off, sizeof si - off); - if (t < 0) { - if (t == EAGAIN || t == EWOULDBLOCK || t == EINTR) - goto again; - else - suicide("signalfd read error"); - } - if (off + (unsigned)t < sizeof si) - off += t; - switch (si.ssi_signo) { - case SIGINT: - case SIGTERM: - case SIGHUP: - exit(EXIT_SUCCESS); - break; - default: - break; - } -} - static void xfer_fd(int fd, char cmd) { char control[sizeof(struct cmsghdr) + 10]; @@ -607,7 +559,7 @@ static void do_sockd_work(void) if (fd == sockdSock[1]) process_client_socket(); else if (fd == signalFd) - signal_dispatch(); + signal_dispatch_subprocess(signalFd, "sockd"); else suicide("sockd: unexpected fd while performing epoll"); } @@ -617,9 +569,11 @@ static void do_sockd_work(void) void sockd_main(void) { prctl(PR_SET_NAME, "ndhc: sockd"); - prctl(PR_SET_PDEATHSIG, SIGHUP); + if (prctl(PR_SET_PDEATHSIG, SIGHUP) < 0) + suicide("%s: (%s) prctl(PR_SET_PDEATHSIG) failed: %s", + client_config.interface, __func__, strerror(errno)); umask(077); - setup_signals_sockd(); + signalFd = 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, diff --git a/src/sys.c b/src/sys.c index 2efbb81..d66e046 100644 --- a/src/sys.c +++ b/src/sys.c @@ -26,12 +26,16 @@ * POSSIBILITY OF SUCH DAMAGE. */ +#include +#include #include #include +#include #include #include #include "nk/log.h" - +#include "nk/io.h" +#include "ndhc.h" #include "sys.h" void epoll_add(int epfd, int fd) @@ -55,3 +59,41 @@ void epoll_del(int epfd, int fd) if (r < 0) suicide("epoll_del failed %s", strerror(errno)); } + +int setup_signals_subprocess(void) +{ + sigset_t mask; + sigemptyset(&mask); + sigaddset(&mask, SIGHUP); + sigaddset(&mask, SIGINT); + sigaddset(&mask, SIGTERM); + if (sigprocmask(SIG_BLOCK, &mask, NULL) < 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) +{ + struct signalfd_siginfo si = {0}; + 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; + } +} + diff --git a/src/sys.h b/src/sys.h index 04bc824..2ff2d59 100644 --- a/src/sys.h +++ b/src/sys.h @@ -46,4 +46,8 @@ static inline size_t min_size_t(size_t a, size_t b) void epoll_add(int epfd, int fd); void epoll_del(int epfd, int fd); +int setup_signals_subprocess(void); +void signal_dispatch_subprocess(int sfd, const char *pname); + #endif /* SYS_H_ */ +