From a7b0cb5f7e790b1e9904a933dbdeedfaf5b78357 Mon Sep 17 00:00:00 2001 From: Gunnar Beutner Date: Thu, 29 Sep 2016 12:35:10 +0200 Subject: [PATCH] Ensure we don't leak file descriptors to child processes fixes #8900 --- doc/17-language-reference.md | 1 - doc/20-development.md | 3 +- icinga-app/icinga.cpp | 10 +- lib/base/process.cpp | 516 +++++++++++++++++++++++++++-------- lib/base/process.hpp | 4 + 5 files changed, 406 insertions(+), 128 deletions(-) diff --git a/doc/17-language-reference.md b/doc/17-language-reference.md index b29d96de0..2e41098f0 100644 --- a/doc/17-language-reference.md +++ b/doc/17-language-reference.md @@ -381,7 +381,6 @@ ObjectsPath |**Read-write.** Contains the path of the Icinga 2 objects f PidPath |**Read-write.** Contains the path of the Icinga 2 PID file. Defaults to RunDir + "/icinga2/icinga2.pid". Vars |**Read-write.** Contains a dictionary with global custom attributes. Not set by default. NodeName |**Read-write.** Contains the cluster node name. Set to the local hostname by default. -UseVfork |**Read-write.** Whether to use vfork(). Only available on *NIX. Defaults to true. EventEngine |**Read-write.** The name of the socket event engine, can be "poll" or "epoll". The epoll interface is only supported on Linux. AttachDebugger |**Read-write.** Whether to attach a debugger when Icinga 2 crashes. Defaults to false. RunAsUser |**Read-write.** Defines the user the Icinga 2 daemon is running as. Used in the `init.conf` configuration file. diff --git a/doc/20-development.md b/doc/20-development.md index 3e0ea545c..9c7aea3f2 100644 --- a/doc/20-development.md +++ b/doc/20-development.md @@ -118,9 +118,8 @@ the duplicate import in your `~/.gdbinit` file. Call GDB with the binary (`/usr/sbin/icinga2` is a wrapper script calling `/usr/lib64/icinga2/sbin/icinga2` since 2.4) and all arguments and run it in foreground. -If VFork causes trouble, disable it inside the gdb run. - # gdb --args /usr/lib64/icinga2/sbin/icinga2 daemon -x debug -DUseVfork=0 --no-stack-rlimit + # gdb --args /usr/lib64/icinga2/sbin/icinga2 daemon -x debug --no-stack-rlimit The exact path to the Icinga 2 binary differs on each distribution. On Ubuntu it is installed into `/usr/lib/x86_64-linux-gnu/icinga2/sbin/icinga2` on 64-bit systems diff --git a/icinga-app/icinga.cpp b/icinga-app/icinga.cpp index 6299429ee..f07508f5a 100644 --- a/icinga-app/icinga.cpp +++ b/icinga-app/icinga.cpp @@ -31,6 +31,7 @@ #include "base/scriptglobal.hpp" #include "base/context.hpp" #include "base/console.hpp" +#include "base/process.hpp" #include "config.h" #include #include @@ -153,13 +154,6 @@ int Main(void) Application::DeclareRunAsGroup(ICINGA_GROUP); Application::DeclareConcurrency(boost::thread::hardware_concurrency()); - if (!ScriptGlobal::Exists("UseVfork")) -#ifdef __APPLE__ - ScriptGlobal::Set("UseVfork", false); -#else /* __APPLE__ */ - ScriptGlobal::Set("UseVfork", true); -#endif /* __APPLE__ */ - ScriptGlobal::Set("AttachDebugger", false); ScriptGlobal::Set("PlatformKernel", Utility::GetPlatformKernel()); @@ -454,6 +448,8 @@ int Main(void) } } } + + Process::InitializeSpawnHelper(); #endif /* _WIN32 */ std::vector args; diff --git a/lib/base/process.cpp b/lib/base/process.cpp index a2bba4d32..ddd0644dd 100644 --- a/lib/base/process.cpp +++ b/lib/base/process.cpp @@ -27,6 +27,7 @@ #include "base/logger.hpp" #include "base/utility.hpp" #include "base/scriptglobal.hpp" +#include "base/json.hpp" #include #include @@ -53,8 +54,13 @@ static HANDLE l_Events[IOTHREADS]; #else /* _WIN32 */ static int l_EventFDs[IOTHREADS][2]; static std::map l_FDs[IOTHREADS]; + +static boost::mutex l_ProcessControlMutex; +static int l_ProcessControlFD = -1; +static pid_t l_ProcessControlPID; #endif /* _WIN32 */ -static boost::once_flag l_OnceFlag = BOOST_ONCE_INIT; +static boost::once_flag l_ProcessOnceFlag = BOOST_ONCE_INIT; +static boost::once_flag l_SpawnHelperOnceFlag = BOOST_ONCE_INIT; Process::Process(const Process::Arguments& arguments, const Dictionary::Ptr& extraEnvironment) : m_Arguments(arguments), m_ExtraEnvironment(extraEnvironment), m_Timeout(600) @@ -74,6 +80,375 @@ Process::~Process(void) #endif /* _WIN32 */ } +#ifndef _WIN32 +static Value ProcessSpawnImpl(struct msghdr *msgh, const Dictionary::Ptr& request) +{ + struct cmsghdr *cmsg = CMSG_FIRSTHDR(msgh); + + if (cmsg == NULL || cmsg->cmsg_level != SOL_SOCKET || cmsg->cmsg_len != CMSG_LEN(sizeof(int) * 3)) { + std::cerr << "Invalid 'spawn' request: FDs missing" << std::endl; + return Empty; + } + + int *fds = (int *)CMSG_DATA(cmsg); + + Array::Ptr arguments = request->Get("arguments"); + Dictionary::Ptr extraEnvironment = request->Get("extraEnvironment"); + + // build argv + char **argv = new char *[arguments->GetLength() + 1]; + + for (unsigned int i = 0; i < arguments->GetLength(); i++) { + String arg = arguments->Get(i); + argv[i] = strdup(arg.CStr()); + } + + argv[arguments->GetLength()] = NULL; + + // build envp + int envc = 0; + + /* count existing environment variables */ + while (environ[envc] != NULL) + envc++; + + char **envp = new char *[envc + (extraEnvironment ? extraEnvironment->GetLength() : 0) + 2]; + + for (int i = 0; i < envc; i++) + envp[i] = strdup(environ[i]); + + if (extraEnvironment) { + ObjectLock olock(extraEnvironment); + + int index = envc; + for (const Dictionary::Pair& kv : extraEnvironment) { + String skv = kv.first + "=" + Convert::ToString(kv.second); + envp[index] = strdup(skv.CStr()); + index++; + } + } + + envp[envc + (extraEnvironment ? extraEnvironment->GetLength() : 0)] = strdup("LC_NUMERIC=C"); + envp[envc + (extraEnvironment ? extraEnvironment->GetLength() : 0) + 1] = NULL; + + extraEnvironment.reset(); + + pid_t pid = fork(); + + if (pid < 0) { + BOOST_THROW_EXCEPTION(posix_error() + << boost::errinfo_api_function("fork") + << boost::errinfo_errno(errno)); + } + + if (pid == 0) { + // child process + + (void)close(l_ProcessControlFD); + + if (setsid() < 0) { + perror("setsid() failed"); + _exit(128); + } + + if (dup2(fds[0], STDIN_FILENO) < 0 || dup2(fds[1], STDOUT_FILENO) < 0 || dup2(fds[2], STDERR_FILENO) < 0) { + perror("dup2() failed"); + _exit(128); + } + + (void)close(fds[0]); + (void)close(fds[1]); + (void)close(fds[2]); + +#ifdef HAVE_NICE + if (nice(5) < 0) + Log(LogWarning, "base", "Failed to renice child process."); +#endif /* HAVE_NICE */ + + if (icinga2_execvpe(argv[0], argv, envp) < 0) { + char errmsg[512]; + strcpy(errmsg, "execvpe("); + strncat(errmsg, argv[0], sizeof(errmsg) - strlen(errmsg) - 1); + strncat(errmsg, ") failed", sizeof(errmsg) - strlen(errmsg) - 1); + errmsg[sizeof(errmsg) - 1] = '\0'; + perror(errmsg); + _exit(128); + } + + _exit(128); + } + + (void)close(fds[0]); + (void)close(fds[1]); + (void)close(fds[2]); + + // free arguments + for (int i = 0; argv[i] != NULL; i++) + free(argv[i]); + + delete[] argv; + + // free environment + for (int i = 0; envp[i] != NULL; i++) + free(envp[i]); + + delete[] envp; + + Dictionary::Ptr response = new Dictionary(); + response->Set("rc", pid); + return response; +} + +static Value ProcessKillImpl(struct msghdr *msgh, const Dictionary::Ptr& request) +{ + pid_t pid = request->Get("pid"); + int signum = request->Get("signum"); + + int rc = kill(pid, signum); + + Dictionary::Ptr response = new Dictionary(); + response->Set("rc", rc); + + return response; +} + +static Value ProcessWaitPIDImpl(struct msghdr *msgh, const Dictionary::Ptr& request) +{ + pid_t pid = request->Get("pid"); + + int status; + int rc = waitpid(pid, &status, 0); + + Dictionary::Ptr response = new Dictionary(); + response->Set("status", status); + response->Set("rc", rc); + + return response; +} + +static void ProcessHandler(void) +{ + sigset_t mask; + sigfillset(&mask); + sigprocmask(SIG_SETMASK, &mask, NULL); + + rlimit rl; + if (getrlimit(RLIMIT_NOFILE, &rl) >= 0) { + rlim_t maxfds = rl.rlim_max; + + if (maxfds == RLIM_INFINITY) + maxfds = 65536; + + for (rlim_t i = 3; i < maxfds; i++) + if (i != static_cast(l_ProcessControlFD)) + (void)close(i); + } + + for (;;) { + struct msghdr msg; + memset(&msg, 0, sizeof(msg)); + + char mbuf[4096]; + struct iovec io; + io.iov_base = mbuf; + io.iov_len = sizeof(mbuf); + + msg.msg_iov = &io; + msg.msg_iovlen = 1; + + char cbuf[4096]; + msg.msg_control = cbuf; + msg.msg_controllen = sizeof(cbuf); + + int rc = recvmsg(l_ProcessControlFD, &msg, 0); + + if (rc <= 0) { + if (rc < 0 && (errno == EINTR || errno == EAGAIN)) + continue; + + break; + } + + String jrequest = String(mbuf, mbuf + rc); + + Dictionary::Ptr request = JsonDecode(jrequest); + + String command = request->Get("command"); + + Value response; + + if (command == "spawn") + response = ProcessSpawnImpl(&msg, request); + else if (command == "waitpid") + response = ProcessWaitPIDImpl(&msg, request); + else if (command == "kill") + response = ProcessKillImpl(&msg, request); + else + response = Empty; + + String jresponse = JsonEncode(response); + + if (send(l_ProcessControlFD, jresponse.CStr(), jresponse.GetLength(), 0) < 0) { + BOOST_THROW_EXCEPTION(posix_error() + << boost::errinfo_api_function("send") + << boost::errinfo_errno(errno)); + } + } + + _exit(0); +} + +static void StartSpawnProcessHelper(void) +{ + if (l_ProcessControlFD != -1) { + (void)close(l_ProcessControlFD); + + int status; + (void)waitpid(l_ProcessControlPID, &status, 0); + } + + int controlFDs[2]; + if (socketpair(AF_UNIX, SOCK_STREAM, 0, controlFDs) < 0) { + BOOST_THROW_EXCEPTION(posix_error() + << boost::errinfo_api_function("socketpair") + << boost::errinfo_errno(errno)); + } + + pid_t pid = fork(); + + if (pid < 0) { + BOOST_THROW_EXCEPTION(posix_error() + << boost::errinfo_api_function("fork") + << boost::errinfo_errno(errno)); + } + + if (pid == 0) { + (void)close(controlFDs[1]); + + l_ProcessControlFD = controlFDs[0]; + + ProcessHandler(); + + _exit(1); + } + + (void)close(controlFDs[0]); + + l_ProcessControlFD = controlFDs[1]; + l_ProcessControlPID = pid; +} + +static pid_t ProcessSpawn(const std::vector& arguments, const Dictionary::Ptr& extraEnvironment, int fds[3]) +{ + Dictionary::Ptr request = new Dictionary(); + request->Set("command", "spawn"); + request->Set("arguments", Array::FromVector(arguments)); + request->Set("extraEnvironment", extraEnvironment); + + String jrequest = JsonEncode(request); + + boost::mutex::scoped_lock lock(l_ProcessControlMutex); + + struct msghdr msg; + memset(&msg, 0, sizeof(msg)); + + struct iovec io; + io.iov_base = const_cast(jrequest.CStr()); + io.iov_len = jrequest.GetLength(); + + msg.msg_iov = &io; + msg.msg_iovlen = 1; + + char cbuf[CMSG_SPACE(sizeof(int) * 3)]; + msg.msg_control = cbuf; + msg.msg_controllen = sizeof(cbuf); + + struct cmsghdr *cmsg = CMSG_FIRSTHDR(&msg); + cmsg->cmsg_level = SOL_SOCKET; + cmsg->cmsg_type = SCM_RIGHTS; + cmsg->cmsg_len = CMSG_LEN(sizeof(int) * 3); + + memcpy(CMSG_DATA(cmsg), fds, sizeof(int) * 3); + + msg.msg_controllen = cmsg->cmsg_len; + + while (sendmsg(l_ProcessControlFD, &msg, 0) < 0) + StartSpawnProcessHelper(); + + char buf[4096]; + + ssize_t rc = recv(l_ProcessControlFD, buf, sizeof(buf), 0); + + if (rc < 0) + return -1; + + String jresponse = String(buf, buf + rc); + + Dictionary::Ptr response = JsonDecode(jresponse); + return response->Get("rc"); +} + +static int ProcessKill(pid_t pid, int signum) +{ + Dictionary::Ptr request = new Dictionary(); + request->Set("command", "kill"); + request->Set("pid", pid); + request->Set("signum", signum); + + String jrequest = JsonEncode(request); + + boost::mutex::scoped_lock lock(l_ProcessControlMutex); + + while (send(l_ProcessControlFD, jrequest.CStr(), jrequest.GetLength(), 0) < 0) + StartSpawnProcessHelper(); + + char buf[4096]; + + ssize_t rc = recv(l_ProcessControlFD, buf, sizeof(buf), 0); + + if (rc < 0) + return -1; + + String jresponse = String(buf, buf + rc); + + Dictionary::Ptr response = JsonDecode(jresponse); + return response->Get("rc"); +} + +static int ProcessWaitPID(pid_t pid, int *status) +{ + Dictionary::Ptr request = new Dictionary(); + request->Set("command", "waitpid"); + request->Set("pid", pid); + + String jrequest = JsonEncode(request); + + boost::mutex::scoped_lock lock(l_ProcessControlMutex); + + while (send(l_ProcessControlFD, jrequest.CStr(), jrequest.GetLength(), 0) < 0) + StartSpawnProcessHelper(); + + char buf[4096]; + + ssize_t rc = recv(l_ProcessControlFD, buf, sizeof(buf), 0); + + if (rc < 0) + return -1; + + String jresponse = String(buf, buf + rc); + + Dictionary::Ptr response = JsonDecode(jresponse); + *status = response->Get("status"); + return response->Get("rc"); +} + +void Process::InitializeSpawnHelper(void) +{ + if (l_ProcessControlFD == -1) + StartSpawnProcessHelper(); +} +#endif /* _WIN32 */ + static void InitializeProcess(void) { for (int tid = 0; tid < IOTHREADS; tid++) { @@ -351,7 +726,8 @@ static BOOL CreatePipeOverlapped(HANDLE *outReadPipe, HANDLE *outWritePipe, void Process::Run(const boost::function& callback) { - boost::call_once(l_OnceFlag, &Process::ThreadInitialize); + boost::call_once(l_SpawnHelperOnceFlag, &Process::InitializeSpawnHelper); + boost::call_once(l_ProcessOnceFlag, &Process::ThreadInitialize); m_Result.ExecutionStart = Utility::GetTime(); @@ -510,20 +886,20 @@ void Process::Run(const boost::function& callback) << "Running command " << PrettyPrintArguments(m_Arguments) << ": PID " << m_PID; #else /* _WIN32 */ - int fds[2]; + int outfds[2]; #ifdef HAVE_PIPE2 - if (pipe2(fds, O_CLOEXEC) < 0) { + if (pipe2(outfds, O_CLOEXEC) < 0) { if (errno == ENOSYS) { #endif /* HAVE_PIPE2 */ - if (pipe(fds) < 0) { + if (pipe(outfds) < 0) { BOOST_THROW_EXCEPTION(posix_error() << boost::errinfo_api_function("pipe") << boost::errinfo_errno(errno)); } - Utility::SetCloExec(fds[0]); - Utility::SetCloExec(fds[1]); + Utility::SetCloExec(outfds[0]); + Utility::SetCloExec(outfds[1]); #ifdef HAVE_PIPE2 } else { BOOST_THROW_EXCEPTION(posix_error() @@ -533,117 +909,22 @@ void Process::Run(const boost::function& callback) } #endif /* HAVE_PIPE2 */ - // build argv - char **argv = new char *[m_Arguments.size() + 1]; - - for (unsigned int i = 0; i < m_Arguments.size(); i++) - argv[i] = strdup(m_Arguments[i].CStr()); - - argv[m_Arguments.size()] = NULL; - - // build envp - int envc = 0; - - /* count existing environment variables */ - while (environ[envc] != NULL) - envc++; - - char **envp = new char *[envc + (m_ExtraEnvironment ? m_ExtraEnvironment->GetLength() : 0) + 2]; - - for (int i = 0; i < envc; i++) - envp[i] = strdup(environ[i]); - - if (m_ExtraEnvironment) { - ObjectLock olock(m_ExtraEnvironment); - - int index = envc; - for (const Dictionary::Pair& kv : m_ExtraEnvironment) { - String skv = kv.first + "=" + Convert::ToString(kv.second); - envp[index] = strdup(skv.CStr()); - index++; - } - } - - envp[envc + (m_ExtraEnvironment ? m_ExtraEnvironment->GetLength() : 0)] = strdup("LC_NUMERIC=C"); - envp[envc + (m_ExtraEnvironment ? m_ExtraEnvironment->GetLength() : 0) + 1] = NULL; - - m_ExtraEnvironment.reset(); - -#ifdef HAVE_VFORK - Value use_vfork = ScriptGlobal::Get("UseVfork"); - - if (use_vfork.IsEmpty() || static_cast(use_vfork)) - m_Process = vfork(); - else - m_Process = fork(); -#else /* HAVE_VFORK */ - m_Process = fork(); -#endif /* HAVE_VFORK */ - - if (m_Process < 0) { - BOOST_THROW_EXCEPTION(posix_error() - << boost::errinfo_api_function("fork") - << boost::errinfo_errno(errno)); - } - - if (m_Process == 0) { - // child process - - if (setsid() < 0) { - perror("setsid() failed"); - _exit(128); - } - - if (dup2(fds[1], STDOUT_FILENO) < 0 || dup2(fds[1], STDERR_FILENO) < 0) { - perror("dup2() failed"); - _exit(128); - } - - (void)close(fds[0]); - (void)close(fds[1]); - -#ifdef HAVE_NICE - if (nice(5) < 0) - Log(LogWarning, "base", "Failed to renice child process."); -#endif /* HAVE_NICE */ - - if (icinga2_execvpe(argv[0], argv, envp) < 0) { - char errmsg[512]; - strcpy(errmsg, "execvpe("); - strncat(errmsg, argv[0], sizeof(errmsg) - strlen(errmsg) - 1); - strncat(errmsg, ") failed", sizeof(errmsg) - strlen(errmsg) - 1); - errmsg[sizeof(errmsg) - 1] = '\0'; - perror(errmsg); - _exit(128); - } - - _exit(128); - } - - // parent process + int fds[3]; + fds[0] = STDIN_FILENO; + fds[1] = outfds[1]; + fds[2] = outfds[1]; + m_Process = ProcessSpawn(m_Arguments, m_ExtraEnvironment, fds); m_PID = m_Process; Log(LogNotice, "Process") - << "Running command " << PrettyPrintArguments(m_Arguments) <<": PID " << m_PID; - - // free arguments - for (int i = 0; argv[i] != NULL; i++) - free(argv[i]); - - delete[] argv; - - // free environment - for (int i = 0; envp[i] != NULL; i++) - free(envp[i]); - - delete[] envp; + << "Running command " << PrettyPrintArguments(m_Arguments) << ": PID " << m_PID; - (void)close(fds[1]); + (void)close(outfds[1]); - Utility::SetNonBlocking(fds[0]); + Utility::SetNonBlocking(outfds[0]); - m_FD = fds[0]; + m_FD = outfds[0]; #endif /* _WIN32 */ m_Callback = callback; @@ -682,7 +963,7 @@ bool Process::DoEvents(void) #ifdef _WIN32 TerminateProcess(m_Process, 1); #else /* _WIN32 */ - kill(-m_Process, SIGKILL); + ProcessKill(-m_Process, SIGKILL); #endif /* _WIN32 */ is_timeout = true; @@ -728,13 +1009,12 @@ bool Process::DoEvents(void) << "PID " << m_PID << " (" << PrettyPrintArguments(m_Arguments) << ") terminated with exit code " << exitcode; #else /* _WIN32 */ int status, exitcode; - if (waitpid(m_Process, &status, 0) != m_Process) { - BOOST_THROW_EXCEPTION(posix_error() - << boost::errinfo_api_function("waitpid") - << boost::errinfo_errno(errno)); - } + if (ProcessWaitPID(m_Process, &status) != m_Process) { + exitcode = 128; - if (WIFEXITED(status)) { + Log(LogWarning, "Process") + << "PID " << m_PID << " (" << PrettyPrintArguments(m_Arguments) << ") died mysteriously: waitpid failed"; + } else if (WIFEXITED(status)) { exitcode = WEXITSTATUS(status); Log(LogNotice, "Process") diff --git a/lib/base/process.hpp b/lib/base/process.hpp index bcc629d4e..16f0abd52 100644 --- a/lib/base/process.hpp +++ b/lib/base/process.hpp @@ -83,6 +83,10 @@ public: static String PrettyPrintArguments(const Arguments& arguments); +#ifndef _WIN32 + static void InitializeSpawnHelper(void); +#endif /* _WIN32 */ + private: Arguments m_Arguments; Dictionary::Ptr m_ExtraEnvironment; -- 2.49.0