Browse Source

Use thread-safe types to store the LOG_PROTOCOL_WARN severity

Fixes a race condition; resolves 23954.
Nick Mathewson 6 years ago
parent
commit
da778f2921
6 changed files with 50 additions and 8 deletions
  1. 4 0
      changes/bug23954
  2. 41 8
      src/or/config.c
  3. 1 0
      src/or/config.h
  4. 1 0
      src/or/main.c
  5. 2 0
      src/test/fuzz/fuzzing_common.c
  6. 1 0
      src/test/testing_common.c

+ 4 - 0
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.

+ 41 - 8
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

+ 1 - 0
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);

+ 1 - 0
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();

+ 2 - 0
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));

+ 1 - 0
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)) {