commit da778f2921d0ae49c47abb4ba4ebe5f92a999ae2 Author: Nick Mathewson <nickm@torproject.org> Date: Wed Jan 24 12:02:44 2018 -0500 Use thread-safe types to store the LOG_PROTOCOL_WARN severity Fixes a race condition; resolves 23954. --- changes/bug23954 | 4 ++++ src/or/config.c | 49 +++++++++++++++++++++++++++++++++++------- src/or/config.h | 1 + src/or/main.c | 1 + src/test/fuzz/fuzzing_common.c | 2 ++ src/test/testing_common.c | 1 + 6 files changed, 50 insertions(+), 8 deletions(-) diff --git a/changes/bug23954 b/changes/bug23954 new file mode 100644 index 000000000..185814f12 --- /dev/null +++ b/changes/bug23954 @@ -0,0 +1,4 @@ + o Minor bugfixes (logging, race conditions): + - Fix a (mostly harmless) race condition when invoking + LOG_PROTOCOL_WARN message from a subthread while the options are + changing. Fixes bug 23954; bugfix on 0.1.1.9-alpha. diff --git a/src/or/config.c b/src/or/config.c index afaf86785..f035bbaba 100644 --- a/src/or/config.c +++ b/src/or/config.c @@ -766,6 +766,8 @@ static int options_validate_cb(void *old_options, void *options, int from_setconf, char **msg); static uint64_t compute_real_max_mem_in_queues(const uint64_t val, int log_guess); +static void cleanup_protocol_warning_severity_level(void); +static void set_protocol_warning_severity_level(int warning_severity); /** Magic value for or_options_t. */ #define OR_OPTIONS_MAGIC 9090909 @@ -999,6 +1001,8 @@ config_free_all(void) tor_free(the_short_tor_version); tor_free(the_tor_version); + cleanup_protocol_warning_severity_level(); + have_parsed_cmdline = 0; libevent_initialized = 0; } @@ -1064,17 +1068,46 @@ escaped_safe_str(const char *address) * The severity level that should be used for warnings of severity * LOG_PROTOCOL_WARN. * - * We keep this outside the options, in case somebody needs to use - * LOG_PROTOCOL_WARN while an option transition is happening. + * We keep this outside the options, and we use an atomic_counter_t, in case + * one thread needs to use LOG_PROTOCOL_WARN while an option transition is + * happening in the main thread. */ -static int protocol_warning_severity_level = LOG_WARN; +static atomic_counter_t protocol_warning_severity_level; /** Return the severity level that should be used for warnings of severity * LOG_PROTOCOL_WARN. */ int get_protocol_warning_severity_level(void) { - return protocol_warning_severity_level; + return (int) atomic_counter_get(&protocol_warning_severity_level); +} + +/** Set the protocol warning severity level to <b>severity</b>. */ +static void +set_protocol_warning_severity_level(int warning_severity) +{ + atomic_counter_exchange(&protocol_warning_severity_level, + warning_severity); +} + +/** + * Initialize the log warning severity level for protocol warnings. Call + * only once at startup. + */ +void +init_protocol_warning_severity_level(void) +{ + atomic_counter_init(&protocol_warning_severity_level); + set_protocol_warning_severity_level(LOG_WARN); +} + +/** + * Tear down protocol_warning_severity_level. + */ +static void +cleanup_protocol_warning_severity_level(void) +{ + atomic_counter_destroy(&protocol_warning_severity_level); } /** List of default directory authorities */ @@ -1794,10 +1827,10 @@ options_act(const or_options_t *old_options) return -1; } - if (options->ProtocolWarnings) - protocol_warning_severity_level = LOG_WARN; - else - protocol_warning_severity_level = LOG_INFO; + { + int warning_severity = options->ProtocolWarnings ? LOG_WARN : LOG_INFO; + set_protocol_warning_severity_level(warning_severity); + } if (consider_adding_dir_servers(options, old_options) < 0) { // XXXX This should get validated earlier, and committed here, to diff --git a/src/or/config.h b/src/or/config.h index 7c7ef1825..2f23809b2 100644 --- a/src/or/config.h +++ b/src/or/config.h @@ -31,6 +31,7 @@ const char *safe_str_client(const char *address); const char *safe_str(const char *address); const char *escaped_safe_str_client(const char *address); const char *escaped_safe_str(const char *address); +void init_protocol_warning_severity_level(void); int get_protocol_warning_severity_level(void); const char *get_version(void); const char *get_short_version(void); diff --git a/src/or/main.c b/src/or/main.c index 10e606f3a..841a37255 100644 --- a/src/or/main.c +++ b/src/or/main.c @@ -4009,6 +4009,7 @@ tor_run_main(const tor_main_configuration_t *tor_cfg) #endif /* defined(_WIN32) */ configure_backtrace_handler(get_version()); + init_protocol_warning_severity_level(); update_approx_time(time(NULL)); tor_threads_init(); diff --git a/src/test/fuzz/fuzzing_common.c b/src/test/fuzz/fuzzing_common.c index 1d54e41db..7c9fac748 100644 --- a/src/test/fuzz/fuzzing_common.c +++ b/src/test/fuzz/fuzzing_common.c @@ -152,6 +152,8 @@ main(int argc, char **argv) } } + init_protocol_warning_severity_level(); + { log_severity_list_t s; memset(&s, 0, sizeof(s)); diff --git a/src/test/testing_common.c b/src/test/testing_common.c index 142c68107..52729147b 100644 --- a/src/test/testing_common.c +++ b/src/test/testing_common.c @@ -278,6 +278,7 @@ main(int c, const char **v) s.masks[LOG_WARN-LOG_ERR] |= LD_BUG; add_stream_log(&s, "", fileno(stdout)); } + init_protocol_warning_severity_level(); options->command = CMD_RUN_UNITTESTS; if (crypto_global_init(accel_crypto, NULL, NULL)) {