From: Alberto Leiva Popper Date: Wed, 16 Oct 2024 19:28:09 +0000 (-0600) Subject: Move the signal code out of the logging module X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=0d011065633a1c9ccb5a9290829ad081d1b09754;p=thirdparty%2FFORT-validator.git Move the signal code out of the logging module The sigaction() code was in logging because it was originally conceived by the SIGSEGV stack trace printing hack. The SIGPIPE ignorer was also incidentally moved there at some point, but it has never had anything to do with logging. And I'm going to catch more signals in the upcoming commits, so this really needs to be formalized into its own module. --- diff --git a/src/Makefile.am b/src/Makefile.am index f30c90c2..86cc34aa 100644 --- a/src/Makefile.am +++ b/src/Makefile.am @@ -68,6 +68,7 @@ fort_SOURCES += rtr/pdu_sender.c rtr/pdu_sender.h fort_SOURCES += rtr/pdu_stream.c rtr/pdu_stream.h fort_SOURCES += rtr/primitive_writer.c rtr/primitive_writer.h fort_SOURCES += rtr/rtr.c rtr/rtr.h +fort_SOURCES += sig.h sig.c fort_SOURCES += slurm/db_slurm.c slurm/db_slurm.h fort_SOURCES += slurm/slurm_loader.c slurm/slurm_loader.h fort_SOURCES += slurm/slurm_parser.c slurm/slurm_parser.h diff --git a/src/log.c b/src/log.c index 741d508b..09b81b17 100644 --- a/src/log.c +++ b/src/log.c @@ -6,7 +6,6 @@ #endif #include #include -#include #include #include #include @@ -112,99 +111,6 @@ static void init_config(struct log_config *cfg) cfg->facility = LOG_DAEMON; } -#ifdef BACKTRACE_ENABLED - -static void -sigsegv_handler(int signum) -{ - /* - * IMPORTANT: See https://stackoverflow.com/questions/29982643 - * I went with rationalcoder's answer, because I think not printing - * stack traces on segfaults is a nice way of ending up committing - * suicide. - * - * Here's a list of legal functions: - * https://stackoverflow.com/a/16891799/1735458 - * (Man, I wish POSIX standards were easier to link to.) - */ - -#define STACK_SIZE 64 - void *array[STACK_SIZE]; - size_t size; - - size = backtrace(array, STACK_SIZE); - backtrace_symbols_fd(array, size, STDERR_FILENO); - - /* Trigger default handler. */ - signal(signum, SIG_DFL); - kill(getpid(), signum); -} - -#endif - -/* - * Register better handlers for some signals. - * - * Remember to enable -rdynamic (See print_stack_trace()). - */ -static void -register_signal_handlers(void) -{ -#ifdef BACKTRACE_ENABLED - struct sigaction action; - void* dummy; - - /* - * Make sure libgcc is loaded; otherwise backtrace() might allocate - * during a signal handler. (Which is illegal.) - */ - dummy = NULL; - backtrace(&dummy, 1); - - /* Register the segmentation fault handler */ - memset(&action, 0, sizeof(action)); - action.sa_handler = sigsegv_handler; - sigemptyset(&action.sa_mask); - action.sa_flags = 0; - if (sigaction(SIGSEGV, &action, NULL) == -1) { - /* - * Not fatal; it just means we will not print stack traces on - * Segmentation Faults. - */ - pr_op_err("SIGSEGV handler registration failure: %s", - strerror(errno)); - } -#endif - - /* - * SIGPIPE can be triggered by any I/O function. libcurl is particularly - * tricky: - * - * > libcurl makes an effort to never cause such SIGPIPEs to trigger, - * > but some operating systems have no way to avoid them and even on - * > those that have there are some corner cases when they may still - * > happen - * (Documentation of CURLOPT_NOSIGNAL) - * - * All SIGPIPE means is "the peer closed the connection for some - * reason." - * Which is a normal I/O error, and should be handled by the normal - * error propagation logic, not by a signal handler. - * So, ignore SIGPIPE. - * - * https://github.com/NICMx/FORT-validator/issues/49 - */ - if (signal(SIGPIPE, SIG_IGN) == SIG_ERR) { - /* - * Not fatal. It just means that, if a broken pipe happens, we - * will die silently. - * But they're somewhat rare, so whatever. - */ - pr_op_err("SIGPIPE handler registration failure: %s", - strerror(errno)); - } -} - int log_setup(void) { @@ -236,8 +142,6 @@ log_setup(void) return error; } - register_signal_handlers(); - return 0; } diff --git a/src/main.c b/src/main.c index db09bea5..014a1afc 100644 --- a/src/main.c +++ b/src/main.c @@ -10,6 +10,7 @@ #include "print_file.h" #include "relax_ng.h" #include "rtr/rtr.h" +#include "sig.h" #include "thread_var.h" static int @@ -119,7 +120,7 @@ main(int argc, char **argv) error = log_setup(); if (error) goto just_quit; - + register_signal_handlers(); error = thvar_init(); if (error) goto revert_log; diff --git a/src/sig.c b/src/sig.c new file mode 100644 index 00000000..96918a15 --- /dev/null +++ b/src/sig.c @@ -0,0 +1,103 @@ +#include "sig.h" + +#include +#ifdef BACKTRACE_ENABLED +#include +#endif +#include +#include +#include +#include +#include + +#include "log.h" + +/* + * Ensures libgcc is loaded; otherwise backtrace() might allocate + * during a signal handler (which is illegal). + */ +static void +setup_backtrace(void) +{ +#ifdef BACKTRACE_ENABLED + void *dummy; + dummy = NULL; + backtrace(&dummy, 1); +#endif +} + +void +print_stack_trace(void) +{ +#ifdef BACKTRACE_ENABLED + /* + * See https://stackoverflow.com/questions/29982643 + * I went with rationalcoder's answer, because I think not printing + * stack traces on segfaults is a nice way of ending up committing + * suicide. + */ + void *array[64]; + size_t size; + size = backtrace(array, 64); + backtrace_symbols_fd(array, size, STDERR_FILENO); +#endif +} + +/* + * THIS IS A SIGNAL HANDLER. + * Legal functions: https://pubs.opengroup.org/onlinepubs/9799919799/ + */ +static void +do_cleanup(int signum) +{ + print_stack_trace(); + + /* Trigger default handler */ + signal(signum, SIG_DFL); + kill(getpid(), signum); +} + +/* Remember to enable -rdynamic (See print_stack_trace()). */ +void +register_signal_handlers(void) +{ + /* Important: All of these need to terminate by default */ + int const cleanups[] = { SIGSEGV, SIGBUS, 0 }; + struct sigaction action; + unsigned int i; + + setup_backtrace(); + + memset(&action, 0, sizeof(action)); + action.sa_handler = do_cleanup; + sigfillset(&action.sa_mask); + action.sa_flags = 0; + + for (i = 0; cleanups[i]; i++) + if (sigaction(cleanups[i], &action, NULL) < 0) + pr_op_err("'%s' signal action registration failure: %s", + strsignal(cleanups[i]), strerror(errno)); + + /* + * SIGPIPE can be triggered by any I/O function. libcurl is particularly + * tricky: + * + * > libcurl makes an effort to never cause such SIGPIPEs to trigger, + * > but some operating systems have no way to avoid them and even on + * > those that have there are some corner cases when they may still + * > happen + * (Documentation of CURLOPT_NOSIGNAL) + * + * All SIGPIPE means is "the peer closed the connection for some + * reason." + * Which is a normal I/O error, and should be handled by the normal + * error propagation logic, not by a signal handler. + * So, ignore SIGPIPE. + * + * https://github.com/NICMx/FORT-validator/issues/49 + */ + action.sa_handler = SIG_IGN; + if (sigaction(SIGPIPE, &action, NULL) < 0) + pr_op_err("SIGPIPE action registration failure: %s", + strerror(errno)); +} diff --git a/src/sig.h b/src/sig.h new file mode 100644 index 00000000..37ea33f6 --- /dev/null +++ b/src/sig.h @@ -0,0 +1,6 @@ +#ifndef SRC_SIG_H_ +#define SRC_SIG_H_ + +void register_signal_handlers(void); + +#endif /* SRC_SIG_H_ */