Commit c2aea613 authored by Nick Mathewson's avatar Nick Mathewson 🥄
Browse files

Merge remote-tracking branch 'tor-github/pr/1723/head' into maint-0.4.3

parents 1ae0839e e849881d
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.
...@@ -649,6 +649,7 @@ AC_CHECK_FUNCS( ...@@ -649,6 +649,7 @@ AC_CHECK_FUNCS(
explicit_bzero \ explicit_bzero \
timingsafe_memcmp \ timingsafe_memcmp \
flock \ flock \
fsync \
ftime \ ftime \
get_current_dir_name \ get_current_dir_name \
getaddrinfo \ getaddrinfo \
......
...@@ -151,29 +151,27 @@ tor_log_reset_sigsafe_err_fds(void) ...@@ -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: * 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 * This function is safe to call during signal handlers.
* process shutdown.
*/ */
void 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; int n_fds, i;
const int *fds = NULL; const int *fds = NULL;
n_fds = tor_log_get_sigsafe_err_fds(&fds); n_fds = tor_log_get_sigsafe_err_fds(&fds);
for (i = 0; i < n_fds; ++i) { for (i = 0; i < n_fds; ++i) {
/* tor_log_close_sigsafe_err_fds_on_error() is called on error and on /* This function is called on error and on shutdown, so we don't log, or
* shutdown, so we can't log or take any useful action if close() * take any other action, if fsync() fails. */
* fails. */ (void)fsync(fds[i]);
(void)close(fds[i]);
} }
#endif /* defined(HAVE_FSYNC) */
/* Don't even try logging, we've closed all the log fds. */
tor_log_set_sigsafe_err_fds(NULL, 0);
} }
/** /**
...@@ -217,13 +215,13 @@ tor_raw_assertion_failed_msg_(const char *file, int line, const char *expr, ...@@ -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 * 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. * are written before process termination.
**/ **/
void void
tor_raw_abort_(void) tor_raw_abort_(void)
{ {
tor_log_close_sigsafe_err_fds(); tor_log_flush_sigsafe_err_fds();
abort(); abort();
} }
......
...@@ -40,7 +40,7 @@ void tor_log_err_sigsafe(const char *m, ...); ...@@ -40,7 +40,7 @@ void tor_log_err_sigsafe(const char *m, ...);
int tor_log_get_sigsafe_err_fds(const int **out); 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_set_sigsafe_err_fds(const int *fds, int n);
void tor_log_reset_sigsafe_err_fds(void); 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_log_sigsafe_err_set_granularity(int ms);
void tor_raw_abort_(void) ATTR_NORETURN; void tor_raw_abort_(void) ATTR_NORETURN;
......
...@@ -27,11 +27,9 @@ subsys_torerr_initialize(void) ...@@ -27,11 +27,9 @@ subsys_torerr_initialize(void)
static void static void
subsys_torerr_shutdown(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(); clean_up_backtrace_handler();
/* We can't log any log messages after this point: we've closed all the log tor_log_flush_sigsafe_err_fds();
* fds, including stdio. */
tor_log_close_sigsafe_err_fds();
} }
const subsys_fns_t sys_torerr = { const subsys_fns_t sys_torerr = {
......
...@@ -665,24 +665,15 @@ tor_log_update_sigsafe_err_fds(void) ...@@ -665,24 +665,15 @@ tor_log_update_sigsafe_err_fds(void)
const logfile_t *lf; const logfile_t *lf;
int found_real_stderr = 0; int found_real_stderr = 0;
/* log_fds and err_fds contain matching entries: log_fds are the fds used by /* The fds are the file descriptors of tor's stdout, stderr, and file
* the log module, and err_fds are the fds used by the err module. * logs. The log and err modules flush these fds during their shutdowns. */
* For stdio logs, the log_fd and err_fd values are identical, int fds[TOR_SIGSAFE_LOG_MAX_FDS];
* 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.)
*/
int log_fds[TOR_SIGSAFE_LOG_MAX_FDS];
int err_fds[TOR_SIGSAFE_LOG_MAX_FDS];
int n_fds; int n_fds;
LOCK_LOGS(); LOCK_LOGS();
/* Reserve the first one for stderr. This is safe because when we daemonize, /* Reserve the first one for stderr. This is safe because when we daemonize,
* we dup2 /dev/null to stderr. * we dup2 /dev/null to stderr. */
* For stderr, log_fds and err_fds are the same. */ fds[0] = STDERR_FILENO;
log_fds[0] = err_fds[0] = STDERR_FILENO;
n_fds = 1; n_fds = 1;
for (lf = logfiles; lf; lf = lf->next) { for (lf = logfiles; lf; lf = lf->next) {
...@@ -697,21 +688,11 @@ tor_log_update_sigsafe_err_fds(void) ...@@ -697,21 +688,11 @@ tor_log_update_sigsafe_err_fds(void)
(LD_BUG|LD_GENERAL)) { (LD_BUG|LD_GENERAL)) {
if (lf->fd == STDERR_FILENO) if (lf->fd == STDERR_FILENO)
found_real_stderr = 1; found_real_stderr = 1;
/* Avoid duplicates by checking the log module fd against log_fds */ /* Avoid duplicates by checking the log module fd against fds */
if (int_array_contains(log_fds, n_fds, lf->fd)) if (int_array_contains(fds, n_fds, lf->fd))
continue; continue;
/* Update log_fds using the log module's fd */ /* Update fds using the log module's fd */
log_fds[n_fds] = lf->fd; 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. */
err_fds[n_fds] = lf->fd;
}
n_fds++; n_fds++;
if (n_fds == TOR_SIGSAFE_LOG_MAX_FDS) if (n_fds == TOR_SIGSAFE_LOG_MAX_FDS)
break; break;
...@@ -719,20 +700,19 @@ tor_log_update_sigsafe_err_fds(void) ...@@ -719,20 +700,19 @@ tor_log_update_sigsafe_err_fds(void)
} }
if (!found_real_stderr && if (!found_real_stderr &&
int_array_contains(log_fds, n_fds, STDOUT_FILENO)) { int_array_contains(fds, n_fds, STDOUT_FILENO)) {
/* Don't use a virtual stderr when we're also logging to stdout. /* Don't use a virtual stderr when we're also logging to stdout.
* If we reached max_fds logs, we'll now have (max_fds - 1) logs. * If we reached max_fds logs, we'll now have (max_fds - 1) logs.
* That's ok, max_fds is large enough that most tor instances don't exceed * That's ok, max_fds is large enough that most tor instances don't exceed
* it. */ * it. */
raw_assert(n_fds >= 2); /* Don't tor_assert inside log fns */ raw_assert(n_fds >= 2); /* Don't tor_assert inside log fns */
--n_fds; --n_fds;
log_fds[0] = log_fds[n_fds]; fds[0] = fds[n_fds];
err_fds[0] = err_fds[n_fds];
} }
UNLOCK_LOGS(); UNLOCK_LOGS();
tor_log_set_sigsafe_err_fds(err_fds, n_fds); tor_log_set_sigsafe_err_fds(fds, n_fds);
} }
/** Add to <b>out</b> a copy of every currently configured log file name. Used /** Add to <b>out</b> a copy of every currently configured log file name. Used
...@@ -841,16 +821,16 @@ logs_free_all(void) ...@@ -841,16 +821,16 @@ logs_free_all(void)
* log mutex. */ * log mutex. */
} }
/** Close signal-safe log files. /** Flush the 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 * This function is safe to call from a signal handler. It is currenly called
* called when shutting down the log or err modules. It is currenly called * by the BUG() macros, when terminating the process on an abnormal condition.
* by the err module, when terminating the process on an abnormal condition.
*/ */
void 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; logfile_t *victim, *next;
/* We can't LOCK_LOGS() in a signal handler, because it may call /* We can't LOCK_LOGS() in a signal handler, because it may call
* signal-unsafe functions. And we can't deallocate memory, either. */ * signal-unsafe functions. And we can't deallocate memory, either. */
...@@ -860,9 +840,11 @@ logs_close_sigsafe(void) ...@@ -860,9 +840,11 @@ logs_close_sigsafe(void)
victim = next; victim = next;
next = next->next; next = next->next;
if (victim->needs_close) { if (victim->needs_close) {
close_log_sigsafe(victim); /* We can't do anything useful if the flush fails. */
(void)fsync(victim->fd);
} }
} }
#endif /* defined(HAVE_FSYNC) */
} }
/** Remove and free the log entry <b>victim</b> from the linked-list /** Remove and free the log entry <b>victim</b> from the linked-list
......
...@@ -186,7 +186,7 @@ void logs_set_domain_logging(int enabled); ...@@ -186,7 +186,7 @@ void logs_set_domain_logging(int enabled);
int get_min_log_level(void); int get_min_log_level(void);
void switch_logs_debug(void); void switch_logs_debug(void);
void logs_free_all(void); void logs_free_all(void);
void logs_close_sigsafe(void); void logs_flush_sigsafe(void);
void add_default_log(int min_severity); void add_default_log(int min_severity);
void close_temp_logs(void); void close_temp_logs(void);
void rollback_log_changes(void); void rollback_log_changes(void);
......
...@@ -172,7 +172,7 @@ tor_bug_occurred_(const char *fname, unsigned int line, ...@@ -172,7 +172,7 @@ tor_bug_occurred_(const char *fname, unsigned int line,
void void
tor_abort_(void) tor_abort_(void)
{ {
logs_close_sigsafe(); logs_flush_sigsafe();
tor_raw_abort_(); tor_raw_abort_();
} }
......
Supports Markdown
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment