diff --git a/SOURCES/012-fork-close.patch b/SOURCES/012-fork-close.patch new file mode 100644 index 0000000..4da5be6 --- /dev/null +++ b/SOURCES/012-fork-close.patch @@ -0,0 +1,63 @@ +From 5a73027642b826793658774df8a0536975120b19 Mon Sep 17 00:00:00 2001 +From: John Eckersberg +Date: Fri, 11 Oct 2019 08:59:41 -0400 +Subject: [PATCH] Fix: libcrmservice: try not to spam close() file descriptors + +With large file descriptor limits, action_launch_child can close +millions of non-existent file descriptors. + +Instead, try to read open file descriptors from /proc or /dev/fd and +close only those which are open. + +See rhbz#1762025 +--- + lib/services/services_linux.c | 26 ++++++++++++++++++++++++-- + 1 file changed, 24 insertions(+), 2 deletions(-) + +diff --git a/lib/services/services_linux.c b/lib/services/services_linux.c +index 90c1f44..464fc5b 100644 +--- a/lib/services/services_linux.c ++++ b/lib/services/services_linux.c +@@ -445,6 +445,7 @@ static void + action_launch_child(svc_action_t *op) + { + int lpc; ++ DIR *dir; + + /* SIGPIPE is ignored (which is different from signal blocking) by the gnutls library. + * Depending on the libqb version in use, libqb may set SIGPIPE to be ignored as well. +@@ -476,8 +477,29 @@ action_launch_child(svc_action_t *op) + setpgid(0, 0); + + // Close all file descriptors except stdin/stdout/stderr +- for (lpc = getdtablesize() - 1; lpc > STDERR_FILENO; lpc--) { +- close(lpc); ++#if SUPPORT_PROCFS ++ dir = opendir("/proc/self/fd"); ++#else ++ dir = opendir("/dev/fd"); ++#endif ++ if (dir == NULL) { /* /proc or /dev/fd not available */ ++ /* Iterate over all possible fds, might be slow */ ++ for (lpc = getdtablesize() - 1; lpc > STDERR_FILENO; lpc--) { ++ close(lpc); ++ } ++ } else { ++ /* Iterate over fds obtained from /proc or /dev/fd */ ++ struct dirent *entry; ++ int dir_fd = dirfd(dir); ++ ++ while ((entry = readdir(dir)) != NULL) { ++ lpc = atoi(entry->d_name); ++ if (lpc > STDERR_FILENO && lpc != dir_fd) { ++ close(lpc); ++ } ++ } ++ ++ closedir(dir); + } + + #if SUPPORT_CIBSECRETS +-- +1.8.3.1 + diff --git a/SOURCES/013-soft-limit.patch b/SOURCES/013-soft-limit.patch new file mode 100644 index 0000000..4a3c626 --- /dev/null +++ b/SOURCES/013-soft-limit.patch @@ -0,0 +1,216 @@ +From 68d6a69a8bc2f25e935608344d5b7e2b52cde85f Mon Sep 17 00:00:00 2001 +From: Ken Gaillot +Date: Mon, 21 Oct 2019 19:02:47 -0500 +Subject: [PATCH] Low: libcrmservice: don't close descriptors above current + limit + +This is irrelevant in normal use. However, valgrind can open high-numbered +file descriptors for its own use above the soft limit of the process being run +under valgrind. If that process forks a child that tries to close all open file +descriptors (e.g. the executor running an agent), the close fails because the +file descriptors are invalid, and (ironically) valgrind warns about that. + +This allows 5a73027 to work under valgrind. Additionally, we extend the +efficient close method from that commit to pacemakerd's spawning of children. +--- + daemons/pacemakerd/pacemakerd.c | 10 ++---- + include/crm/common/internal.h | 3 ++ + lib/common/io.c | 72 +++++++++++++++++++++++++++++++++++++++++ + lib/services/services_linux.c | 29 +---------------- + 4 files changed, 79 insertions(+), 35 deletions(-) + +diff --git a/daemons/pacemakerd/pacemakerd.c b/daemons/pacemakerd/pacemakerd.c +index fdc1d9f..d8ff53d 100644 +--- a/daemons/pacemakerd/pacemakerd.c ++++ b/daemons/pacemakerd/pacemakerd.c +@@ -13,6 +13,8 @@ + #include + #include + #include ++#include ++#include + #include + #include + #include +@@ -290,10 +292,8 @@ static char *opts_vgrind[] = { NULL, NULL, NULL, NULL, NULL }; + static gboolean + start_child(pcmk_child_t * child) + { +- int lpc = 0; + uid_t uid = 0; + gid_t gid = 0; +- struct rlimit oflimits; + gboolean use_valgrind = FALSE; + gboolean use_callgrind = FALSE; + const char *devnull = "/dev/null"; +@@ -396,11 +396,7 @@ start_child(pcmk_child_t * child) + crm_perror(LOG_ERR, "Could not set user to %d (%s)", uid, child->uid); + } + +- /* Close all open file descriptors */ +- getrlimit(RLIMIT_NOFILE, &oflimits); +- for (lpc = 0; lpc < oflimits.rlim_cur; lpc++) { +- close(lpc); +- } ++ pcmk__close_fds_in_child(true); + + (void)open(devnull, O_RDONLY); /* Stdin: fd 0 */ + (void)open(devnull, O_WRONLY); /* Stdout: fd 1 */ +diff --git a/include/crm/common/internal.h b/include/crm/common/internal.h +index b2eec00..da2c7d7 100644 +--- a/include/crm/common/internal.h ++++ b/include/crm/common/internal.h +@@ -13,6 +13,7 @@ + #include /* for gboolean */ + #include /* for struct dirent */ + #include /* for getpid() */ ++#include /* for bool */ + #include /* for uid_t and gid_t */ + + #include +@@ -33,6 +34,8 @@ int crm_write_sync(int fd, const char *contents); + int crm_set_nonblocking(int fd); + const char *crm_get_tmpdir(void); + ++void pcmk__close_fds_in_child(bool); ++ + + /* internal procfs utilities (from procfs.c) */ + +diff --git a/lib/common/io.c b/lib/common/io.c +index fa438dd..6cbab0a 100644 +--- a/lib/common/io.c ++++ b/lib/common/io.c +@@ -16,6 +16,7 @@ + #include + #include + #include ++#include + + #include + #include +@@ -501,3 +502,74 @@ crm_get_tmpdir() + + return (dir && (*dir == '/'))? dir : "/tmp"; + } ++ ++/*! ++ * \internal ++ * \brief Close open file descriptors ++ * ++ * Close all file descriptors (except optionally stdin, stdout, and stderr), ++ * which is a best practice for a new child process forked for the purpose of ++ * executing an external program. ++ * ++ * \param[in] bool If true, close stdin, stdout, and stderr as well ++ */ ++void ++pcmk__close_fds_in_child(bool all) ++{ ++ DIR *dir; ++ struct rlimit rlim; ++ rlim_t max_fd; ++ int min_fd = (all? 0 : (STDERR_FILENO + 1)); ++ ++ /* Find the current process's (soft) limit for open files. getrlimit() ++ * should always work, but have a fallback just in case. ++ */ ++ if (getrlimit(RLIMIT_NOFILE, &rlim) == 0) { ++ max_fd = rlim.rlim_cur - 1; ++ } else { ++ long conf_max = sysconf(_SC_OPEN_MAX); ++ ++ max_fd = (conf_max > 0)? conf_max : 1024; ++ } ++ ++ /* /proc/self/fd (on Linux) or /dev/fd (on most OSes) contains symlinks to ++ * all open files for the current process, named as the file descriptor. ++ * Use this if available, because it's more efficient than a shotgun ++ * approach to closing descriptors. ++ */ ++#if SUPPORT_PROCFS ++ dir = opendir("/proc/self/fd"); ++ if (dir == NULL) { ++ dir = opendir("/dev/fd"); ++ } ++#else ++ dir = opendir("/dev/fd"); ++#endif ++ if (dir != NULL) { ++ struct dirent *entry; ++ int dir_fd = dirfd(dir); ++ ++ while ((entry = readdir(dir)) != NULL) { ++ int lpc = atoi(entry->d_name); ++ ++ /* How could one of these entries be higher than max_fd, you ask? ++ * It isn't possible in normal operation, but when run under ++ * valgrind, valgrind can open high-numbered file descriptors for ++ * its own use that are higher than the process's soft limit. ++ * These will show up in the fd directory but aren't closable. ++ */ ++ if ((lpc >= min_fd) && (lpc <= max_fd) && (lpc != dir_fd)) { ++ close(lpc); ++ } ++ } ++ closedir(dir); ++ return; ++ } ++ ++ /* If no fd directory is available, iterate over all possible descriptors. ++ * This is less efficient due to the overhead of many system calls. ++ */ ++ for (int lpc = max_fd; lpc >= min_fd; lpc--) { ++ close(lpc); ++ } ++} +diff --git a/lib/services/services_linux.c b/lib/services/services_linux.c +index 464fc5b..6870273 100644 +--- a/lib/services/services_linux.c ++++ b/lib/services/services_linux.c +@@ -444,9 +444,6 @@ services_handle_exec_error(svc_action_t * op, int error) + static void + action_launch_child(svc_action_t *op) + { +- int lpc; +- DIR *dir; +- + /* SIGPIPE is ignored (which is different from signal blocking) by the gnutls library. + * Depending on the libqb version in use, libqb may set SIGPIPE to be ignored as well. + * We do not want this to be inherited by the child process. By resetting this the signal +@@ -476,31 +473,7 @@ action_launch_child(svc_action_t *op) + */ + setpgid(0, 0); + +- // Close all file descriptors except stdin/stdout/stderr +-#if SUPPORT_PROCFS +- dir = opendir("/proc/self/fd"); +-#else +- dir = opendir("/dev/fd"); +-#endif +- if (dir == NULL) { /* /proc or /dev/fd not available */ +- /* Iterate over all possible fds, might be slow */ +- for (lpc = getdtablesize() - 1; lpc > STDERR_FILENO; lpc--) { +- close(lpc); +- } +- } else { +- /* Iterate over fds obtained from /proc or /dev/fd */ +- struct dirent *entry; +- int dir_fd = dirfd(dir); +- +- while ((entry = readdir(dir)) != NULL) { +- lpc = atoi(entry->d_name); +- if (lpc > STDERR_FILENO && lpc != dir_fd) { +- close(lpc); +- } +- } +- +- closedir(dir); +- } ++ pcmk__close_fds_in_child(false); + + #if SUPPORT_CIBSECRETS + if (replace_secret_params(op->rsc, op->params) < 0) { +-- +1.8.3.1 + diff --git a/SPECS/pacemaker.spec b/SPECS/pacemaker.spec index 7c1a689..045bbf2 100644 --- a/SPECS/pacemaker.spec +++ b/SPECS/pacemaker.spec @@ -195,7 +195,7 @@ Name: pacemaker Summary: Scalable High-Availability cluster resource manager Version: %{pcmkversion} -Release: %{pcmk_release}%{?dist} +Release: %{pcmk_release}%{?dist}.2 %if %{defined _unitdir} License: GPLv2+ and LGPLv2+ %else @@ -222,6 +222,8 @@ Patch8: 008-stonith_admin-header-refactoring.patch Patch9: 009-improve-pacemaker_remote-handling.patch Patch10: 010-fix-history-handing-on-fenced-restart.patch Patch11: 011-crm_report.patch +Patch12: 012-fork-close.patch +Patch13: 013-soft-limit.patch Requires: resource-agents Requires: %{name}-libs%{?_isa} = %{version}-%{release} @@ -233,7 +235,7 @@ Requires: psmisc %endif %{?systemd_requires} -ExclusiveArch: aarch64 i686 ppc64le s390x x86_64 %{arm} +ExclusiveArch: aarch64 i686 ppc64le s390x x86_64 Requires: %{python_path} BuildRequires: %{python_name}-devel @@ -902,6 +904,14 @@ exit 0 %license %{nagios_name}-%{nagios_hash}/COPYING %changelog +* Mon Oct 28 2019 Ken Gaillot - 2.0.2-3.2 +- Correct gating test syntax and add z-stream tag to build +- Resolves: rhbz#1764181 + +* Mon Oct 28 2019 Ken Gaillot - 2.0.2-3.1 +- Improve efficiency when closing file descriptors after a fork +- Resolves: rhbz#1764181 + * Mon Aug 26 2019 Ken Gaillot - 2.0.2-3 - Make pacemaker-cli require tar and bzip2 - Resolves: rhbz#1741580