From 3d0a4d2cf7a86735378c4390b70b82ecb135ad6d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 17 Jan 2019 12:23:21 +0100 Subject: [PATCH 1/4] fd-util: rework how we determine highest possible fd (cherry picked from commit 498e265df1c63212ec1a0991c135877a23f1ba4f) Related: RHEL-18302 --- src/basic/fd-util.c | 37 ++++++++++++++++++++++++++----------- 1 file changed, 26 insertions(+), 11 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index e085dc23b4..fea93d2039 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -188,6 +188,27 @@ _pure_ static bool fd_in_set(int fd, const int fdset[], size_t n_fdset) { return false; } +static int get_max_fd(void) { + struct rlimit rl; + rlim_t m; + + /* Return the highest possible fd, based RLIMIT_NOFILE, but enforcing FD_SETSIZE-1 as lower boundary + * and INT_MAX as upper boundary. */ + + if (getrlimit(RLIMIT_NOFILE, &rl) < 0) + return -errno; + + m = MAX(rl.rlim_cur, rl.rlim_max); + if (m < FD_SETSIZE) /* Let's always cover at least 1024 fds */ + return FD_SETSIZE-1; + + if (m == RLIM_INFINITY || m > INT_MAX) /* Saturate on overflow. After all fds are "int", hence can + * never be above INT_MAX */ + return INT_MAX; + + return (int) (m - 1); +} + int close_all_fds(const int except[], size_t n_except) { _cleanup_closedir_ DIR *d = NULL; struct dirent *de; @@ -197,20 +218,14 @@ int close_all_fds(const int except[], size_t n_except) { d = opendir("/proc/self/fd"); if (!d) { - struct rlimit rl; int fd, max_fd; - /* When /proc isn't available (for example in chroots) the fallback is brute forcing through the fd - * table */ - - assert_se(getrlimit(RLIMIT_NOFILE, &rl) >= 0); - - if (rl.rlim_max == 0) - return -EINVAL; + /* When /proc isn't available (for example in chroots) the fallback is brute forcing through + * the fd table */ - /* Let's take special care if the resource limit is set to unlimited, or actually larger than the range - * of 'int'. Let's avoid implicit overflows. */ - max_fd = (rl.rlim_max == RLIM_INFINITY || rl.rlim_max > INT_MAX) ? INT_MAX : (int) (rl.rlim_max - 1); + max_fd = get_max_fd(); + if (max_fd < 0) + return max_fd; for (fd = 3; fd >= 0; fd = fd < max_fd ? fd + 1 : -1) { int q; From 922bd45deaa4fc6a9092a50664831c5512f13c8c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Fri, 15 Mar 2019 15:13:25 +0100 Subject: [PATCH 2/4] basic/fd-util: refuse "infinite" loop in close_all_fds() I had a test machine with ulimit -n set to 1073741816 through pam ("session required pam_limits.so set_all", which copies the limits from PID 1, left over from testing of #10921). test-execute would "hang" and then fail with a timeout when running exec-inaccessiblepaths-proc.service. It turns out that the problem was in close_all_fds(), which would go to the fallback path of doing close() 1073741813 times. Let's just fail if we hit this case. This only matters for cases where both /proc is inaccessible, and the *soft* limit has been raised. (gdb) bt #0 0x00007f7e2e73fdc8 in close () from target:/lib64/libc.so.6 #1 0x00007f7e2e42cdfd in close_nointr () from target:/home/zbyszek/src/systemd-work3/build-rawhide/src/shared/libsystemd-shared-241.so #2 0x00007f7e2e42d525 in close_all_fds () from target:/home/zbyszek/src/systemd-work3/build-rawhide/src/shared/libsystemd-shared-241.so #3 0x0000000000426e53 in exec_child () #4 0x0000000000429578 in exec_spawn () #5 0x00000000004ce1ab in service_spawn () #6 0x00000000004cff77 in service_enter_start () #7 0x00000000004d028f in service_enter_start_pre () #8 0x00000000004d16f2 in service_start () #9 0x00000000004568f4 in unit_start () #10 0x0000000000416987 in test () #11 0x0000000000417632 in test_exec_inaccessiblepaths () #12 0x0000000000419362 in run_tests () #13 0x0000000000419632 in main () (cherry picked from commit 6a461d1f59850ff27bd254a3b71fe9ade0523e76) Related: RHEL-18302 --- src/basic/fd-util.c | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index fea93d2039..5d0df11d7e 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -24,6 +24,10 @@ #include "stdio-util.h" #include "util.h" +/* The maximum number of iterations in the loop to close descriptors in the fallback case + * when /proc/self/fd/ is inaccessible. */ +#define MAX_FD_LOOP_LIMIT (1024*1024) + int close_nointr(int fd) { assert(fd >= 0); @@ -227,6 +231,13 @@ int close_all_fds(const int except[], size_t n_except) { if (max_fd < 0) return max_fd; + /* Refuse to do the loop over more too many elements. It's better to fail immediately than to + * spin the CPU for a long time. */ + if (max_fd > MAX_FD_LOOP_LIMIT) + return log_debug_errno(EPERM, + "/proc/self/fd is inaccessible. Refusing to loop over %d potential fds.", + max_fd); + for (fd = 3; fd >= 0; fd = fd < max_fd ? fd + 1 : -1) { int q; From cb9eea3119598d8a371c56ddeaca62960c6b25c9 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Oct 2021 15:53:27 +0200 Subject: [PATCH 3/4] fd-util: split out inner fallback loop of close_all_fds() as close_all_fds_without_malloc() (cherry picked from commit 11966552a88039869972ca4b450f622664bd1c5e) Related: RHEL-18302 --- src/basic/fd-util.c | 63 +++++++++++++++++++++++++-------------------- src/basic/fd-util.h | 1 + 2 files changed, 36 insertions(+), 28 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 5d0df11d7e..7e4611dfda 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -213,44 +213,51 @@ static int get_max_fd(void) { return (int) (m - 1); } -int close_all_fds(const int except[], size_t n_except) { - _cleanup_closedir_ DIR *d = NULL; - struct dirent *de; - int r = 0; +int close_all_fds_without_malloc(const int except[], size_t n_except) { + int max_fd, r = 0; assert(n_except == 0 || except); - d = opendir("/proc/self/fd"); - if (!d) { - int fd, max_fd; + /* This is the inner fallback core of close_all_fds(). This never calls malloc() or opendir() or so + * and hence is safe to be called in signal handler context. Most users should call close_all_fds(), + * but when we assume we are called from signal handler context, then use this simpler call + * instead. */ - /* When /proc isn't available (for example in chroots) the fallback is brute forcing through - * the fd table */ + max_fd = get_max_fd(); + if (max_fd < 0) + return max_fd; - max_fd = get_max_fd(); - if (max_fd < 0) - return max_fd; + /* Refuse to do the loop over more too many elements. It's better to fail immediately than to + * spin the CPU for a long time. */ + if (max_fd > MAX_FD_LOOP_LIMIT) + return log_debug_errno(EPERM, + "Refusing to loop over %d potential fds.", + max_fd); - /* Refuse to do the loop over more too many elements. It's better to fail immediately than to - * spin the CPU for a long time. */ - if (max_fd > MAX_FD_LOOP_LIMIT) - return log_debug_errno(EPERM, - "/proc/self/fd is inaccessible. Refusing to loop over %d potential fds.", - max_fd); + for (int fd = 3; fd >= 0; fd = fd < max_fd ? fd + 1 : -1) { + int q; - for (fd = 3; fd >= 0; fd = fd < max_fd ? fd + 1 : -1) { - int q; + if (fd_in_set(fd, except, n_except)) + continue; - if (fd_in_set(fd, except, n_except)) - continue; + q = close_nointr(fd); + if (q < 0 && q != -EBADF && r >= 0) + r = q; + } - q = close_nointr(fd); - if (q < 0 && q != -EBADF && r >= 0) - r = q; - } + return r; +} - return r; - } +int close_all_fds(const int except[], size_t n_except) { + _cleanup_closedir_ DIR *d = NULL; + struct dirent *de; + int r = 0; + + assert(n_except == 0 || except); + + d = opendir("/proc/self/fd"); + if (!d) + return close_all_fds_without_malloc(except, n_except); /* ultimate fallback if /proc/ is not available */ FOREACH_DIRENT(de, d, return -errno) { int fd = -1, q; diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index 8adc959da8..b2837d3588 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -54,6 +54,7 @@ int fd_nonblock(int fd, bool nonblock); int fd_cloexec(int fd, bool cloexec); int close_all_fds(const int except[], size_t n_except); +int close_all_fds_without_malloc(const int except[], size_t n_except); int same_fd(int a, int b); From d8c98a3039e8bf4fd2e0806a47c58be8ad05da58 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 29 Jul 2021 16:50:44 +0200 Subject: [PATCH 4/4] exec-util: use close_all_fds_without_malloc() from freeze() (cherry picked from commit ab27b2fe56c6c4bd0295b248448adb1c698e9284) Resolves: RHEL-18302 --- src/basic/process-util.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 6016d83d41..9e2237375d 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -991,8 +991,10 @@ _noreturn_ void freeze(void) { log_close(); - /* Make sure nobody waits for us on a socket anymore */ - close_all_fds(NULL, 0); + /* Make sure nobody waits for us (i.e. on one of our sockets) anymore. Note that we use + * close_all_fds_without_malloc() instead of plain close_all_fds() here, since we want this function + * to be compatible with being called from signal handlers. */ + (void) close_all_fds_without_malloc(NULL, 0); sync();