@cryptotaxi247 / netdata-1 / commits / 8ee7e8b26

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

Dim-P committed Jan 19, 2023 at 12:01 UTC 8ee7e8b26162c1c29e46c8894903b57d7bbd687f
6 files changed +86 -17
configure.ac
+1
@@ -251,6 +251,7 @@ AC_SEARCH_LIBS([clock_gettime], [rt posix4])
251 AC_CHECK_FUNCS([clock_gettime])
252 AC_CHECK_FUNCS([sched_setscheduler sched_getscheduler sched_getparam sched_get_priority_min sched_get_priority_max getpriority setpriority nice])
253 AC_CHECK_FUNCS([recvmmsg])
254 +AC_CHECK_FUNCS([close_range])
255
256 AC_TYPE_INT8_T
257 AC_TYPE_INT16_T
daemon/main.c
+2 -5
@@ -1700,15 +1700,12 @@ int main(int argc, char **argv) {
1700 }
1701 }
1702
1703 -#ifdef _SC_OPEN_MAX
1703 if (close_open_fds == true) {
1704 // close all open file descriptors, except the standard ones
1705 // the caller may have left open files (lxc-attach has this issue)
1707 - for(int fd = (int) (sysconf(_SC_OPEN_MAX) - 1); fd > 2; fd--)
1708 - if(fd_is_valid(fd))
1709 - close(fd);
1706 + for_each_open_fd(OPEN_FD_ACTION_CLOSE, OPEN_FD_EXCLUDE_STDIN | OPEN_FD_EXCLUDE_STDOUT | OPEN_FD_EXCLUDE_STDERR);
1707 }
1711 -#endif
1708 +
1709
1710 if(!config_loaded) {
1711 load_netdata_conf(NULL, 0);
libnetdata/libnetdata.c
+67
@@ -1948,3 +1948,70 @@ bool run_command_and_copy_output_to_stdout(const char *command, int max_line_len
1948 netdata_pclose(NULL, fp, pid);
1949 return true;
1950 }
1951 +
1952 +void for_each_open_fd(OPEN_FD_ACTION action, OPEN_FD_EXCLUDE excluded_fds){
1953 + int fd;
1954 +
1955 + switch(action){
1956 + case OPEN_FD_ACTION_CLOSE:
1957 + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDIN)) (void)close(STDIN_FILENO);
1958 + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDOUT)) (void)close(STDOUT_FILENO);
1959 + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDERR)) (void)close(STDERR_FILENO);
1960 + break;
1961 + case OPEN_FD_ACTION_FD_CLOEXEC:
1962 + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDIN)) (void)fcntl(STDIN_FILENO, F_SETFD, FD_CLOEXEC);
1963 + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDOUT)) (void)fcntl(STDOUT_FILENO, F_SETFD, FD_CLOEXEC);
1964 + if(!(excluded_fds & OPEN_FD_EXCLUDE_STDERR)) (void)fcntl(STDERR_FILENO, F_SETFD, FD_CLOEXEC);
1965 + break;
1966 + default:
1967 + break; // do nothing
1968 + }
1969 +
1970 +#if defined(HAVE_CLOSE_RANGE)
1971 + if(close_range(STDERR_FILENO + 1, ~0U, (action == OPEN_FD_ACTION_FD_CLOEXEC ? CLOSE_RANGE_CLOEXEC : 0)) == 0) return;
1972 + error("close_range() failed, will try to close fds manually");
1973 +#endif
1974 +
1975 + DIR *dir = opendir("/proc/self/fd");
1976 + if (dir == NULL) {
1977 + struct rlimit rl;
1978 + int open_max = -1;
1979 +
1980 + if(getrlimit(RLIMIT_NOFILE, &rl) == 0 && rl.rlim_max != RLIM_INFINITY) open_max = rl.rlim_max;
1981 +#ifdef _SC_OPEN_MAX
1982 + else open_max = sysconf(_SC_OPEN_MAX);
1983 +#endif
1984 +
1985 + if (open_max == -1) open_max = 65535; // 65535 arbitrary default if everything else fails
1986 +
1987 + for (fd = STDERR_FILENO + 1; fd < open_max; fd++) {
1988 + switch(action){
1989 + case OPEN_FD_ACTION_CLOSE:
1990 + if(fd_is_valid(fd)) (void)close(fd);
1991 + break;
1992 + case OPEN_FD_ACTION_FD_CLOEXEC:
1993 + (void)fcntl(fd, F_SETFD, FD_CLOEXEC);
1994 + break;
1995 + default:
1996 + break; // do nothing
1997 + }
1998 + }
1999 + } else {
2000 + struct dirent *entry;
2001 + while ((entry = readdir(dir)) != NULL) {
2002 + fd = str2i(entry->d_name);
2003 + if(unlikely((fd == STDIN_FILENO ) || (fd == STDOUT_FILENO) || (fd == STDERR_FILENO) )) continue;
2004 + switch(action){
2005 + case OPEN_FD_ACTION_CLOSE:
2006 + if(fd_is_valid(fd)) (void)close(fd);
2007 + break;
2008 + case OPEN_FD_ACTION_FD_CLOEXEC:
2009 + (void)fcntl(fd, F_SETFD, FD_CLOEXEC);
2010 + break;
2011 + default:
2012 + break; // do nothing
2013 + }
2014 + }
2015 + closedir(dir);
2016 + }
2017 +}
libnetdata/libnetdata.h
+11
@@ -432,6 +432,17 @@ static inline char *get_word(char **words, size_t num_words, size_t index) {
432
433 bool run_command_and_copy_output_to_stdout(const char *command, int max_line_length);
434
435 +typedef enum {
436 + OPEN_FD_ACTION_CLOSE,
437 + OPEN_FD_ACTION_FD_CLOEXEC
438 +} OPEN_FD_ACTION;
439 +typedef enum {
440 + OPEN_FD_EXCLUDE_STDIN = 0x01,
441 + OPEN_FD_EXCLUDE_STDOUT = 0x02,
442 + OPEN_FD_EXCLUDE_STDERR = 0x04
443 +} OPEN_FD_EXCLUDE;
444 +void for_each_open_fd(OPEN_FD_ACTION action, OPEN_FD_EXCLUDE excluded_fds);
445 +
446 void netdata_cleanup_and_exit(int ret) NORETURN;
447 void send_statistics(const char *action, const char *action_result, const char *action_data);
448 extern char *netdata_configured_host_prefix;
libnetdata/popen/popen.c
+4 -8
@@ -163,8 +163,7 @@ static int popene_internal(volatile pid_t *pidptr, char **env, uint8_t flags, FI
163 posix_spawnattr_t attr;
164 posix_spawn_file_actions_t fa;
165
166 - int stdin_fd_to_exclude_from_closing = -1;
167 - int stdout_fd_to_exclude_from_closing = -1;
166 + unsigned int fds_to_exclude_from_closing = OPEN_FD_EXCLUDE_STDERR;
167
168 if(posix_spawn_file_actions_init(&fa)) {
169 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
194 if (posix_spawn_file_actions_addopen(&fa, STDIN_FILENO, "/dev/null", O_RDONLY, 0)) {
195 error("POPEN: posix_spawn_file_actions_addopen() on stdin to /dev/null failed.");
196 // this is not a fatal error
198 - stdin_fd_to_exclude_from_closing = STDIN_FILENO;
197 + fds_to_exclude_from_closing |= OPEN_FD_EXCLUDE_STDIN;
198 }
199 }
200
@@ -222,16 +221,13 @@ static int popene_internal(volatile pid_t *pidptr, char **env, uint8_t flags, FI
221 if (posix_spawn_file_actions_addopen(&fa, STDOUT_FILENO, "/dev/null", O_WRONLY, 0)) {
222 error("POPEN: posix_spawn_file_actions_addopen() on stdout to /dev/null failed.");
223 // this is not a fatal error
225 - stdout_fd_to_exclude_from_closing = STDOUT_FILENO;
224 + fds_to_exclude_from_closing |= OPEN_FD_EXCLUDE_STDOUT;
225 }
226 }
227
228 if(flags & POPEN_FLAG_CLOSE_FD) {
229 // Mark all files to be closed by the exec() stage of posix_spawn()
231 - for(int i = (int)(sysconf(_SC_OPEN_MAX) - 1); i >= 0; i--) {
232 - if(likely(i != STDERR_FILENO && i != stdin_fd_to_exclude_from_closing && i != stdout_fd_to_exclude_from_closing))
233 - (void)fcntl(i, F_SETFD, FD_CLOEXEC);
234 - }
230 + for_each_open_fd(OPEN_FD_ACTION_FD_CLOEXEC, fds_to_exclude_from_closing);
231 }
232
233 attr_rc = posix_spawnattr_init(&attr);
spawn/spawn_server.c
+1 -4
@@ -317,10 +317,7 @@ void spawn_server(void)
317
318 // close all open file descriptors, except the standard ones
319 // the caller may have left open files (lxc-attach has this issue)
320 - int fd;
321 - for(fd = (int)(sysconf(_SC_OPEN_MAX) - 1) ; fd > 2 ; --fd)
322 - if(fd_is_valid(fd))
323 - close(fd);
320 + for_each_open_fd(OPEN_FD_ACTION_CLOSE, OPEN_FD_EXCLUDE_STDIN | OPEN_FD_EXCLUDE_STDOUT | OPEN_FD_EXCLUDE_STDERR);
321
322 // Have the libuv IPC pipe be closed when forking child processes
323 (void) fcntl(0, F_SETFD, FD_CLOEXEC);