[tor-commits] [tor/master] Threadproof our log_backtrace implementation

[nickm at torproject.org]

commit a3ab31f5dc55b2edde9436f6fb5f990548950702
Author: Nick Mathewson <nickm at torproject.org>
Date:   Mon Feb 24 12:15:32 2014 -0500

    Threadproof our log_backtrace implementation
    
    It's possible for two threads to hit assertion failures at the same
    time.  If that happens, let's keep them from stomping on the same
    cb_buf field.
    
    Fixes bug 11048; bugfix on 0.2.5.2-alpha. Reported by "cypherpunks".
---
 changes/bug11048       |    8 ++++++++
 src/common/backtrace.c |   22 ++++++++++++++++++----
 2 files changed, 26 insertions(+), 4 deletions(-)

diff --git a/changes/bug11048 b/changes/bug11048
new file mode 100644
index 0000000..684abf9
--- /dev/null
+++ b/changes/bug11048
@@ -0,0 +1,8 @@
+  o Minor bugfixes:
+
+    - Avoid strange behavior if two threads hit failed asswertions
+      at the same time and both try to log backtraces at
+      once. (Previously, if this had happened, both threads would
+      have stored their intermediate results in the same buffer, and
+      generated junk outputs.) Reported by "cypherpunks". Fixes bug
+      11048; bugfix on 0.2.5.2-alpha.
diff --git a/src/common/backtrace.c b/src/common/backtrace.c
index 5049298..3db2d6c 100644
--- a/src/common/backtrace.c
+++ b/src/common/backtrace.c
@@ -49,6 +49,8 @@ static char *bt_version = NULL;
 /** Static allocation of stack to dump. This is static so we avoid stack
  * pressure. */
 static void *cb_buf[MAX_DEPTH];
+/** Protects cb_buf_mutex from concurrent access */
+static tor_mutex_t cb_buf_mutex;
 
 /** Change a stacktrace in <b>stack</b> of depth <b>depth</b> so that it will
  * log the correct function from which a signal was received with context
@@ -84,18 +86,27 @@ clean_backtrace(void **stack, int depth, const ucontext_t *ctx)
 void
 log_backtrace(int severity, int domain, const char *msg)
 {
-  int depth = backtrace(cb_buf, MAX_DEPTH);
-  char **symbols = backtrace_symbols(cb_buf, depth);
+  int depth;
+  char **symbols;
   int i;
+
+  tor_mutex_acquire(&cb_buf_mutex);
+
+  depth = backtrace(cb_buf, MAX_DEPTH);
+  symbols = backtrace_symbols(cb_buf, depth);
+
   tor_log(severity, domain, "%s. Stack trace:", msg);
   if (!symbols) {
     tor_log(severity, domain, "    Unable to generate backtrace.");
-    return;
+    goto done;
   }
   for (i=0; i < depth; ++i) {
     tor_log(severity, domain, "    %s", symbols[i]);
   }
   free(symbols);
+
+ done:
+  tor_mutex_release(&cb_buf_mutex);
 }
 
 static void crash_handler(int sig, siginfo_t *si, void *ctx_)
@@ -140,6 +151,9 @@ install_bt_handler(void)
   int i, rv=0;
 
   struct sigaction sa;
+
+  tor_mutex_init(&cb_buf_mutex);
+
   memset(&sa, 0, sizeof(sa));
   sa.sa_sigaction = crash_handler;
   sa.sa_flags = SA_SIGINFO;
@@ -158,7 +172,7 @@ install_bt_handler(void)
 static void
 remove_bt_handler(void)
 {
-  /* We don't need to actually free anything at exit here. */
+  tor_mutex_uninit(&cb_buf_mutex);
 }
 #endif