From 79cdfc8ca650dec3ec6ea5e6eafbca2e3a2ebc9e Mon Sep 17 00:00:00 2001 From: Zdenek Kabelac Date: Sat, 13 Apr 2024 00:39:43 +0200 Subject: [PATCH] libdaemon: implement daemon_close_stray_fds Refactor existing code from tools/lvmcmdline.c to libdaemon/server/daemon-stray.h daemon_close_stray_fds() used to close stray descriptors above some specified Fd. This is code parses content of /proc dir to minimize 'blind' closing of all possible descriptors within rlimits range. As we have the same code in few other places in it's more 'trivial' version - these were actually sensitive to high amount of descriptors, which might be configured on some system. With this patch we effectively resolve this reported gitlab issue: https://gitlab.com/lvmteam/lvm2/-/issues/5 TODO: Current placement might not be ideal - however considering existing code base constrains it's not so simple. ATM it uses lib/misc/lvm-file.h for custom_fds declaration and rest of functinality is included in daemon header file. --- libdaemon/server/daemon-server.c | 23 ++--- libdaemon/server/daemon-stray.h | 169 +++++++++++++++++++++++++++++++ tools/lvmcmdline.c | 139 +------------------------ 3 files changed, 179 insertions(+), 152 deletions(-) create mode 100644 libdaemon/server/daemon-stray.h diff --git a/libdaemon/server/daemon-server.c b/libdaemon/server/daemon-server.c index 38d8b4b60..6169c0e23 100644 --- a/libdaemon/server/daemon-server.c +++ b/libdaemon/server/daemon-server.c @@ -14,6 +14,7 @@ #include "daemon-server.h" #include "daemon-log.h" +#include "daemon-stray.h" #include "libdaemon/client/daemon-io.h" #include @@ -326,11 +327,14 @@ static void _remove_lockfile(const char *file) static void _daemonise(daemon_state s) { int child_status; - int fd, ffd; + int fd; pid_t pid; - struct rlimit rlim; struct timeval tval; sigset_t my_sigset; + struct custom_fds custom_fds = { + /* Do not close fds preloaded by systemd! */ + .out = (_systemd_activation) ? SD_FD_SOCKET_SERVER : -1 + }; if ((fd = open("/dev/null", O_RDWR)) == -1) { fprintf(stderr, "Unable to open /dev/null.\n"); @@ -392,20 +396,7 @@ static void _daemonise(daemon_state s) exit(3); } - /* Switch to sysconf(_SC_OPEN_MAX) ?? */ - if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) - ffd = 256; /* just have to guess */ - else - ffd = rlim.rlim_cur; - - for (--ffd; ffd > STDERR_FILENO; ffd--) { -#ifdef __linux__ - /* Do not close fds preloaded by systemd! */ - if (_systemd_activation && ffd == SD_FD_SOCKET_SERVER) - continue; -#endif - (void) close(ffd); - } + daemon_close_stray_fds(s.name, 0, STDERR_FILENO, &custom_fds); setsid(); diff --git a/libdaemon/server/daemon-stray.h b/libdaemon/server/daemon-stray.h new file mode 100644 index 000000000..53e0dcfae --- /dev/null +++ b/libdaemon/server/daemon-stray.h @@ -0,0 +1,169 @@ +/* + * Copyright (C) 2024 Red Hat, Inc. + * + * This file is part of LVM2. + * + * This copyrighted material is made available to anyone wishing to use, + * modify, copy, or redistribute it subject to the terms and conditions + * of the GNU Lesser General Public License v.2.1. + * + * You should have received a copy of the GNU Lesser General Public License + * along with this program; if not, write to the Free Software Foundation, + * Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#ifndef _LVM_DAEMON_STRAY_H +#define _LVM_DAEMON_STRAY_H + +/* + * needs dm -> #include "device_mapper/all.h" + * needs logging -> #include "libdm/misc/dmlib.h" + */ +#include "lib/misc/lvm-file.h" + +/* + * When compiling with valgrind pool support, skip closing descriptors + * as there is couple more of them being held by valgrind itself. + */ +#ifndef VALGRIND_POOL + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +#ifdef HAVE_VALGRIND +#include +#endif + +static void _daemon_get_cmdline(pid_t pid, char *cmdline, size_t size) +{ + char buf[sizeof(DEFAULT_PROC_DIR) + 32]; + int fd, n = 0; + + snprintf(buf, sizeof(buf), DEFAULT_PROC_DIR "/%u/cmdline", pid); + /* FIXME Use generic read code. */ + if ((fd = open(buf, O_RDONLY)) >= 0) { + if ((n = read(fd, cmdline, size - 1)) < 0) { + //perror("read", buf); + n = 0; + } + (void) close(fd); + } + cmdline[n] = '\0'; +} + +static void _daemon_get_filename(int fd, char *filename, size_t size) +{ + char buf[sizeof(DEFAULT_PROC_DIR) + 32]; + ssize_t lsize; + + snprintf(buf, sizeof(buf), DEFAULT_PROC_DIR "/self/fd/%u", fd); + + if ((lsize = readlink(buf, filename, sizeof(filename) - 1)) == -1) + filename[0] = '\0'; + else + filename[lsize] = '\0'; +} + +static void _daemon_close_descriptor(int fd, unsigned suppress_warnings, + const char *command, pid_t ppid, + const char *parent_cmdline) +{ + char filename[PATH_MAX]; + int r; + + /* Ignore bad file descriptors */ + if (!is_valid_fd(fd)) + return; + + if (!suppress_warnings) + _daemon_get_filename(fd, filename, sizeof(filename)); + + r = close(fd); + if ((fd <= STDERR_FILENO) || suppress_warnings) + return; + + if (!r) + fprintf(stderr, "File descriptor %d (%s) leaked on " + "%s invocation.", fd, filename, command); + else if (errno == EBADF) + return; + else + fprintf(stderr, "Close failed on stray file descriptor " + "%d (%s): %s", fd, filename, strerror(errno)); + + fprintf(stderr, " Parent PID %d: %s\n", (int)ppid, parent_cmdline); +} + +#endif /* VALGRIND_POOL */ + +/* Close all stray descriptor except custom fds. + * Note: when 'from_fd' is set to -1, unused 'custom_fds' must use same value! + * + * command: print command name with warning message + * suppress_warning: whether to print warning messages + * above_fd: close all descriptors above this descriptor + * custom_fds: preserve descriptors from this set of descriptors + */ +static int daemon_close_stray_fds(const char *command, int suppress_warning, + int from_fd, const struct custom_fds *custom_fds) +{ +#ifndef VALGRIND_POOL + struct rlimit rlim; + int fd; + unsigned suppress_warnings = 0; + pid_t ppid = getppid(); + char parent_cmdline[64]; + static const char _fd_dir[] = DEFAULT_PROC_DIR "/self/fd"; + struct dirent *dirent; + DIR *d; + +#ifdef HAVE_VALGRIND + if (RUNNING_ON_VALGRIND) + /* Skipping close of descriptors within valgrind execution. */ + return 1; +#endif /* HAVE_VALGRIND */ + _daemon_get_cmdline(ppid, parent_cmdline, sizeof(parent_cmdline)); + + if ((d = opendir(_fd_dir))) { + /* Discover openned descriptors from /proc/self/fd listing */ + while ((dirent = readdir(d))) { + fd = atoi(dirent->d_name); + if ((fd > from_fd) && + (fd != dirfd(d)) && + (fd != custom_fds->out) && + (fd != custom_fds->err) && + (fd != custom_fds->report)) + _daemon_close_descriptor(fd, suppress_warnings, + command, ppid, parent_cmdline); + } + + (void) closedir(d); + } else if (errno == ENOENT) { + /* Path does not exist, use the old way */ + if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) + fd = 256; /* just have to guess */ + else if ((fd = (int)rlim.rlim_cur) > 65536) + fd = 65536; /* do not bother with more then 64K fds */ + + while (--fd > from_fd) { + if ((fd != custom_fds->out) && + (fd != custom_fds->err) && + (fd != custom_fds->report)) + _daemon_close_descriptor(fd, suppress_warnings, command, + ppid, parent_cmdline); + } + } else + return 0; /* broken system */ +#endif /* VALGRIND_POOL */ + return 1; +} + +#endif diff --git a/tools/lvmcmdline.c b/tools/lvmcmdline.c index 638577261..d35a144f6 100644 --- a/tools/lvmcmdline.c +++ b/tools/lvmcmdline.c @@ -21,6 +21,7 @@ #include "lvm-version.h" #include "lib/locking/lvmlockd.h" #include "lib/datastruct/str_list.h" +#include "libdaemon/server/daemon-stray.h" /* coverity[unnecessary_header] */ #include "stub.h" @@ -34,10 +35,6 @@ #include #include -#ifdef HAVE_VALGRIND -#include -#endif - #ifdef HAVE_GETOPTLONG # include # define GETOPTLONG_FN(a, b, c, d, e) getopt_long((a), (b), (c), (d), (e)) @@ -3502,137 +3499,6 @@ static int _get_custom_fds(struct custom_fds *custom_fds) _do_get_custom_fd(LVM_REPORT_FD_ENV_VAR_NAME, &custom_fds->report); } -static const char *_get_cmdline(pid_t pid) -{ - static char _proc_cmdline[32]; - char buf[256]; - int fd, n = 0; - - snprintf(buf, sizeof(buf), DEFAULT_PROC_DIR "/%u/cmdline", pid); - /* FIXME Use generic read code. */ - if ((fd = open(buf, O_RDONLY)) >= 0) { - if ((n = read(fd, _proc_cmdline, sizeof(_proc_cmdline) - 1)) < 0) { - log_sys_error("read", buf); - n = 0; - } - if (close(fd)) - log_sys_error("close", buf); - } - _proc_cmdline[n] = '\0'; - - return _proc_cmdline; -} - -static const char *_get_filename(int fd) -{ - static char filename[PATH_MAX]; - char buf[32]; /* Assumes short DEFAULT_PROC_DIR */ - int size; - - snprintf(buf, sizeof(buf), DEFAULT_PROC_DIR "/self/fd/%u", fd); - - if ((size = readlink(buf, filename, sizeof(filename) - 1)) == -1) - filename[0] = '\0'; - else - filename[size] = '\0'; - - return filename; -} - -static void _close_descriptor(int fd, unsigned suppress_warnings, - const char *command, pid_t ppid, - const char *parent_cmdline) -{ - int r; - const char *filename; - - /* Ignore bad file descriptors */ - if (!is_valid_fd(fd)) - return; - - if (!suppress_warnings) - filename = _get_filename(fd); - - r = close(fd); - if (suppress_warnings) - return; - - if (!r) - fprintf(stderr, "File descriptor %d (%s) leaked on " - "%s invocation.", fd, filename, command); - else if (errno == EBADF) - return; - else - fprintf(stderr, "Close failed on stray file descriptor " - "%d (%s): %s", fd, filename, strerror(errno)); - - fprintf(stderr, " Parent PID %" PRIpid_t ": %s\n", ppid, parent_cmdline); -} - -static int _close_stray_fds(const char *command, struct custom_fds *custom_fds) -{ -#ifndef VALGRIND_POOL - struct rlimit rlim; - int fd; - unsigned suppress_warnings = 0; - pid_t ppid = getppid(); - const char *parent_cmdline = _get_cmdline(ppid); - static const char _fd_dir[] = DEFAULT_PROC_DIR "/self/fd"; - struct dirent *dirent; - DIR *d; - -#ifdef HAVE_VALGRIND - if (RUNNING_ON_VALGRIND) { - log_debug("Skipping close of descriptors within valgrind execution."); - return 1; - } -#endif - - if (getenv("LVM_SUPPRESS_FD_WARNINGS")) - suppress_warnings = 1; - - if (!(d = opendir(_fd_dir))) { - if (errno != ENOENT) { - log_sys_error("opendir", _fd_dir); - return 0; /* broken system */ - } - - /* Path does not exist, use the old way */ - if (getrlimit(RLIMIT_NOFILE, &rlim) < 0) { - log_sys_error("getrlimit", "RLIMIT_NOFILE"); - return 1; - } - - for (fd = 3; fd < (int)rlim.rlim_cur; fd++) { - if ((fd != custom_fds->out) && - (fd != custom_fds->err) && - (fd != custom_fds->report)) { - _close_descriptor(fd, suppress_warnings, command, ppid, - parent_cmdline); - } - } - return 1; - } - - while ((dirent = readdir(d))) { - fd = atoi(dirent->d_name); - if ((fd > 2) && - (fd != dirfd(d)) && - (fd != custom_fds->out) && - (fd != custom_fds->err) && - (fd != custom_fds->report)) { - _close_descriptor(fd, suppress_warnings, - command, ppid, parent_cmdline); - } - } - - if (closedir(d)) - log_sys_debug("closedir", _fd_dir); -#endif - - return 1; -} - struct cmd_context *init_lvm(unsigned set_connections, unsigned set_filters, unsigned threaded) @@ -3761,7 +3627,8 @@ int lvm2_main(int argc, char **argv) if (!_get_custom_fds(&custom_fds)) return EINIT_FAILED; - if (!_close_stray_fds(base, &custom_fds)) + if (!daemon_close_stray_fds(base, getenv("LVM_SUPPRESS_FD_WARNINGS") ? 1 : 0, + STDERR_FILENO, &custom_fds)) return EINIT_FAILED; if (!init_custom_log_streams(&custom_fds))