From: Sage Weil Date: Mon, 9 Jul 2018 18:26:39 +0000 (-0500) Subject: global/global_init: fix stdout/stderr/stdin closing for daemonization X-Git-Tag: v13.2.1~28^2 X-Git-Url: http://git-server-git.apps.pok.os.sepia.ceph.com/?a=commitdiff_plain;h=035cff29728754cf122a5151ae4dc064e178530d;p=ceph.git global/global_init: fix stdout/stderr/stdin closing for daemonization 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 (cherry picked from commit 3c2d91ed11c19ee8769acef8268938d2cab59152) --- diff --git a/src/global/global_init.cc b/src/global/global_init.cc index a990d5d3e7d..8fd9247b042 100644 --- a/src/global/global_init.cc +++ b/src/global/global_init.cc @@ -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; }