Merge branch 'tor-github/pr/1303'

4 files changed, 70 insertions, 8 deletions

diff --git a/src/lib/err/backtrace.c b/src/lib/err/backtrace.c
index 75d5093c54..c2011285c0 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 48fcf35e06..0a4ee5d417 100644
--- a/src/lib/err/torerr.c
+++ b/src/lib/err/torerr.c
@@ -110,6 +110,14 @@ 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.
+ *
+ * These fds must remain open even after the log module has shut down. (And
+ * they should remain open even while logs are being reconfigured.) Therefore,
+ * any fds closed by the log module should be dup()ed, and the duplicate fd
+ * should be given to the err module in fds. In particular, the log module
+ * closes the file log fds, but does not close the stdio log 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 +126,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 +151,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.
  */
@@ -171,6 +215,18 @@ tor_raw_assertion_failed_msg_(const char *file, int line, const char *expr,
   tor_log_err_sigsafe_write("\n");
 }
 
+/**
+ * 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
@@ -205,7 +261,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];
@@ -214,7 +270,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 c2da6697a9..0e839cb1ba 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,
@@ -40,8 +40,11 @@ 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;
+
 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/err/torerr_sys.c b/src/lib/err/torerr_sys.c
index 34f70f1f0b..eb818004fb 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 = {