From 0a4d4e11c91cfeaa9bcad96856d30c5c5f538f19 Mon Sep 17 00:00:00 2001 From: =?utf8?q?David=20H=C3=A4rdeman?= Date: Sun, 16 Nov 2025 14:00:36 +0100 Subject: [PATCH] odhcpd: simplify signal handling MIME-Version: 1.0 Content-Type: text/plain; charset=utf8 Content-Transfer-Encoding: 8bit Currently, odhcpd's main() function will: - call uloop_init() - set up some signal handlers for SIGUSR1/SIGINT/SIGTERM (SIGUSR1 is ignored, the other handlers call uloop_end()) - call odhcpd_run(), which will: - set up some signal handlers for SIGTERM/SIGINT/SIGHUP (the first two call uloop_end(), SIGHUP calls odhcpd_reload()) Note that uloop_init() will call uloop_setup_signals() which will call uloop_setup_signals() which will install handlers for SIGINT/SIGTERM which will set uloop_cancelled to true (e.g. the same thing that uloop_end() does). In other words, the signal handlers are modified three times, and since the default "exit" action is to call uloop_end(), there's nothing that will actually react to signals until uloop_run() is called, meaning that the startup will be uninterruptible if e.g. ubus_init() fails since it'll just retry in a loop with no handling of signals. So, simplify and fix this by letting uloop handle the signals and checking if uloop has been told to exit, removing a bunch of special-purpose code. Signed-off-by: David Härdeman Link: https://github.com/openwrt/odhcpd/pull/312 Signed-off-by: Álvaro Fernández Rojas --- src/config.c | 41 ++++++++++++++--------------------------- src/odhcpd.c | 22 ++++++---------------- src/odhcpd.h | 2 +- 3 files changed, 21 insertions(+), 44 deletions(-) diff --git a/src/config.c b/src/config.c index 9246d64..a84021e 100644 --- a/src/config.c +++ b/src/config.c @@ -24,7 +24,6 @@ #include "dhcpv6-pxe.h" static struct blob_buf b; -static int reload_pipe[2] = { -1, -1 }; static int lease_cfg_cmp(const void *k1, const void *k2, void *ptr); static void lease_cfg_update(struct vlist_tree *tree, struct vlist_node *node_new, @@ -2433,40 +2432,28 @@ void odhcpd_reload(void) uci_free_context(uci); } -static void handle_signal(int signal) +static void signal_reload(_o_unused struct uloop_signal *signal) { - char b[1] = {0}; - - if (signal == SIGHUP) { - if (write(reload_pipe[1], b, sizeof(b)) < 0) {} - } else - uloop_end(); -} - -static void reload_cb(struct uloop_fd *u, _o_unused unsigned int events) -{ - char b[512]; - if (read(u->fd, b, sizeof(b)) < 0) {} - odhcpd_reload(); } -static struct uloop_fd reload_fd = { .fd = -1, .cb = reload_cb }; - -void odhcpd_run(void) +int odhcpd_run(void) { - if (pipe2(reload_pipe, O_NONBLOCK | O_CLOEXEC) < 0) {} + static struct uloop_signal sighup = { .signo = SIGHUP, .cb = signal_reload }; - reload_fd.fd = reload_pipe[0]; - uloop_fd_add(&reload_fd, ULOOP_READ); - - signal(SIGTERM, handle_signal); - signal(SIGINT, handle_signal); - signal(SIGHUP, handle_signal); - - while (ubus_init()) + while (ubus_init()) { + if (uloop_cancelled) + return EXIT_FAILURE; sleep(1); + } odhcpd_reload(); + + /* uloop_init() already handles SIGINT/SIGTERM */ + if (uloop_signal_add(&sighup) < 0) + return EXIT_FAILURE; + uloop_run(); + + return EXIT_SUCCESS; } diff --git a/src/odhcpd.c b/src/odhcpd.c index 1f2417b..bc0a833 100644 --- a/src/odhcpd.c +++ b/src/odhcpd.c @@ -67,11 +67,6 @@ void __iflog(int lvl, const char *fmt, ...) va_end(ap); } -static void sighandler(_o_unused int signal) -{ - uloop_end(); -} - static void print_usage(const char *app) { printf("== %s Usage ==\n" @@ -146,6 +141,11 @@ int main(int argc, char **argv) } } + if (getuid() != 0) { + error("Must be run as root!"); + return 2; + } + if (config.log_syslog) { openlog("odhcpd", LOG_PERROR | LOG_PID, LOG_DAEMON); setlogmask(LOG_UPTO(config.log_level)); @@ -153,19 +153,10 @@ int main(int argc, char **argv) uloop_init(); - if (getuid() != 0) { - error("Must be run as root!"); - return 2; - } - ioctl_sock = socket(AF_INET, SOCK_DGRAM | SOCK_CLOEXEC, 0); if (ioctl_sock < 0) return 4; - signal(SIGUSR1, SIG_IGN); - signal(SIGINT, sighandler); - signal(SIGTERM, sighandler); - if (netlink_init()) return 4; @@ -187,8 +178,7 @@ int main(int argc, char **argv) return 4; #endif - odhcpd_run(); - return 0; + return odhcpd_run(); } diff --git a/src/odhcpd.h b/src/odhcpd.h index eadb0fc..f41aaa1 100644 --- a/src/odhcpd.h +++ b/src/odhcpd.h @@ -536,7 +536,7 @@ int odhcpd_get_flags(const struct interface *iface); struct interface* odhcpd_get_interface_by_index(int ifindex); void odhcpd_urandom(void *data, size_t len); -void odhcpd_run(void); +int odhcpd_run(void); time_t odhcpd_time(void); ssize_t odhcpd_unhexlify(uint8_t *dst, size_t len, const char *src); void odhcpd_hexlify(char *dst, const uint8_t *src, size_t len); -- 2.30.2