[tor-commits] [tor/release-0.4.1] log: Don't close file log fds that are being used by the err module

[nickm@xxxxxxxxxxxxxx]

commit a22fbab98690f802ae3bda276078cc7fc767feba
Author: teor <teor@xxxxxxxxxxxxxx>
Date:   Wed Sep 4 15:38:58 2019 +1000

    log: Don't close file log fds that are being used by the err module
    
    Instead, dup() file log fds, before passing them to the err module.
    
    Closes 31613, part of 31594.
---
 src/lib/err/torerr.c |  6 ++++++
 src/lib/log/log.c    | 43 ++++++++++++++++++++++++++++++++++---------
 2 files changed, 40 insertions(+), 9 deletions(-)

diff --git a/src/lib/err/torerr.c b/src/lib/err/torerr.c
index 2c4a10a5b..f460fd837 100644
--- a/src/lib/err/torerr.c
+++ b/src/lib/err/torerr.c
@@ -111,6 +111,12 @@ tor_log_get_sigsafe_err_fds(const int **out)
  * 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
diff --git a/src/lib/log/log.c b/src/lib/log/log.c
index d21d8d1d4..2214d4b59 100644
--- a/src/lib/log/log.c
+++ b/src/lib/log/log.c
@@ -664,13 +664,24 @@ tor_log_update_sigsafe_err_fds(void)
   const logfile_t *lf;
   int found_real_stderr = 0;
 
-  int fds[TOR_SIGSAFE_LOG_MAX_FDS];
+  /* 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.)
+   */
+  int log_fds[TOR_SIGSAFE_LOG_MAX_FDS];
+  int err_fds[TOR_SIGSAFE_LOG_MAX_FDS];
   int n_fds;
 
   LOCK_LOGS();
   /* Reserve the first one for stderr. This is safe because when we daemonize,
-   * we dup2 /dev/null to stderr, */
-  fds[0] = STDERR_FILENO;
+   * we dup2 /dev/null to stderr.
+   * For stderr, log_fds and err_fds are the same. */
+  log_fds[0] = err_fds[0] = STDERR_FILENO;
   n_fds = 1;
 
   for (lf = logfiles; lf; lf = lf->next) {
@@ -684,25 +695,39 @@ tor_log_update_sigsafe_err_fds(void)
         (LD_BUG|LD_GENERAL)) {
       if (lf->fd == STDERR_FILENO)
         found_real_stderr = 1;
-      /* Avoid duplicates */
-      if (int_array_contains(fds, n_fds, lf->fd))
+      /* Avoid duplicates by checking the log module fd against log_fds */
+      if (int_array_contains(log_fds, n_fds, lf->fd))
         continue;
-      fds[n_fds++] = lf->fd;
+      /* Update log_fds using the log module's fd */
+      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.  */
+        err_fds[n_fds] = lf->fd;
+      }
+      n_fds++;
       if (n_fds == TOR_SIGSAFE_LOG_MAX_FDS)
         break;
     }
   }
 
   if (!found_real_stderr &&
-      int_array_contains(fds, n_fds, STDOUT_FILENO)) {
+      int_array_contains(log_fds, n_fds, STDOUT_FILENO)) {
     /* Don't use a virtual stderr when we're also logging to stdout. */
     raw_assert(n_fds >= 2); /* Don't tor_assert inside log fns */
-    fds[0] = fds[--n_fds];
+    --n_fds;
+    log_fds[0] = log_fds[n_fds];
+    err_fds[0] = err_fds[n_fds];
   }
 
   UNLOCK_LOGS();
 
-  tor_log_set_sigsafe_err_fds(fds, n_fds);
+  tor_log_set_sigsafe_err_fds(err_fds, n_fds);
 }
 
 /** Add to <b>out</b> a copy of every currently configured log file name. Used



_______________________________________________
tor-commits mailing list
tor-commits@xxxxxxxxxxxxxxxxxxxx
https://lists.torproject.org/cgi-bin/mailman/listinfo/tor-commits