October 2019 - tor-commits - lists.torproject.org

[tor/release-0.4.1] torerr: Close sigsafe fds on shutdown
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit d02ced4cafaed5b11079585f03f47e73034dd980 Author: teor <teor(a)torproject.org> Date: Wed Sep 4 14:54:08 2019 +1000 torerr: Close sigsafe fds on shutdown And clear the list of error fds. Part of 31594. --- src/lib/err/torerr.c | 42 ++++++++++++++++++++++++++++++++++++++++-- src/lib/err/torerr.h | 1 + src/lib/err/torerr_sys.c | 5 ++++- 3 files changed, 45 insertions(+), 3 deletions(-) diff --git a/src/lib/err/torerr.c b/src/lib/err/torerr.c index ecffb7f7b..2c4a10a5b 100644 --- a/src/lib/err/torerr.c +++ b/src/lib/err/torerr.c @@ -110,6 +110,8 @@ tor_log_get_sigsafe_err_fds(const int **out) * Update the list of fds that get errors from inside a signal handler or * other emergency condition. Ignore any beyond the first * TOR_SIGSAFE_LOG_MAX_FDS. + * + * If fds is NULL or n is 0, clears the list of error fds. */ void tor_log_set_sigsafe_err_fds(const int *fds, int n) @@ -118,8 +120,18 @@ tor_log_set_sigsafe_err_fds(const int *fds, int n) n = TOR_SIGSAFE_LOG_MAX_FDS; } - memcpy(sigsafe_log_fds, fds, n * sizeof(int)); - n_sigsafe_log_fds = n; + /* Clear the entire array. This code mitigates against some race conditions, + * but there are still some races here: + * - err logs are disabled while the array is cleared, and + * - a thread can read the old value of n_sigsafe_log_fds, then read a + * partially written array. + * We could fix these races using atomics, but atomics use the err module. */ + n_sigsafe_log_fds = 0; + memset(sigsafe_log_fds, 0, sizeof(sigsafe_log_fds)); + if (fds && n > 0) { + memcpy(sigsafe_log_fds, fds, n * sizeof(int)); + n_sigsafe_log_fds = n; + } } /** @@ -133,6 +145,32 @@ tor_log_reset_sigsafe_err_fds(void) } /** + * Close the list of fds that get errors from inside a signal handler or + * other emergency condition. These fds are shared with the logging code: + * closing them flushes the log buffers, and prevents any further logging. + * + * This function closes stderr, so it should only be called immediately before + * process shutdown. + */ +void +tor_log_close_sigsafe_err_fds(void) +{ + int n_fds, i; + const int *fds = NULL; + + n_fds = tor_log_get_sigsafe_err_fds(&fds); + for (i = 0; i < n_fds; ++i) { + /* tor_log_close_sigsafe_err_fds_on_error() is called on error and on + * shutdown, so we can't log or take any useful action if close() + * fails. */ + (void)close(fds[i]); + } + + /* Don't even try logging, we've closed all the log fds. */ + tor_log_set_sigsafe_err_fds(NULL, 0); +} + +/** * Set the granularity (in ms) to use when reporting fatal errors outside * the logging system. */ diff --git a/src/lib/err/torerr.h b/src/lib/err/torerr.h index 0badaf7c6..3b86d2039 100644 --- a/src/lib/err/torerr.h +++ b/src/lib/err/torerr.h @@ -40,6 +40,7 @@ void tor_log_err_sigsafe(const char *m, ...); int tor_log_get_sigsafe_err_fds(const int **out); void tor_log_set_sigsafe_err_fds(const int *fds, int n); void tor_log_reset_sigsafe_err_fds(void); +void tor_log_close_sigsafe_err_fds(void); void tor_log_sigsafe_err_set_granularity(int ms); int format_hex_number_sigsafe(unsigned long x, char *buf, int max_len); diff --git a/src/lib/err/torerr_sys.c b/src/lib/err/torerr_sys.c index 3ab1b3c4e..a14c46f94 100644 --- a/src/lib/err/torerr_sys.c +++ b/src/lib/err/torerr_sys.c @@ -27,8 +27,11 @@ subsys_torerr_initialize(void) static void subsys_torerr_shutdown(void) { - tor_log_reset_sigsafe_err_fds(); + /* Stop handling signals with backtraces, then close the logs. */ clean_up_backtrace_handler(); + /* We can't log any log messages after this point: we've closed all the log + * fds, including stdio. */ + tor_log_close_sigsafe_err_fds(); } const subsys_fns_t sys_torerr = {

1 0

[tor/release-0.4.1] log: Close log and err file descriptors before aborting
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 1609916c79612b5cc3a9b363a22f1a9035f2f77b Author: teor <teor(a)torproject.org> Date: Wed Sep 4 17:16:49 2019 +1000 log: Close log and err file descriptors before aborting Part of 31594. --- src/lib/err/backtrace.c | 2 +- src/lib/err/torerr.c | 16 ++++++++++++++-- src/lib/err/torerr.h | 6 ++++-- src/lib/log/log.c | 42 ++++++++++++++++++++++++++++++++++++++++-- src/lib/log/log.h | 1 + src/lib/log/util_bug.c | 11 +++++++---- src/trunnel/trunnel-local.h | 1 + 7 files changed, 68 insertions(+), 11 deletions(-) diff --git a/src/lib/err/backtrace.c b/src/lib/err/backtrace.c index 1d1b3bcfa..643fe862b 100644 --- a/src/lib/err/backtrace.c +++ b/src/lib/err/backtrace.c @@ -172,7 +172,7 @@ crash_handler(int sig, siginfo_t *si, void *ctx_) for (i=0; i < n_fds; ++i) backtrace_symbols_fd(cb_buf, (int)depth, fds[i]); - abort(); + tor_raw_abort_(); } /** Write a backtrace to all of the emergency-error fds. */ diff --git a/src/lib/err/torerr.c b/src/lib/err/torerr.c index f460fd837..21b28a5f6 100644 --- a/src/lib/err/torerr.c +++ b/src/lib/err/torerr.c @@ -208,6 +208,18 @@ tor_raw_assertion_failed_msg_(const char *file, int line, const char *expr, dump_stack_symbols_to_error_fds(); } +/** + * Call the abort() function to kill the current process with a fatal + * error. But first, close the raw error file descriptors, so error messages + * are written before process termination. + **/ +void +tor_raw_abort_(void) +{ + tor_log_close_sigsafe_err_fds(); + abort(); +} + /* As format_{hex,dex}_number_sigsafe, but takes a <b>radix</b> argument * in range 2..16 inclusive. */ static int @@ -242,7 +254,7 @@ format_number_sigsafe(unsigned long x, char *buf, int buf_len, unsigned digit = (unsigned) (x % radix); if (cp <= buf) { /* Not tor_assert(); see above. */ - abort(); + tor_raw_abort_(); } --cp; *cp = "0123456789ABCDEF"[digit]; @@ -251,7 +263,7 @@ format_number_sigsafe(unsigned long x, char *buf, int buf_len, /* NOT tor_assert; see above. */ if (cp != buf) { - abort(); // LCOV_EXCL_LINE + tor_raw_abort_(); // LCOV_EXCL_LINE } return len; diff --git a/src/lib/err/torerr.h b/src/lib/err/torerr.h index 3b86d2039..a41109527 100644 --- a/src/lib/err/torerr.h +++ b/src/lib/err/torerr.h @@ -20,13 +20,13 @@ #define raw_assert(expr) STMT_BEGIN \ if (!(expr)) { \ tor_raw_assertion_failed_msg_(__FILE__, __LINE__, #expr, NULL); \ - abort(); \ + tor_raw_abort_(); \ } \ STMT_END #define raw_assert_unreached(expr) raw_assert(0) #define raw_assert_unreached_msg(msg) STMT_BEGIN \ tor_raw_assertion_failed_msg_(__FILE__, __LINE__, "0", (msg)); \ - abort(); \ + tor_raw_abort_(); \ STMT_END void tor_raw_assertion_failed_msg_(const char *file, int line, @@ -43,6 +43,8 @@ void tor_log_reset_sigsafe_err_fds(void); void tor_log_close_sigsafe_err_fds(void); void tor_log_sigsafe_err_set_granularity(int ms); +void tor_raw_abort_(void) ATTR_NORETURN; + int format_hex_number_sigsafe(unsigned long x, char *buf, int max_len); int format_dec_number_sigsafe(unsigned long x, char *buf, int max_len); diff --git a/src/lib/log/log.c b/src/lib/log/log.c index 2214d4b59..4adcc5cf5 100644 --- a/src/lib/log/log.c +++ b/src/lib/log/log.c @@ -224,6 +224,7 @@ int log_global_min_severity_ = LOG_NOTICE; static void delete_log(logfile_t *victim); static void close_log(logfile_t *victim); +static void close_log_sigsafe(logfile_t *victim); static char *domain_to_string(log_domain_mask_t domain, char *buf, size_t buflen); @@ -833,6 +834,30 @@ logs_free_all(void) * that happened between here and the end of execution. */ } +/** Close signal-safe log files. + * Closing the log files makes the process and OS flush log buffers. + * + * This function is safe to call from a signal handler. It should only be + * called when shutting down the log or err modules. It is currenly called + * by the err module, when terminating the process on an abnormal condition. + */ +void +logs_close_sigsafe(void) +{ + logfile_t *victim, *next; + /* We can't LOCK_LOGS() in a signal handler, because it may call + * signal-unsafe functions. And we can't deallocate memory, either. */ + next = logfiles; + logfiles = NULL; + while (next) { + victim = next; + next = next->next; + if (victim->needs_close) { + close_log_sigsafe(victim); + } + } +} + /** Remove and free the log entry <b>victim</b> from the linked-list * logfiles (it is probably present, but it might not be due to thread * racing issues). After this function is called, the caller shouldn't @@ -859,13 +884,26 @@ delete_log(logfile_t *victim) } /** Helper: release system resources (but not memory) held by a single - * logfile_t. */ + * signal-safe logfile_t. If the log's resources can not be released in + * a signal handler, does nothing. */ static void -close_log(logfile_t *victim) +close_log_sigsafe(logfile_t *victim) { if (victim->needs_close && victim->fd >= 0) { + /* We can't do anything useful here if close() fails: we're shutting + * down logging, and the err module only does fatal errors. */ close(victim->fd); victim->fd = -1; + } +} + +/** Helper: release system resources (but not memory) held by a single + * logfile_t. */ +static void +close_log(logfile_t *victim) +{ + if (victim->needs_close) { + close_log_sigsafe(victim); } else if (victim->is_syslog) { #ifdef HAVE_SYSLOG_H if (--syslog_count == 0) { diff --git a/src/lib/log/log.h b/src/lib/log/log.h index dbc1c4702..360951783 100644 --- a/src/lib/log/log.h +++ b/src/lib/log/log.h @@ -170,6 +170,7 @@ void logs_set_domain_logging(int enabled); int get_min_log_level(void); void switch_logs_debug(void); void logs_free_all(void); +void logs_close_sigsafe(void); void add_temp_log(int min_severity); void close_temp_logs(void); void rollback_log_changes(void); diff --git a/src/lib/log/util_bug.c b/src/lib/log/util_bug.c index c65a91ae9..343510884 100644 --- a/src/lib/log/util_bug.c +++ b/src/lib/log/util_bug.c @@ -11,6 +11,7 @@ #include "lib/log/util_bug.h" #include "lib/log/log.h" #include "lib/err/backtrace.h" +#include "lib/err/torerr.h" #ifdef TOR_UNIT_TESTS #include "lib/smartlist_core/smartlist_core.h" #include "lib/smartlist_core/smartlist_foreach.h" @@ -122,16 +123,18 @@ tor_bug_occurred_(const char *fname, unsigned int line, } /** - * Call the abort() function to kill the current process with a fatal - * error. + * Call the tor_raw_abort_() function to close raw logs, then kill the current + * process with a fatal error. But first, close the file-based log file + * descriptors, so error messages are written before process termination. * * (This is a separate function so that we declare it in util_bug.h without - * including stdlib in all the users of util_bug.h) + * including torerr.h in all the users of util_bug.h) **/ void tor_abort_(void) { - abort(); + logs_close_sigsafe(); + tor_raw_abort_(); } #ifdef _WIN32 diff --git a/src/trunnel/trunnel-local.h b/src/trunnel/trunnel-local.h index c4118fce4..80da37156 100644 --- a/src/trunnel/trunnel-local.h +++ b/src/trunnel/trunnel-local.h @@ -14,5 +14,6 @@ #define trunnel_reallocarray tor_reallocarray #define trunnel_assert tor_assert #define trunnel_memwipe(mem, len) memwipe((mem), 0, (len)) +#define trunnel_abort tor_abort_ #endif

1 0

[tor/release-0.4.1] changes: file for 31594
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit ebce7059ffbc3a4a8b7ff7cf923b0e6a402f4f33 Author: teor <teor(a)torproject.org> Date: Mon Sep 9 13:53:58 2019 +1000 changes: file for 31594 --- changes/bug31594 | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/changes/bug31594 b/changes/bug31594 new file mode 100644 index 000000000..75e6ec33c --- /dev/null +++ b/changes/bug31594 @@ -0,0 +1,5 @@ + o Minor bugfixes (error handling): + - When tor aborts due to an error, close log file descriptors before + aborting. Closing the logs makes some OSes flush log file buffers, + rather than deleting buffered log lines. Fixes bug 31594; + bugfix on 0.2.5.2-alpha.

1 0

[tor/maint-0.4.1] backtrace: Disable signal handlers in remove_bt_handler()
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 6b97a5a843341b5ea2a9039bdd5290d524a44cd4 Author: teor <teor(a)torproject.org> Date: Wed Sep 4 14:50:09 2019 +1000 backtrace: Disable signal handlers in remove_bt_handler() Fixes bug 31614; bugfix on 0.2.5.2-alpha. --- src/lib/err/backtrace.c | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/src/lib/err/backtrace.c b/src/lib/err/backtrace.c index 1d1b3bcfa..77dc6b8cb 100644 --- a/src/lib/err/backtrace.c +++ b/src/lib/err/backtrace.c @@ -190,13 +190,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 }; + /** Install signal handlers as needed so that when we crash, we produce a * useful stack trace. Return 0 on success, -errno on failure. */ static int install_bt_handler(const char *software) { - int trap_signals[] = { SIGSEGV, SIGILL, SIGFPE, SIGBUS, SIGSYS, - SIGIO, -1 }; int i, rv=0; strncpy(bt_version, software, sizeof(bt_version) - 1); @@ -235,6 +237,19 @@ install_bt_handler(const char *software) static void remove_bt_handler(void) { + int i; + + struct sigaction sa; + + memset(&sa, 0, sizeof(sa)); + sa.sa_handler = SIG_DFL; + sigfillset(&sa.sa_mask); + + for (i = 0; trap_signals[i] >= 0; ++i) { + /* remove_bt_handler() is called on shutdown, from low-level code. + * It's not a fatal error, so we just ignore it. */ + (void)sigaction(trap_signals[i], &sa, NULL); + } } #endif /* defined(USE_BACKTRACE) */

1 0

[tor/maint-0.4.1] backtrace: avoid undefined behaviour on re-initialisation
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 315f14c709d019c55731faedb19a97b771f9c42f Author: teor <teor(a)torproject.org> Date: Fri Sep 20 11:40:05 2019 +1000 backtrace: avoid undefined behaviour on re-initialisation 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. --- src/lib/err/backtrace.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/lib/err/backtrace.c b/src/lib/err/backtrace.c index bf833c162..2a956e611 100644 --- a/src/lib/err/backtrace.c +++ b/src/lib/err/backtrace.c @@ -251,6 +251,10 @@ remove_bt_handler(void) * It's not a fatal error, so we just ignore it. */ (void)sigaction(trap_signals[i], &sa, NULL); } + + /* cb_buf_mutex is statically initialised, so we can not destroy it. + * If we destroy it, and then re-initialise tor, all our backtraces will + * fail. */ } #endif /* defined(USE_BACKTRACE) */

1 0

[tor/maint-0.4.1] changes: file for 31614
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit c9c046c365f10f5cbffada921931413edc690dbe Author: teor <teor(a)torproject.org> Date: Thu Sep 26 12:09:56 2019 +1000 changes: file for 31614 --- changes/bug31614 | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/changes/bug31614 b/changes/bug31614 new file mode 100644 index 000000000..c425a9fcd --- /dev/null +++ b/changes/bug31614 @@ -0,0 +1,9 @@ + o Minor bugfixes (logging): + - Disable backtrace signal handlers when shutting down tor. + Fixes bug 31614; bugfix on 0.2.5.2-alpha. + - Add a missing check for HAVE_PTHREAD_H, because the backtrace code uses + mutexes. Fixes bug 31614; bugfix on 0.2.5.2-alpha. + o Documentation: + - Explain why we can't destroy the backtrace buffer mutex. Explain why + we don't need to destroy the log mutex. + Closes ticket 31736.

1 0

[tor/maint-0.4.1] sandbox: Allow backtrace signals to be disabled
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit 2f8a9a2db692a101b7e6241cbcdf9ed87841310b Author: teor <teor(a)torproject.org> Date: Tue Sep 24 13:51:38 2019 +1000 sandbox: Allow backtrace signals to be disabled Part of 31614. --- src/lib/sandbox/sandbox.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/lib/sandbox/sandbox.c b/src/lib/sandbox/sandbox.c index e2356a172..5c02c012e 100644 --- a/src/lib/sandbox/sandbox.c +++ b/src/lib/sandbox/sandbox.c @@ -295,6 +295,7 @@ sb_rt_sigaction(scmp_filter_ctx ctx, sandbox_cfg_t *filter) unsigned i; int rc; int param[] = { SIGINT, SIGTERM, SIGPIPE, SIGUSR1, SIGUSR2, SIGHUP, SIGCHLD, + SIGSEGV, SIGILL, SIGFPE, SIGBUS, SIGSYS, SIGIO, #ifdef SIGXFSZ SIGXFSZ #endif

1 0

[tor/maint-0.4.1] backtrace: Add a missing check for HAVE_PTHREAD_H before using mutexes
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit ab7bfdf40402449d6a2214e1dd5bc3fe5bc9a2e7 Author: teor <teor(a)torproject.org> Date: Mon Sep 9 11:21:15 2019 +1000 backtrace: Add a missing check for HAVE_PTHREAD_H before using mutexes Fixes bug 31614; bugfix on 0.2.5.2-alpha. --- src/lib/err/backtrace.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/err/backtrace.c b/src/lib/err/backtrace.c index 77dc6b8cb..bf833c162 100644 --- a/src/lib/err/backtrace.c +++ b/src/lib/err/backtrace.c @@ -57,7 +57,8 @@ #include "lib/err/torerr.h" #if defined(HAVE_EXECINFO_H) && defined(HAVE_BACKTRACE) && \ - defined(HAVE_BACKTRACE_SYMBOLS_FD) && defined(HAVE_SIGACTION) + defined(HAVE_BACKTRACE_SYMBOLS_FD) && defined(HAVE_SIGACTION) && \ + defined(HAVE_PTHREAD_H) #define USE_BACKTRACE #endif

1 0

[tor/maint-0.4.2] Merge remote-tracking branch 'tor-github/pr/1302' into maint-0.4.1
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit c1bf4260b69fa4de09471e31994b7fa242dce872 Merge: d4dde249a 749c2e176 Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 22 12:13:47 2019 -0400 Merge remote-tracking branch 'tor-github/pr/1302' into maint-0.4.1 changes/bug31614 | 9 +++++++++ src/lib/err/backtrace.c | 26 +++++++++++++++++++++++--- src/lib/log/log.c | 5 ++++- src/lib/sandbox/sandbox.c | 1 + 4 files changed, 37 insertions(+), 4 deletions(-)

1 0

[tor/maint-0.4.2] Merge branch 'maint-0.4.1' into maint-0.4.2
by nickm＠torproject.org 22 Oct '19

22 Oct '19

commit c55c75bc9747b8c0d9763c4f5cb248b65836c615 Merge: 34f3d8d40 c1bf4260b Author: Nick Mathewson <nickm(a)torproject.org> Date: Tue Oct 22 12:13:55 2019 -0400 Merge branch 'maint-0.4.1' into maint-0.4.2

1 0