From c8242e4c0ae838121ed00d5a38cf0dae052e9d13 Mon Sep 17 00:00:00 2001
From: teor <teor@torproject.org>
Date: Wed, 12 Feb 2020 12:47:15 +1000
Subject: [PATCH] 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(-)
 create mode 100644 changes/bug33087

diff --git a/changes/bug33087 b/changes/bug33087
new file mode 100644
index 0000000000..7acf72a835
--- /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 b7e32a3e20..5095c28ba9 100644
--- a/src/lib/err/torerr.c
+++ b/src/lib/err/torerr.c
@@ -150,32 +150,6 @@ tor_log_reset_sigsafe_err_fds(void)
   tor_log_set_sigsafe_err_fds(fds, 1);
 }
 
-/**
- * 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 0e839cb1ba..a1822d9c9a 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 a14c46f945..6a51a45a4d 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 ec7c2fa24e..69624e9cac 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 4291418eb6..c4a27782c3 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 0e99be35a4..640fe050e6 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_();
 }