April 2020 - tor-commits - lists.torproject.org

[tor/master] Merge branch 'maint-0.4.3'
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 25729910af31266f74e55f4a00ee86be188f92c3 Merge: 29693b83b c4da0a509 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Apr 9 08:33:36 2020 -0400 Merge branch 'maint-0.4.3' src/lib/sandbox/sandbox.c | 1 + 1 file changed, 1 insertion(+)

1 0

[tor/maint-0.4.3] Add fsync to list of syscalls permitted by sandbox
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit c4da0a5094e21241db8ce0d8b12c2e4272fa49ef Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Apr 9 08:33:11 2020 -0400 Add fsync to list of syscalls permitted by sandbox (Our fix for 33087 requires this, I believe.) --- 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 1d2c40c75..626a4dc5a 100644 --- a/src/lib/sandbox/sandbox.c +++ b/src/lib/sandbox/sandbox.c @@ -166,6 +166,7 @@ static int filter_nopar_gen[] = { #ifdef __NR_fstat64 SCMP_SYS(fstat64), #endif + SCMP_SYS(fsync), SCMP_SYS(futex), SCMP_SYS(getdents), SCMP_SYS(getdents64),

1 0

[tor/maint-0.4.3] Merge remote-tracking branch 'tor-github/pr/1723/head' into maint-0.4.3
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit c2aea6134a3333bbace155f37fcabf8a5b4e2744 Merge: 1ae0839ef e849881d3 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Apr 9 08:30:14 2020 -0400 Merge remote-tracking branch 'tor-github/pr/1723/head' into maint-0.4.3 changes/bug33087 | 7 ++++++ configure.ac | 1 + src/lib/err/torerr.c | 26 ++++++++++----------- src/lib/err/torerr.h | 2 +- src/lib/err/torerr_sys.c | 6 ++--- src/lib/log/log.c | 60 +++++++++++++++++------------------------------- src/lib/log/log.h | 2 +- src/lib/log/util_bug.c | 2 +- 8 files changed, 46 insertions(+), 60 deletions(-)

1 0

[tor/maint-0.4.3] Merge branch 'bug33087_042' into bug33087_043
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 4f533ed28b831eb794f562dac9add1dbf37f6335 Merge: 8a23393ed b6d1f99cb Author: teor <teor(a)torproject.org> Date: Thu Feb 13 00:02:11 2020 +1000 Merge branch 'bug33087_042' into bug33087_043 "ours" merge, because the 0.4.2 backport deletes the close functions. But the 0.4.3 bug fix replaces them with fsync() where available.

1 0

[tor/release-0.4.3] Merge branch 'bug33087_041' into bug33087_042
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit b6d1f99cb2ef422bb47d67acf7f8d982decb429b Merge: ff5220536 2c0dfb093 Author: teor <teor(a)torproject.org> Date: Wed Feb 12 19:19:43 2020 +1000 Merge branch 'bug33087_041' into bug33087_042 changes/bug33087 | 4 ++++ src/lib/err/torerr.c | 31 ++----------------------------- src/lib/err/torerr.h | 1 - src/lib/err/torerr_sys.c | 5 +---- src/lib/log/log.c | 36 +++--------------------------------- src/lib/log/log.h | 1 - src/lib/log/util_bug.c | 4 +--- 7 files changed, 11 insertions(+), 71 deletions(-)

1 0

[tor/release-0.4.3] Merge branch 'maint-0.4.1' into bug33087_041
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 2c0dfb093e2afb2bd5a06c2f5441d55237729de4 Merge: c8242e4c0 5298113da Author: teor <teor(a)torproject.org> Date: Wed Feb 12 19:19:38 2020 +1000 Merge branch 'maint-0.4.1' into bug33087_041 .travis.yml | 2 -- changes/bug32753 | 3 +++ changes/ticket33075 | 4 ++++ src/app/config/config.c | 2 +- src/feature/relay/router.c | 15 ++++++++++----- src/test/test_config.c | 22 +++++++++++++++++++--- 6 files changed, 37 insertions(+), 11 deletions(-)

1 0

[tor/release-0.4.3] err/log: Stop closing stderr and stdout during shutdown
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit c8242e4c0ae838121ed00d5a38cf0dae052e9d13 Author: teor <teor(a)torproject.org> Date: Wed Feb 12 12:47:15 2020 +1000 err/log: Stop closing stderr and stdout during shutdown Closing these file descriptors can hide sanitiser logs. Fixes bug 33087; bugfix on 0.4.1.6. --- changes/bug33087 | 4 ++++ src/lib/err/torerr.c | 31 ++----------------------------- src/lib/err/torerr.h | 1 - src/lib/err/torerr_sys.c | 5 +---- src/lib/log/log.c | 36 +++--------------------------------- src/lib/log/log.h | 1 - src/lib/log/util_bug.c | 4 +--- 7 files changed, 11 insertions(+), 71 deletions(-) diff --git a/changes/bug33087 b/changes/bug33087 new file mode 100644 index 000000000..7acf72a83 --- /dev/null +++ b/changes/bug33087 @@ -0,0 +1,4 @@ + o Minor bugfixes (logging): + - Stop closing stderr and stdout during shutdown. Closing these file + descriptors can hide sanitiser logs. + Fixes bug 33087; bugfix on 0.4.1.6. diff --git a/src/lib/err/torerr.c b/src/lib/err/torerr.c index b7e32a3e2..5095c28ba 100644 --- a/src/lib/err/torerr.c +++ b/src/lib/err/torerr.c @@ -151,32 +151,6 @@ 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. */ @@ -217,13 +191,12 @@ tor_raw_assertion_failed_msg_(const char *file, int line, const char *expr, /** * 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. + * error. This is a separate function, so that log users don't have to include + * the header for abort(). **/ void tor_raw_abort_(void) { - tor_log_close_sigsafe_err_fds(); abort(); } diff --git a/src/lib/err/torerr.h b/src/lib/err/torerr.h index 0e839cb1b..a1822d9c9 100644 --- a/src/lib/err/torerr.h +++ b/src/lib/err/torerr.h @@ -40,7 +40,6 @@ 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); void tor_raw_abort_(void) ATTR_NORETURN; diff --git a/src/lib/err/torerr_sys.c b/src/lib/err/torerr_sys.c index a14c46f94..6a51a45a4 100644 --- a/src/lib/err/torerr_sys.c +++ b/src/lib/err/torerr_sys.c @@ -27,11 +27,8 @@ subsys_torerr_initialize(void) static void subsys_torerr_shutdown(void) { - /* Stop handling signals with backtraces, then close the logs. */ + /* Stop handling signals with backtraces. */ 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 = { diff --git a/src/lib/log/log.c b/src/lib/log/log.c index ec7c2fa24..69624e9ca 100644 --- a/src/lib/log/log.c +++ b/src/lib/log/log.c @@ -668,12 +668,8 @@ tor_log_update_sigsafe_err_fds(void) /* log_fds and err_fds contain matching entries: log_fds are the fds used by * the log module, and err_fds are the fds used by the err module. - * For stdio logs, the log_fd and err_fd values are identical, - * and the err module closes the fd on shutdown. - * For file logs, the err_fd is a dup() of the log_fd, - * and the log and err modules both close their respective fds on shutdown. - * (Once all fds representing a file are closed, the underlying file is - * closed.) + * For stdio logs, the log_fd and err_fd values are identical. + * For file logs, the err_fd is a dup() of the log_fd. */ int log_fds[TOR_SIGSAFE_LOG_MAX_FDS]; int err_fds[TOR_SIGSAFE_LOG_MAX_FDS]; @@ -704,12 +700,10 @@ tor_log_update_sigsafe_err_fds(void) log_fds[n_fds] = lf->fd; if (lf->needs_close) { /* File log fds are duplicated, because close_log() closes the log - * module's fd, and tor_log_close_sigsafe_err_fds() closes the err * module's fd. Both refer to the same file. */ err_fds[n_fds] = dup(lf->fd); } else { - /* stdio log fds are not closed by the log module. - * tor_log_close_sigsafe_err_fds() closes stdio logs. */ + /* stdio log fds are not closed by the log module. */ err_fds[n_fds] = lf->fd; } n_fds++; @@ -838,30 +832,6 @@ logs_free_all(void) * log mutex. */ } -/** 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 diff --git a/src/lib/log/log.h b/src/lib/log/log.h index 4291418eb..c4a27782c 100644 --- a/src/lib/log/log.h +++ b/src/lib/log/log.h @@ -173,7 +173,6 @@ 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 0e99be35a..640fe050e 100644 --- a/src/lib/log/util_bug.c +++ b/src/lib/log/util_bug.c @@ -163,8 +163,7 @@ tor_bug_occurred_(const char *fname, unsigned int line, /** * 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. + * process with a fatal error. * * (This is a separate function so that we declare it in util_bug.h without * including torerr.h in all the users of util_bug.h) @@ -172,7 +171,6 @@ tor_bug_occurred_(const char *fname, unsigned int line, void tor_abort_(void) { - logs_close_sigsafe(); tor_raw_abort_(); }

1 0

[tor/release-0.4.3] err/log: Stop closing stderr and stdout during shutdown
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 3d1ef3b6f89e760b4340ba77e0b3db1246dc5c80 Author: teor <teor(a)torproject.org> Date: Wed Feb 12 12:47:15 2020 +1000 err/log: Stop closing stderr and stdout during shutdown Closing these file descriptors can hide sanitiser logs. Instead, flush the logs before tor exits, using fsync(). Some Windows environments don't have fsync(), so we check for it at compile time. Fixes bug 33087; bugfix on 0.4.1.6. --- changes/bug33087 | 7 +++++++ configure.ac | 1 + src/lib/err/torerr.c | 26 ++++++++++++-------------- src/lib/err/torerr.h | 2 +- src/lib/err/torerr_sys.c | 6 ++---- src/lib/log/log.c | 29 ++++++++++++++--------------- src/lib/log/log.h | 2 +- src/lib/log/util_bug.c | 2 +- 8 files changed, 39 insertions(+), 36 deletions(-) diff --git a/changes/bug33087 b/changes/bug33087 new file mode 100644 index 000000000..ab6df58cc --- /dev/null +++ b/changes/bug33087 @@ -0,0 +1,7 @@ + o Minor bugfixes (logging): + - Stop closing stderr and stdout during shutdown. Closing these file + descriptors can hide sanitiser logs. + Fixes bug 33087; bugfix on 0.4.1.6. + - Flush stderr, stdout, and file logs during shutdown, if supported by the + OS. This change helps make sure that any final logs are recorded. + Fixes bug 33087; bugfix on 0.4.1.6. diff --git a/configure.ac b/configure.ac index 009d23aaa..1a5b6817b 100644 --- a/configure.ac +++ b/configure.ac @@ -649,6 +649,7 @@ AC_CHECK_FUNCS( explicit_bzero \ timingsafe_memcmp \ flock \ + fsync \ ftime \ get_current_dir_name \ getaddrinfo \ diff --git a/src/lib/err/torerr.c b/src/lib/err/torerr.c index 92ef80e56..c2dd862e1 100644 --- a/src/lib/err/torerr.c +++ b/src/lib/err/torerr.c @@ -151,29 +151,27 @@ tor_log_reset_sigsafe_err_fds(void) } /** - * Close the list of fds that get errors from inside a signal handler or + * Flush 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. + * flushing them also flushes the log buffers. * - * This function closes stderr, so it should only be called immediately before - * process shutdown. + * This function is safe to call during signal handlers. */ void -tor_log_close_sigsafe_err_fds(void) +tor_log_flush_sigsafe_err_fds(void) { + /* If we don't have fsync() in unistd.h, we can't flush the logs. */ +#ifdef HAVE_FSYNC 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]); + /* This function is called on error and on shutdown, so we don't log, or + * take any other action, if fsync() fails. */ + (void)fsync(fds[i]); } - - /* Don't even try logging, we've closed all the log fds. */ - tor_log_set_sigsafe_err_fds(NULL, 0); +#endif } /** @@ -217,13 +215,13 @@ tor_raw_assertion_failed_msg_(const char *file, int line, const char *expr, /** * Call the abort() function to kill the current process with a fatal - * error. But first, close the raw error file descriptors, so error messages + * error. But first, flush the raw error file descriptors, so error messages * are written before process termination. **/ void tor_raw_abort_(void) { - tor_log_close_sigsafe_err_fds(); + tor_log_flush_sigsafe_err_fds(); abort(); } diff --git a/src/lib/err/torerr.h b/src/lib/err/torerr.h index f5b36eef9..ce1b049c4 100644 --- a/src/lib/err/torerr.h +++ b/src/lib/err/torerr.h @@ -40,7 +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_flush_sigsafe_err_fds(void); void tor_log_sigsafe_err_set_granularity(int ms); void tor_raw_abort_(void) ATTR_NORETURN; diff --git a/src/lib/err/torerr_sys.c b/src/lib/err/torerr_sys.c index aa49ba36b..46fc85355 100644 --- a/src/lib/err/torerr_sys.c +++ b/src/lib/err/torerr_sys.c @@ -27,11 +27,9 @@ subsys_torerr_initialize(void) static void subsys_torerr_shutdown(void) { - /* Stop handling signals with backtraces, then close the logs. */ + /* Stop handling signals with backtraces, then flush 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(); + tor_log_flush_sigsafe_err_fds(); } const subsys_fns_t sys_torerr = { diff --git a/src/lib/log/log.c b/src/lib/log/log.c index 75f8f79da..4813a4fae 100644 --- a/src/lib/log/log.c +++ b/src/lib/log/log.c @@ -667,12 +667,9 @@ tor_log_update_sigsafe_err_fds(void) /* log_fds and err_fds contain matching entries: log_fds are the fds used by * the log module, and err_fds are the fds used by the err module. - * For stdio logs, the log_fd and err_fd values are identical, - * and the err module closes the fd on shutdown. - * For file logs, the err_fd is a dup() of the log_fd, - * and the log and err modules both close their respective fds on shutdown. - * (Once all fds representing a file are closed, the underlying file is - * closed.) + * For stdio logs, the log_fd and err_fd values are identical. + * For file logs, the err_fd is a dup() of the log_fd. + * Both the log and err modules flush these fds on shutdown. */ int log_fds[TOR_SIGSAFE_LOG_MAX_FDS]; int err_fds[TOR_SIGSAFE_LOG_MAX_FDS]; @@ -704,12 +701,12 @@ tor_log_update_sigsafe_err_fds(void) log_fds[n_fds] = lf->fd; if (lf->needs_close) { /* File log fds are duplicated, because close_log() closes the log - * module's fd, and tor_log_close_sigsafe_err_fds() closes the err + * module's fd, and tor_log_flush_sigsafe_err_fds() closes the err * module's fd. Both refer to the same file. */ err_fds[n_fds] = dup(lf->fd); } else { /* stdio log fds are not closed by the log module. - * tor_log_close_sigsafe_err_fds() closes stdio logs. */ + * tor_log_flush_sigsafe_err_fds() closes stdio logs. */ err_fds[n_fds] = lf->fd; } n_fds++; @@ -841,16 +838,16 @@ logs_free_all(void) * log mutex. */ } -/** Close signal-safe log files. - * Closing the log files makes the process and OS flush log buffers. +/** Flush the signal-safe log files. * - * 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. + * This function is safe to call from a signal handler. It is currenly called + * by the BUG() macros, when terminating the process on an abnormal condition. */ void -logs_close_sigsafe(void) +logs_flush_sigsafe(void) { + /* If we don't have fsync() in unistd.h, we can't flush the logs. */ +#ifdef HAVE_FSYNC 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. */ @@ -860,9 +857,11 @@ logs_close_sigsafe(void) victim = next; next = next->next; if (victim->needs_close) { - close_log_sigsafe(victim); + /* We can't do anything useful if the flush fails. */ + (void)fsync(victim->fd); } } +#endif } /** Remove and free the log entry <b>victim</b> from the linked-list diff --git a/src/lib/log/log.h b/src/lib/log/log.h index cb588635d..aafbf9be2 100644 --- a/src/lib/log/log.h +++ b/src/lib/log/log.h @@ -186,7 +186,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 logs_flush_sigsafe(void); void add_default_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 de44d30e4..581ae85f4 100644 --- a/src/lib/log/util_bug.c +++ b/src/lib/log/util_bug.c @@ -172,7 +172,7 @@ tor_bug_occurred_(const char *fname, unsigned int line, void tor_abort_(void) { - logs_close_sigsafe(); + logs_flush_sigsafe(); tor_raw_abort_(); }

1 0

[tor/release-0.4.3] Merge remote-tracking branch 'tor-github/pr/1723/head' into maint-0.4.3
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit c2aea6134a3333bbace155f37fcabf8a5b4e2744 Merge: 1ae0839ef e849881d3 Author: Nick Mathewson <nickm(a)torproject.org> Date: Thu Apr 9 08:30:14 2020 -0400 Merge remote-tracking branch 'tor-github/pr/1723/head' into maint-0.4.3 changes/bug33087 | 7 ++++++ configure.ac | 1 + src/lib/err/torerr.c | 26 ++++++++++----------- src/lib/err/torerr.h | 2 +- src/lib/err/torerr_sys.c | 6 ++--- src/lib/log/log.c | 60 +++++++++++++++++------------------------------- src/lib/log/log.h | 2 +- src/lib/log/util_bug.c | 2 +- 8 files changed, 46 insertions(+), 60 deletions(-)

1 0

[tor/release-0.4.3] Merge branch 'bug33087_042' into bug33087_043
by nickm＠torproject.org 09 Apr '20

09 Apr '20

commit 4f533ed28b831eb794f562dac9add1dbf37f6335 Merge: 8a23393ed b6d1f99cb Author: teor <teor(a)torproject.org> Date: Thu Feb 13 00:02:11 2020 +1000 Merge branch 'bug33087_042' into bug33087_043 "ours" merge, because the 0.4.2 backport deletes the close functions. But the 0.4.3 bug fix replaces them with fsync() where available.

1 0