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

+  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.

configure.ac

@@ -649,6 +649,7 @@ AC_CHECK_FUNCS(
 	explicit_bzero \
 	timingsafe_memcmp \
 	flock \
+	fsync \
 	ftime \
 	get_current_dir_name \
 	getaddrinfo \

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();
 }
 

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;

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 = {

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

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);

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_();
 }