Bug31614 035 #1302
Bug31614 035 #1302
Conversation
Pull Request Test Coverage Report for Build 6392
|
| @@ -190,13 +191,15 @@ dump_stack_symbols_to_error_fds(void) | |||
| backtrace_symbols_fd(cb_buf, (int)depth, fds[i]); | |||
| } | |||
|
|
|||
| /* The signals that we want our backtrace handler to trap */ | |||
| static int trap_signals[] = { SIGSEGV, SIGILL, SIGFPE, SIGBUS, SIGSYS, | |||
| SIGIO, -1 }; | |||
This in theory should be indented properly to follow our coding standards. I'm not going to hold up on this though to get it merged but if you have a minute, great! :)
src/lib/err/backtrace.c
Outdated
| /* We can't use tor_mutex_free() here, because lock uses err. | ||
| * This function is called while err is being shut down, so we can't | ||
| * log or take any useful action if pthread_mutex_destroy() fails. */ | ||
| (void)pthread_mutex_destroy(&cb_buf_mutex); |
Is this comment accurate? It seems to me it is saying we can be in this handler while the mutex is locked? I would be a bit surprised but just in case, this can't be done. Man page states:
It shall be safe to destroy an initialized mutex that is unlocked. Attempting to destroy a locked mutex or a mutex that is referenced (for example, while being used in a pthread_cond_timedwait() or pthread_cond_wait()) by another thread results in undefined behavior.
You're right, there are two race conditions here:
-
log_backtrace_impl() can be called from other threads, and lock cb_buf_mutex, while the main thread is calling this function during shut down, which leads to undefined behaviour
-
cb_buf is used in a lot of functions, which could be called concurrently:
a) install_bt_handler
b) dump_stack_symbols_to_error_fds
c) crash_handler
d) log_backtrace_impl (protected by cb_buf_mutex)
Which could lead to corrupted stack traces.
I don't know if there is an easy way to fix this. Maybe I should just open a ticket.
@nmathewson do you have any ideas?
teor2345
force-pushed
the
bug31614_035
branch
2 times, most recently
from
daecd85
to
1ddc988
Fixes bug 31614; bugfix on 0.2.5.2-alpha.
Fixes bug 31614; bugfix on 0.2.5.2-alpha.
cb_buf_mutex is statically initialised, so we can not destroy it when we are shutting down the err subsystem. If we destroy it, and then re-initialise tor, all our backtraces will fail. Part of 31736, but committed in this branch to avoid merge conflicts.
Part of 31614.
The log mutex is dynamically initialized, guarded by log_mutex_initialized. We don't want to destroy it, because after it is destroyed, we won't see any more logs. If tor is re-initialized, log_mutex_initialized will still be 1. So we won't trigger any undefined behaviour by trying to re-initialize the log mutex. Part of 31736, but committed in this branch to avoid merge conflicts.
No description provided.
The text was updated successfully, but these errors were encountered: