From fdd5734875a49c4bfdebde45189038dd42306504 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 15 Dec 2017 12:45:30 -0500 Subject: [PATCH 1/3] Remove the unused is_parent==0 option from handle_signals. --- src/or/main.c | 56 +++++++++++++++++---------------------------------- src/or/main.h | 2 +- 2 files changed, 20 insertions(+), 38 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index aae98dd8ab..38f25cae50 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -2530,7 +2530,7 @@ do_main_loop(void) } } - handle_signals(1); + handle_signals(); monotime_init(); timers_initialize(); @@ -3052,46 +3052,28 @@ static struct { { -1, -1, NULL } }; -/** Set up the signal handlers for either parent or child process */ +/** Set up the signal handlers for this process. */ void -handle_signals(int is_parent) +handle_signals(void) { int i; - if (is_parent) { - for (i = 0; signal_handlers[i].signal_value >= 0; ++i) { - if (signal_handlers[i].try_to_register) { - signal_handlers[i].signal_event = - tor_evsignal_new(tor_libevent_get_base(), - signal_handlers[i].signal_value, - signal_callback, - &signal_handlers[i].signal_value); - if (event_add(signal_handlers[i].signal_event, NULL)) - log_warn(LD_BUG, "Error from libevent when adding " - "event for signal %d", - signal_handlers[i].signal_value); - } else { - signal_handlers[i].signal_event = - tor_event_new(tor_libevent_get_base(), -1, - EV_SIGNAL, signal_callback, - &signal_handlers[i].signal_value); - } + for (i = 0; signal_handlers[i].signal_value >= 0; ++i) { + if (signal_handlers[i].try_to_register) { + signal_handlers[i].signal_event = + tor_evsignal_new(tor_libevent_get_base(), + signal_handlers[i].signal_value, + signal_callback, + &signal_handlers[i].signal_value); + if (event_add(signal_handlers[i].signal_event, NULL)) + log_warn(LD_BUG, "Error from libevent when adding " + "event for signal %d", + signal_handlers[i].signal_value); + } else { + signal_handlers[i].signal_event = + tor_event_new(tor_libevent_get_base(), -1, + EV_SIGNAL, signal_callback, + &signal_handlers[i].signal_value); } - } else { -#ifndef _WIN32 - struct sigaction action; - action.sa_flags = 0; - sigemptyset(&action.sa_mask); - action.sa_handler = SIG_IGN; - sigaction(SIGINT, &action, NULL); - sigaction(SIGTERM, &action, NULL); - sigaction(SIGPIPE, &action, NULL); - sigaction(SIGUSR1, &action, NULL); - sigaction(SIGUSR2, &action, NULL); - sigaction(SIGHUP, &action, NULL); -#ifdef SIGXFSZ - sigaction(SIGXFSZ, &action, NULL); -#endif -#endif /* !defined(_WIN32) */ } } diff --git a/src/or/main.h b/src/or/main.h index 8eb977575e..49292f80a5 100644 --- a/src/or/main.h +++ b/src/or/main.h @@ -66,7 +66,7 @@ MOCK_DECL(long,get_uptime,(void)); unsigned get_signewnym_epoch(void); -void handle_signals(int is_parent); +void handle_signals(void); void activate_signal(int signal_num); int try_locking(const or_options_t *options, int err_if_locked); From 20f802ea3cd5d8b3993a101da03dfec95bc9049e Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 15 Dec 2017 12:48:29 -0500 Subject: [PATCH 2/3] Add an option to disable signal handler installation. Closes ticket 24588. --- changes/ticket24588 | 5 +++++ src/or/config.c | 7 +++++++ src/or/main.c | 4 +++- src/or/or.h | 5 +++++ 4 files changed, 20 insertions(+), 1 deletion(-) create mode 100644 changes/ticket24588 diff --git a/changes/ticket24588 b/changes/ticket24588 new file mode 100644 index 0000000000..e64872d74b --- /dev/null +++ b/changes/ticket24588 @@ -0,0 +1,5 @@ + o Minor features (embedding, mobile): + - Applications that want to embed Tor can now tell Tor not to register + any of its own POSIX signal handlers, using the __DisableSignalHandlers + option. This option is not meant for general use. Closes ticket 24588. + diff --git a/src/or/config.c b/src/or/config.c index 016d87f81f..ebbe726c7b 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -564,6 +564,7 @@ static config_var_t option_vars_[] = { VAR("__ReloadTorrcOnSIGHUP", BOOL, ReloadTorrcOnSIGHUP, "1"), VAR("__AllDirActionsPrivate", BOOL, AllDirActionsPrivate, "0"), VAR("__DisablePredictedCircuits",BOOL,DisablePredictedCircuits, "0"), + VAR("__DisableSignalHandlers", BOOL, DisableSignalHandlers, "0"), VAR("__LeaveStreamsUnattached",BOOL, LeaveStreamsUnattached, "0"), VAR("__HashedControlSessionPassword", LINELIST, HashedControlSessionPassword, NULL), @@ -4652,6 +4653,12 @@ options_transition_allowed(const or_options_t *old, return -1; } + if (old->DisableSignalHandlers != new_val->DisableSignalHandlers) { + *msg = tor_strdup("While Tor is running, changing DisableSignalHandlers " + "is not allowed."); + return -1; + } + if (strcmp(old->DataDirectory,new_val->DataDirectory)!=0) { tor_asprintf(msg, "While Tor is running, changing DataDirectory " diff --git a/src/or/main.c b/src/or/main.c index 38f25cae50..9adad07941 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -3057,8 +3057,10 @@ void handle_signals(void) { int i; + const int enabled = !get_options()->DisableSignalHandlers; + for (i = 0; signal_handlers[i].signal_value >= 0; ++i) { - if (signal_handlers[i].try_to_register) { + if (enabled && signal_handlers[i].try_to_register) { signal_handlers[i].signal_event = tor_evsignal_new(tor_libevent_get_base(), signal_handlers[i].signal_value, diff --git a/src/or/or.h b/src/or/or.h index 9c53949879..91d6436389 100644 --- a/src/or/or.h +++ b/src/or/or.h @@ -4651,6 +4651,11 @@ typedef struct { /** List of files that were opened by %include in torrc and torrc-defaults */ smartlist_t *FilesOpenedByIncludes; + + /** If true, Tor shouldn't install any posix signal handlers, since it is + * running embedded inside another process. + */ + int DisableSignalHandlers; } or_options_t; #define LOG_PROTOCOL_WARN (get_protocol_warning_severity_level()) From 65a27d95e750e118162f22797dc22c4d550b9fc8 Mon Sep 17 00:00:00 2001 From: Nick Mathewson Date: Fri, 19 Jan 2018 10:02:20 -0500 Subject: [PATCH 3/3] Improve documentation for signal code --- src/or/main.c | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/src/or/main.c b/src/or/main.c index 9adad07941..8cbe28a213 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -3016,9 +3016,15 @@ exit_function(void) #else #define UNIX_ONLY 1 #endif + static struct { + /** A numeric code for this signal. Must match the signal value if + * try_to_register is true. */ int signal_value; + /** True if we should try to register this signal with libevent and catch + * corresponding posix signals. False otherwise. */ int try_to_register; + /** Pointer to hold the event object constructed for this signal. */ struct event *signal_event; } signal_handlers[] = { #ifdef SIGINT @@ -3052,7 +3058,8 @@ static struct { { -1, -1, NULL } }; -/** Set up the signal handlers for this process. */ +/** Set up the signal handler events for this process, and register them + * with libevent if appropriate. */ void handle_signals(void) { @@ -3060,6 +3067,11 @@ handle_signals(void) const int enabled = !get_options()->DisableSignalHandlers; for (i = 0; signal_handlers[i].signal_value >= 0; ++i) { + /* Signal handlers are only registered with libevent if they need to catch + * real POSIX signals. We construct these signal handler events in either + * case, though, so that controllers can activate them with the SIGNAL + * command. + */ if (enabled && signal_handlers[i].try_to_register) { signal_handlers[i].signal_event = tor_evsignal_new(tor_libevent_get_base(), @@ -3079,7 +3091,7 @@ handle_signals(void) } } -/* Make sure the signal handler for signal_num will be called. */ +/* Cause the signal handler for signal_num to be called in the event loop. */ void activate_signal(int signal_num) {