From 06a541261eb8ce39a7240a132e059a32c2ed4f85 Mon Sep 17 00:00:00 2001 From: "Nicholas J. Kain" Date: Tue, 20 Oct 2020 04:23:55 -0400 Subject: [PATCH] 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 | 2 +- src/ifchd.c | 14 +++---- src/ndhc.c | 118 ++++++++++++++++++++++++---------------------------- src/ndhc.h | 13 +++++- src/sockd.c | 14 +++---- src/state.c | 108 ++++++++++++++++++++++++++++------------------- src/state.h | 11 +---- src/sys.c | 42 ++++--------------- src/sys.h | 4 +- 9 files changed, 152 insertions(+), 174 deletions(-) diff --git a/README.md b/README.md index e83fc22..810f054 100644 --- a/README.md +++ b/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. diff --git a/src/ifchd.c b/src/ifchd.c index 121485e..ddfcc8f 100644 --- a/src/ifchd.c +++ b/src/ifchd.c @@ -1,6 +1,6 @@ /* ifchd.c - interface change daemon * - * Copyright 2004-2018 Nicholas J. Kain + * Copyright 2004-2020 Nicholas J. Kain * 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; -/* Slots are for signalFd and the ndhc -> ifchd socket. */ +static int epollfd; +/* 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); diff --git a/src/ndhc.c b/src/ndhc.c index 7425cd9..09a63ef 100644 --- a/src/ndhc.c +++ b/src/ndhc.c @@ -1,6 +1,6 @@ /* ndhc.c - DHCP client * - * Copyright 2004-2018 Nicholas J. Kain + * Copyright 2004-2020 Nicholas J. Kain * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -41,7 +41,6 @@ #include #include #include -#include #include #include #include @@ -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: - break; + case SIGINT: + 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); - sigaddset(&mask, SIGUSR2); - sigaddset(&mask, SIGTERM); - sigaddset(&mask, SIGINT); - if (sigprocmask(SIG_BLOCK, &mask, (sigset_t *)0) < 0) + + if (sigprocmask(0, 0, &mask) < 0) + suicide("sigprocmask failed"); + for (int i = 0; ss[i] != SIGKILL; ++i) + if (sigdelset(&mask, ss[i])) + 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 (sigaction(SIGCHLD, &sa, NULL)) - suicide("sigaction failed"); - - if (cs.signalFd >= 0) { - 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; - } + if (sigemptyset(&sa.sa_mask)) + suicide("sigemptyset failed"); + for (int i = 0; ss[i] != SIGKILL; ++i) + if (sigaction(ss[i], &sa, NULL)) + suicide("sigaction failed"); } 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 (!(events[i].events & EPOLLIN)) - suicide("signalfd closed unexpectedly"); - sev_signal = signal_dispatch(); - } else if (fd == cs.listenFd) { + 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); diff --git a/src/ndhc.h b/src/ndhc.h index f964fb8..9d0b0a7 100644 --- a/src/ndhc.h +++ b/src/ndhc.h @@ -1,6 +1,6 @@ /* ndhc.h - DHCP client * - * Copyright 2014-2018 Nicholas J. Kain + * Copyright 2014-2020 Nicholas J. Kain * 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); diff --git a/src/sockd.c b/src/sockd.c index 3a0d321..6beed09 100644 --- a/src/sockd.c +++ b/src/sockd.c @@ -1,6 +1,6 @@ /* sockd.c - privsep socket creation daemon * - * Copyright 2014-2018 Nicholas J. Kain + * Copyright 2014-2020 Nicholas J. Kain * 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; -/* Slots are for signalFd and the ndhc -> ifchd socket. */ +static int epollfd; +/* 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, diff --git a/src/state.c b/src/state.c index dabbf5b..e41d5aa 100644 --- a/src/state.c +++ b/src/state.c @@ -1,6 +1,6 @@ /* state.c - high level DHCP state machine * - * Copyright 2011-2018 Nicholas J. Kain + * Copyright 2011-2020 Nicholas J. Kain * 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, - int sev_signal) + bool force_fingerprint, bool dhcp_timeout, bool arp_timeout) { scrBegin; reinit: // We're in the SELECTING state here. for (;;) { int ret = COR_SUCCESS; - if (sev_signal == SIGNAL_RELEASE) { - print_release(cs); - goto skip_to_released; - } + SIGNAL_CHECK(nolease); 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) { - print_release(cs); - goto skip_to_released; - } + SIGNAL_CHECK(nolease); 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) { - print_release(cs); - goto skip_to_released; - } + SIGNAL_CHECK(nolease); 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) { - 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; - } - } - } + SIGNAL_CHECK(havelease); 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) { - int r = frenew(cs, false); - if (r) { - ret = COR_ERROR; - scrReturn(ret); - continue; - } - break; - } + SIGNAL_CHECK(released); scrReturn(ret); } sev_dhcp = false; diff --git a/src/state.h b/src/state.h index dc43dd7..3d734d2 100644 --- a/src/state.h +++ b/src/state.h @@ -1,6 +1,6 @@ /* state.h - high level DHCP state machine * - * Copyright 2011-2018 Nicholas J. Kain + * Copyright 2011-2020 Nicholas J. Kain * 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, - int sev_signal); + bool force_fingerprint, bool dhcp_timeout, bool arp_timeout); #endif diff --git a/src/sys.c b/src/sys.c index 548e4d8..35dcefd 100644 --- a/src/sys.c +++ b/src/sys.c @@ -1,6 +1,6 @@ /* sys.c - linux-specific signal and epoll functions * - * Copyright 2010-2018 Nicholas J. Kain + * Copyright 2010-2020 Nicholas J. Kain * All rights reserved. * * Redistribution and use in source and binary forms, with or without @@ -32,7 +32,6 @@ #include #include #include -#include #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); - sigaddset(&mask, SIGHUP); - sigaddset(&mask, SIGINT); - sigaddset(&mask, SIGTERM); - if (sigprocmask(SIG_BLOCK, &mask, (sigset_t *)0) < 0) + if (sigprocmask(0, 0, &mask) < 0) + suicide("sigprocmask failed"); + if (sigaddset(&mask, SIGPIPE)) + suicide("sigaddset failed"); + 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; - } } diff --git a/src/sys.h b/src/sys.h index 2a499bf..c76d317 100644 --- a/src/sys.h +++ b/src/sys.h @@ -1,6 +1,6 @@ /* sys.h - linux-specific signal and epoll functions * - * Copyright 2010-2018 Nicholas J. Kain + * Copyright 2010-2020 Nicholas J. Kain * 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_ */