commit a3ab31f5dc55b2edde9436f6fb5f990548950702 Author: Nick Mathewson nickm@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