diff options
author | Dim-P <Dim-P@users.noreply.github.com> | 2023-01-19 12:01:02 +0000 |
---|---|---|
committer | GitHub <noreply@github.com> | 2023-01-19 12:01:02 +0000 |
commit | 8ee7e8b26162c1c29e46c8894903b57d7bbd687f (patch) | |
tree | daee30f6f2496446bd06539a317aba52a1de8568 | |
parent | c1908d3163185cf65a139edeb11a165a10eca1e9 (diff) |
Improve file descriptor closing loops (#14213)
* Add for_each_open_fd() and fix second instance of _SC_OPEN_MAX
* Add argument to allow exclusion of file descriptors from closing
* Fix clang error
* Address review comments
* Use close_range() if possible and replace macros with enums
-rw-r--r-- | configure.ac | 1 | ||||
-rw-r--r-- | daemon/main.c | 7 | ||||
-rw-r--r-- | libnetdata/libnetdata.c | 67 | ||||
-rw-r--r-- | libnetdata/libnetdata.h | 11 | ||||
-rw-r--r-- | libnetdata/popen/popen.c | 12 | ||||
-rw-r--r-- | spawn/spawn_server.c | 5 |
6 files changed, 86 insertions, 17 deletions
diff --git a/configure.ac b/configure.ac index d8f1e58654..ffc59c87aa 100644 --- a/configure.ac +++ b/configure.ac @@ -251,6 +251,7 @@ AC_SEARCH_LIBS([clock_gettime], [rt posix4]) AC_CHECK_FUNCS([clock_gettime]) AC_CHECK_FUNCS([sched_setscheduler sched_getscheduler sched_getparam sched_get_priority_min sched_get_priority_max getpriority setpriority nice]) AC_CHECK_FUNCS([recvmmsg]) +AC_CHECK_FUNCS([close_range]) AC_TYPE_INT8_T AC_TYPE_INT16_T diff --git a/daemon/main.c b/daemon/main.c index c19d9cc069..408b7a2ded 100644 --- a/daemon/main.c +++ b/daemon/main.c @@ -1700,15 +1700,12 @@ int main(int argc, char **argv) { } } -#ifdef _SC_OPEN_MAX if (close_open_fds == true) { // close all open file descriptors, except the standard ones // the caller may have left open files (lxc-attach has this issue) - for(int fd = (int) (sysconf(_SC_OPEN_MAX) - 1); fd > 2; fd--) - if(fd_is_valid(fd)) - close(fd); + for_each_open_fd(OPEN_FD_ACTION_CLOSE, OPEN_FD_EXCLUDE_STDIN | OPEN_FD_EXCLUDE_STDOUT | OPEN_FD_EXCLUDE_STDERR); } -#endif + if(!config_loaded) { load_netdata_conf(NULL, 0); diff --git a/libnetdata/libnetdata.c b/libnetdata/libnetdata.c index aa7a6d7364..7d86395671 100644 --- a/libnetdata/libnetdata.c +++ b/libnetdata/libnetdata.c @@ -1948,3 +1948,70 @@ bool run_command_and_copy_output_to_stdout(const char *command, int max_line_len netdata_pclose(NULL, fp, pid); return true; } + +void for_each_open_fd(OPEN_FD_ACTION action, OPEN_FD_EXCLUDE excluded_fds){ + int fd; + + switch(action){ + case OPEN_FD_ACTION_CLOSE: + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDIN)) (void)close(STDIN_FILENO); + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDOUT)) (void)close(STDOUT_FILENO); + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDERR)) (void)close(STDERR_FILENO); + break; + case OPEN_FD_ACTION_FD_CLOEXEC: + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDIN)) (void)fcntl(STDIN_FILENO, F_SETFD, FD_CLOEXEC); + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDOUT)) (void)fcntl(STDOUT_FILENO, F_SETFD, FD_CLOEXEC); + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDERR)) (void)fcntl(STDERR_FILENO, F_SETFD, FD_CLOEXEC); + break; + default: + break; // do nothing + } + +#if defined(HAVE_CLOSE_RANGE) + if(close_range(STDERR_FILENO + 1, ~0U, (action == OPEN_FD_ACTION_FD_CLOEXEC ? CLOSE_RANGE_CLOEXEC : 0)) == 0) return; + error("close_range() failed, will try to close fds manually"); +#endif + + DIR *dir = opendir("/proc/self/fd"); + if (dir == NULL) { + struct rlimit rl; + int open_max = -1; + + if(getrlimit(RLIMIT_NOFILE, &rl) == 0 && rl.rlim_max != RLIM_INFINITY) open_max = rl.rlim_max; +#ifdef _SC_OPEN_MAX + else open_max = sysconf(_SC_OPEN_MAX); +#endif + + if (open_max == -1) open_max = 65535; // 65535 arbitrary default if everything else fails + + for (fd = STDERR_FILENO + 1; fd < open_max; fd++) { + switch(action){ + case OPEN_FD_ACTION_CLOSE: + if(fd_is_valid(fd)) (void)close(fd); + break; + case OPEN_FD_ACTION_FD_CLOEXEC: + (void)fcntl(fd, F_SETFD, FD_CLOEXEC); + break; + default: + break; // do nothing + } + } + } else { + struct dirent *entry; + while ((entry = readdir(dir)) != NULL) { + fd = str2i(entry->d_name); + if(unlikely((fd == STDIN_FILENO ) || (fd == STDOUT_FILENO) || (fd == STDERR_FILENO) )) continue; + switch(action){ + case OPEN_FD_ACTION_CLOSE: + if(fd_is_valid(fd)) (void)close(fd); + break; + case OPEN_FD_ACTION_FD_CLOEXEC: + (void)fcntl(fd, F_SETFD, FD_CLOEXEC); + break; + default: + break; // do nothing + } + } + closedir(dir); + } +} diff --git a/libnetdata/libnetdata.h b/libnetdata/libnetdata.h index 7a7fe032ff..56d984dec5 100644 --- a/libnetdata/libnetdata.h +++ b/libnetdata/libnetdata.h @@ -432,6 +432,17 @@ static inline char *get_word(char **words, size_t num_words, size_t index) { bool run_command_and_copy_output_to_stdout(const char *command, int max_line_length); +typedef enum { + OPEN_FD_ACTION_CLOSE, + OPEN_FD_ACTION_FD_CLOEXEC +} OPEN_FD_ACTION; +typedef enum { + OPEN_FD_EXCLUDE_STDIN = 0x01, + OPEN_FD_EXCLUDE_STDOUT = 0x02, + OPEN_FD_EXCLUDE_STDERR = 0x04 +} OPEN_FD_EXCLUDE; +void for_each_open_fd(OPEN_FD_ACTION action, OPEN_FD_EXCLUDE excluded_fds); + void netdata_cleanup_and_exit(int ret) NORETURN; void send_statistics(const char *action, const char *action_result, const char *action_data); extern char *netdata_configured_host_prefix; diff --git a/libnetdata/popen/popen.c b/libnetdata/popen/popen.c index 57f957f636..e7fc772263 100644 --- a/libnetdata/popen/popen.c +++ b/libnetdata/popen/popen.c @@ -163,8 +163,7 @@ static int popene_internal(volatile pid_t *pidptr, char **env, uint8_t flags, FI posix_spawnattr_t attr; posix_spawn_file_actions_t fa; - int stdin_fd_to_exclude_from_closing = -1; - int stdout_fd_to_exclude_from_closing = -1; + unsigned int fds_to_exclude_from_closing = OPEN_FD_EXCLUDE_STDERR; if(posix_spawn_file_actions_init(&fa)) { error("POPEN: posix_spawn_file_actions_init() failed."); @@ -195,7 +194,7 @@ static int popene_internal(volatile pid_t *pidptr, char **env, uint8_t flags, FI if (posix_spawn_file_actions_addopen(&fa, STDIN_FILENO, "/dev/null", O_RDONLY, 0)) { error("POPEN: posix_spawn_file_actions_addopen() on stdin to /dev/null failed."); // this is not a fatal error - stdin_fd_to_exclude_from_closing = STDIN_FILENO; + fds_to_exclude_from_closing |= OPEN_FD_EXCLUDE_STDIN; } } @@ -222,16 +221,13 @@ static int popene_internal(volatile pid_t *pidptr, char **env, uint8_t flags, FI if (posix_spawn_file_actions_addopen(&fa, STDOUT_FILENO, "/dev/null", O_WRONLY, 0)) { error("POPEN: posix_spawn_file_actions_addopen() on stdout to /dev/null failed."); // this is not a fatal error - stdout_fd_to_exclude_from_closing = STDOUT_FILENO; + fds_to_exclude_from_closing |= OPEN_FD_EXCLUDE_STDOUT; } } if(flags & POPEN_FLAG_CLOSE_FD) { // Mark all files to be closed by the exec() stage of posix_spawn() - for(int i = (int)(sysconf(_SC_OPEN_MAX) - 1); i >= 0; i--) { - if(likely(i != STDERR_FILENO && i != stdin_fd_to_exclude_from_closing && i != stdout_fd_to_exclude_from_closing)) - (void)fcntl(i, F_SETFD, FD_CLOEXEC); - } + for_each_open_fd(OPEN_FD_ACTION_FD_CLOEXEC, fds_to_exclude_from_closing); } attr_rc = posix_spawnattr_init(&attr); diff --git a/spawn/spawn_server.c b/spawn/spawn_server.c index 53b143deac..1d79ef15d4 100644 --- a/spawn/spawn_server.c +++ b/spawn/spawn_server.c @@ -317,10 +317,7 @@ void spawn_server(void) // close all open file descriptors, except the standard ones // the caller may have left open files (lxc-attach has this issue) - int fd; - for(fd = (int)(sysconf(_SC_OPEN_MAX) - 1) ; fd > 2 ; --fd) - if(fd_is_valid(fd)) - close(fd); + for_each_open_fd(OPEN_FD_ACTION_CLOSE, OPEN_FD_EXCLUDE_STDIN | OPEN_FD_EXCLUDE_STDOUT | OPEN_FD_EXCLUDE_STDERR); // Have the libuv IPC pipe be closed when forking child processes (void) fcntl(0, F_SETFD, FD_CLOEXEC); |