]> git.apps.os.sepia.ceph.com Git - ceph.git/commitdiff
global/global_init: fix stdout/stderr/stdin closing for daemonization 23024/head
authorSage Weil <sage@redhat.com>
Mon, 9 Jul 2018 18:26:39 +0000 (13:26 -0500)
committerNathan Cutler <ncutler@suse.com>
Fri, 13 Jul 2018 11:20:21 +0000 (13:20 +0200)
The global_init_postfork/prefork helpers close stdout/stdin/stderr on
fork and reopen /dev/null in their place.  This ensures that if later
code writes to those descriptors (e.g., a stray cout or cerr usage) the
output/input will go nowhere instead of interfering with some other open
fd.

However, with the use of preforker, there are other threads running when
these helpers are run, which means we can race with, say, filestore
opening an object file and end up sending log output there.

Fix by atomically replacing the fds with the dup2(2) syscall, which
will implicitly close and reopen the target fd in an atomic fashion.  This
behavior is present on both Linux and FreeBSD.

Fixes: http://tracker.ceph.com/issues/23492
Signed-off-by: Sage Weil <sage@redhat.com>
(cherry picked from commit 3c2d91ed11c19ee8769acef8268938d2cab59152)

src/global/global_init.cc

index a990d5d3e7d6a776435f4b9961cf645d61fe9134..8fd9247b0421a885be4034f9ef4abbfe60e0cd65 100644 (file)
@@ -425,6 +425,29 @@ void global_init_daemonize(CephContext *cct)
 #endif
 }
 
+int reopen_as_null(CephContext *cct, int fd)
+{
+  int newfd = open("/dev/null", O_RDONLY);
+  if (newfd < 0) {
+    int err = errno;
+    lderr(cct) << __func__ << " failed to open /dev/null: " << cpp_strerror(err)
+              << dendl;
+    return -1;
+  }
+  // atomically dup newfd to target fd.  target fd is implicitly closed if
+  // open and atomically replaced; see man dup2
+  int r = dup2(newfd, fd);
+  if (r < 0) {
+    int err = errno;
+    lderr(cct) << __func__ << " failed to dup2 " << fd << ": "
+              << cpp_strerror(err) << dendl;
+    return -1;
+  }
+  // close newfd (we cloned it to target fd)
+  VOID_TEMP_FAILURE_RETRY(close(newfd));
+  return 0;
+}
+
 void global_init_postfork_start(CephContext *cct)
 {
   // restart log thread
@@ -439,13 +462,7 @@ void global_init_postfork_start(CephContext *cct)
    * guarantee that nobody ever writes to stdout, even though they're not
    * supposed to.
    */
-  VOID_TEMP_FAILURE_RETRY(close(STDIN_FILENO));
-  if (open("/dev/null", O_RDONLY) < 0) {
-    int err = errno;
-    derr << "global_init_daemonize: open(/dev/null) failed: error "
-        << err << dendl;
-    exit(1);
-  }
+  reopen_as_null(cct, STDIN_FILENO);
 
   const md_config_t *conf = cct->_conf;
   if (pidfile_write(conf) < 0)
@@ -473,13 +490,7 @@ void global_init_postfork_finish(CephContext *cct)
     }
   }
 
-  VOID_TEMP_FAILURE_RETRY(close(STDOUT_FILENO));
-  if (open("/dev/null", O_RDONLY) < 0) {
-    int err = errno;
-    derr << "global_init_daemonize: open(/dev/null) failed: error "
-        << err << dendl;
-    exit(1);
-  }
+  reopen_as_null(cct, STDOUT_FILENO);
 
   ldout(cct, 1) << "finished global_init_daemonize" << dendl;
 }
@@ -503,13 +514,7 @@ void global_init_chdir(const CephContext *cct)
  */
 int global_init_shutdown_stderr(CephContext *cct)
 {
-  VOID_TEMP_FAILURE_RETRY(close(STDERR_FILENO));
-  if (open("/dev/null", O_RDONLY) < 0) {
-    int err = errno;
-    derr << "global_init_shutdown_stderr: open(/dev/null) failed: error "
-        << err << dendl;
-    return 1;
-  }
+  reopen_as_null(cct, STDERR_FILENO);
   cct->_log->set_stderr_level(-1, -1);
   return 0;
 }