From e7e7c07c50d980a4494b101a2b1b5e6126c7940f Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 29 Jul 2021 16:34:45 +0200 Subject: [PATCH 01/10] Revert "Add variant of close_all_fds() that does not allocate and use it in freeze()" This reverts commit cbcf371abc328167fa869721c1add4850c793240. --- src/basic/fd-util.c | 8 ++++---- src/basic/fd-util.h | 5 +---- src/basic/process-util.c | 2 +- 3 files changed, 6 insertions(+), 9 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 27f651600ec..032f3037895 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -208,7 +208,7 @@ static int get_max_fd(void) { return (int) (m - 1); } -int close_all_fds_full(int except[], size_t n_except, bool allow_alloc) { +int close_all_fds(int except[], size_t n_except) { static bool have_close_range = true; /* Assume we live in the future */ _cleanup_closedir_ DIR *d = NULL; int r = 0; @@ -274,7 +274,7 @@ int close_all_fds_full(int except[], size_t n_except, bool allow_alloc) { /* Fallback for when close_range() is not supported */ opendir_fallback: - d = allow_alloc ? opendir("/proc/self/fd") : NULL; + d = opendir("/proc/self/fd"); if (d) { struct dirent *de; @@ -302,8 +302,8 @@ int close_all_fds_full(int except[], size_t n_except, bool allow_alloc) { return r; } - /* Fallback for when /proc isn't available (for example in chroots) or when we cannot allocate by - * brute-forcing through the file descriptor table. */ + /* Fallback for when /proc isn't available (for example in chroots) by brute-forcing through the file + * descriptor table. */ int max_fd = get_max_fd(); if (max_fd < 0) diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index 2382d52d40c..ab841b67e00 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -57,10 +57,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(DIR*, closedir, NULL); int fd_nonblock(int fd, bool nonblock); int fd_cloexec(int fd, bool cloexec); -int close_all_fds_full(int except[], size_t n_except, bool allow_alloc); -static inline int close_all_fds(int except[], size_t n_except) { - return close_all_fds_full(except, n_except, true); -} +int close_all_fds(int except[], size_t n_except); int same_fd(int a, int b); diff --git a/src/basic/process-util.c b/src/basic/process-util.c index 38df34e3395..fef0c742c70 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1622,7 +1622,7 @@ _noreturn_ void freeze(void) { log_close(); /* Make sure nobody waits for us on a socket anymore */ - (void) close_all_fds_full(NULL, 0, false); + (void) close_all_fds(NULL, 0); /* Let's not freeze right away, but keep reaping zombies. */ for (;;) { From c85cb3bc7f00e857ea90e374b916b2fd7eb773ec Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 29 Jul 2021 16:36:15 +0200 Subject: [PATCH 02/10] Revert "basic/fd-util: sort the 'except' array in place" This reverts commit 9c46228b7deb53d6384545535b37b2844a102b2b. --- src/basic/fd-util.c | 146 +++++++++++++++++++++++---------------- src/basic/fd-util.h | 2 +- src/basic/process-util.c | 7 +- src/basic/process-util.h | 4 +- src/shared/exec-util.c | 2 +- src/shared/exec-util.h | 2 +- 6 files changed, 94 insertions(+), 69 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 032f3037895..dec083daaf9 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -208,9 +208,10 @@ static int get_max_fd(void) { return (int) (m - 1); } -int close_all_fds(int except[], size_t n_except) { +int close_all_fds(const int except[], size_t n_except) { static bool have_close_range = true; /* Assume we live in the future */ _cleanup_closedir_ DIR *d = NULL; + struct dirent *de; int r = 0; assert(n_except == 0 || except); @@ -226,104 +227,129 @@ int close_all_fds(int except[], size_t n_except) { /* Close everything. Yay! */ if (close_range(3, -1, 0) >= 0) - return 0; + return 1; - if (ERRNO_IS_NOT_SUPPORTED(errno) || ERRNO_IS_PRIVILEGE(errno)) - have_close_range = false; - else + if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) return -errno; + have_close_range = false; } else { - typesafe_qsort(except, n_except, cmp_int); + _cleanup_free_ int *sorted_malloc = NULL; + size_t n_sorted; + int *sorted; - for (size_t i = 0; i < n_except; i++) { - int start = i == 0 ? 2 : MAX(except[i-1], 2); /* The first three fds shall always remain open */ - int end = MAX(except[i], 2); + assert(n_except < SIZE_MAX); + n_sorted = n_except + 1; - assert(end >= start); + if (n_sorted > 64) /* Use heap for large numbers of fds, stack otherwise */ + sorted = sorted_malloc = new(int, n_sorted); + else + sorted = newa(int, n_sorted); - if (end - start <= 1) - continue; + if (sorted) { + int c = 0; + + memcpy(sorted, except, n_except * sizeof(int)); + + /* Let's add fd 2 to the list of fds, to simplify the loop below, as this + * allows us to cover the head of the array the same way as the body */ + sorted[n_sorted-1] = 2; + + typesafe_qsort(sorted, n_sorted, cmp_int); + + for (size_t i = 0; i < n_sorted-1; i++) { + int start, end; + + start = MAX(sorted[i], 2); /* The first three fds shall always remain open */ + end = MAX(sorted[i+1], 2); + + assert(end >= start); + + if (end - start <= 1) + continue; + + /* Close everything between the start and end fds (both of which shall stay open) */ + if (close_range(start + 1, end - 1, 0) < 0) { + if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) + return -errno; - /* Close everything between the start and end fds (both of which shall stay open) */ - if (close_range(start + 1, end - 1, 0) < 0) { - if (ERRNO_IS_NOT_SUPPORTED(errno) || ERRNO_IS_PRIVILEGE(errno)) have_close_range = false; - else + break; + } + + c += end - start - 1; + } + + if (have_close_range) { + /* The loop succeeded. Let's now close everything beyond the end */ + + if (sorted[n_sorted-1] >= INT_MAX) /* Dont let the addition below overflow */ + return c; + + if (close_range(sorted[n_sorted-1] + 1, -1, 0) >= 0) + return c + 1; + + if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) return -errno; - goto opendir_fallback; + + have_close_range = false; } } - - /* The loop succeeded. Let's now close everything beyond the end */ - - if (except[n_except-1] >= INT_MAX) /* Don't let the addition below overflow */ - return 0; - - int start = MAX(except[n_except-1], 2); - - if (close_range(start + 1, -1, 0) >= 0) - return 0; - - if (ERRNO_IS_NOT_SUPPORTED(errno) || ERRNO_IS_PRIVILEGE(errno)) - have_close_range = false; - else - return -errno; } + + /* Fallback on OOM or if close_range() is not supported */ } - /* Fallback for when close_range() is not supported */ - opendir_fallback: d = opendir("/proc/self/fd"); - if (d) { - struct dirent *de; + if (!d) { + int fd, max_fd; - FOREACH_DIRENT(de, d, return -errno) { - int fd = -1, q; + /* When /proc isn't available (for example in chroots) the fallback is brute forcing through + * the fd table */ - if (safe_atoi(de->d_name, &fd) < 0) - /* Let's better ignore this, just in case */ - continue; + max_fd = get_max_fd(); + if (max_fd < 0) + return max_fd; - if (fd < 3) - continue; + /* 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(SYNTHETIC_ERRNO(EPERM), + "/proc/self/fd is inaccessible. Refusing to loop over %d potential fds.", + max_fd); - if (fd == dirfd(d)) - continue; + for (fd = 3; fd >= 0; fd = fd < max_fd ? fd + 1 : -1) { + int q; if (fd_in_set(fd, except, n_except)) continue; q = close_nointr(fd); - if (q < 0 && q != -EBADF && r >= 0) /* Valgrind has its own FD and doesn't want to have it closed */ + if (q < 0 && q != -EBADF && r >= 0) r = q; } return r; } - /* Fallback for when /proc isn't available (for example in chroots) by brute-forcing through the file - * descriptor table. */ + FOREACH_DIRENT(de, d, return -errno) { + int fd = -1, q; - int max_fd = get_max_fd(); - if (max_fd < 0) - return max_fd; + if (safe_atoi(de->d_name, &fd) < 0) + /* Let's better ignore this, just in case */ + continue; - /* 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(SYNTHETIC_ERRNO(EPERM), - "/proc/self/fd is inaccessible. Refusing to loop over %d potential fds.", - max_fd); + if (fd < 3) + continue; - for (int fd = 3; fd >= 0; fd = fd < max_fd ? fd + 1 : -1) { - int q; + if (fd == dirfd(d)) + continue; if (fd_in_set(fd, except, n_except)) continue; q = close_nointr(fd); - if (q < 0 && q != -EBADF && r >= 0) + if (q < 0 && q != -EBADF && r >= 0) /* Valgrind has its own FD and doesn't want to have it closed */ r = q; } diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index ab841b67e00..459059c64ee 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -57,7 +57,7 @@ DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(DIR*, closedir, NULL); int fd_nonblock(int fd, bool nonblock); int fd_cloexec(int fd, bool cloexec); -int close_all_fds(int except[], size_t n_except); +int close_all_fds(const int except[], size_t n_except); int same_fd(int a, int b); diff --git a/src/basic/process-util.c b/src/basic/process-util.c index fef0c742c70..5c56a59aabe 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1246,7 +1246,7 @@ static void restore_sigsetp(sigset_t **ssp) { int safe_fork_full( const char *name, - int except_fds[], + const int except_fds[], size_t n_except_fds, ForkFlags flags, pid_t *ret_pid) { @@ -1441,7 +1441,7 @@ int safe_fork_full( int namespace_fork( const char *outer_name, const char *inner_name, - int except_fds[], + const int except_fds[], size_t n_except_fds, ForkFlags flags, int pidns_fd, @@ -1457,8 +1457,7 @@ int namespace_fork( * process. This ensures that we are fully a member of the destination namespace, with pidns an all, so that * /proc/self/fd works correctly. */ - r = safe_fork_full(outer_name, except_fds, n_except_fds, - (flags|FORK_DEATHSIG) & ~(FORK_REOPEN_LOG|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE), ret_pid); + r = safe_fork_full(outer_name, except_fds, n_except_fds, (flags|FORK_DEATHSIG) & ~(FORK_REOPEN_LOG|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE), ret_pid); if (r < 0) return r; if (r == 0) { diff --git a/src/basic/process-util.h b/src/basic/process-util.h index 451d0a5ff40..c7622c98df5 100644 --- a/src/basic/process-util.h +++ b/src/basic/process-util.h @@ -166,13 +166,13 @@ typedef enum ForkFlags { FORK_NEW_USERNS = 1 << 13, /* Run child in its own user namespace */ } ForkFlags; -int safe_fork_full(const char *name, int except_fds[], size_t n_except_fds, ForkFlags flags, pid_t *ret_pid); +int safe_fork_full(const char *name, const int except_fds[], size_t n_except_fds, ForkFlags flags, pid_t *ret_pid); static inline int safe_fork(const char *name, ForkFlags flags, pid_t *ret_pid) { return safe_fork_full(name, NULL, 0, flags, ret_pid); } -int namespace_fork(const char *outer_name, const char *inner_name, int except_fds[], size_t n_except_fds, ForkFlags flags, int pidns_fd, int mntns_fd, int netns_fd, int userns_fd, int root_fd, pid_t *ret_pid); +int namespace_fork(const char *outer_name, const char *inner_name, const int except_fds[], size_t n_except_fds, ForkFlags flags, int pidns_fd, int mntns_fd, int netns_fd, int userns_fd, int root_fd, pid_t *ret_pid); int set_oom_score_adjust(int value); int get_oom_score_adjust(int *ret); diff --git a/src/shared/exec-util.c b/src/shared/exec-util.c index fd0d95c5301..f0a9ea9e9fa 100644 --- a/src/shared/exec-util.c +++ b/src/shared/exec-util.c @@ -471,7 +471,7 @@ int fexecve_or_execve(int executable_fd, const char *executable, char *const arg return -errno; } -int fork_agent(const char *name, int except[], size_t n_except, pid_t *ret_pid, const char *path, ...) { +int fork_agent(const char *name, const int except[], size_t n_except, pid_t *ret_pid, const char *path, ...) { bool stdout_is_tty, stderr_is_tty; size_t n, i; va_list ap; diff --git a/src/shared/exec-util.h b/src/shared/exec-util.h index 21d28608f99..ba4506e5aa9 100644 --- a/src/shared/exec-util.h +++ b/src/shared/exec-util.h @@ -49,4 +49,4 @@ ExecCommandFlags exec_command_flags_from_string(const char *s); int fexecve_or_execve(int executable_fd, const char *executable, char *const argv[], char *const envp[]); -int fork_agent(const char *name, int except[], size_t n_except, pid_t *ret_pid, const char *path, ...) _sentinel_; +int fork_agent(const char *name, const int except[], size_t n_except, pid_t *ret_pid, const char *path, ...) _sentinel_; From 11966552a88039869972ca4b450f622664bd1c5e Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Oct 2021 15:53:27 +0200 Subject: [PATCH 03/10] fd-util: split out inner fallback loop of close_all_fds() as close_all_fds_without_malloc() --- src/basic/fd-util.c | 67 +++++++++++++++++++++++++-------------------- src/basic/fd-util.h | 1 + 2 files changed, 38 insertions(+), 30 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index dec083daaf9..274bbb32cc0 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -208,6 +208,41 @@ static int get_max_fd(void) { return (int) (m - 1); } +int close_all_fds_without_malloc(const int except[], size_t n_except) { + int max_fd, r = 0; + + assert(n_except == 0 || except); + + /* 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. */ + + 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(SYNTHETIC_ERRNO(EPERM), + "Refusing to loop over %d potential fds.", + max_fd); + + for (int fd = 3; fd >= 0; fd = fd < max_fd ? fd + 1 : -1) { + int q; + + if (fd_in_set(fd, except, n_except)) + continue; + + q = close_nointr(fd); + if (q < 0 && q != -EBADF && r >= 0) + r = q; + } + + return r; +} + int close_all_fds(const int except[], size_t n_except) { static bool have_close_range = true; /* Assume we live in the future */ _cleanup_closedir_ DIR *d = NULL; @@ -301,36 +336,8 @@ int close_all_fds(const int except[], size_t n_except) { } d = opendir("/proc/self/fd"); - if (!d) { - int fd, max_fd; - - /* 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; - - /* 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(SYNTHETIC_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; - - if (fd_in_set(fd, except, n_except)) - continue; - - q = close_nointr(fd); - if (q < 0 && q != -EBADF && r >= 0) - r = q; - } - - return r; - } + 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 459059c64ee..e929386b539 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -58,6 +58,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 f498720a3464434e6df9bcfc54ccc8d8594ac2ff Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Oct 2021 15:53:55 +0200 Subject: [PATCH 04/10] fd-util: special case invocation of close_all_fds() with single exception fd Add special case optimization for a single exception fd. It's a pretty common case in our codebase, and the optimization is simple and means we don't need to copy/sort the exception array, so do it. --- src/basic/fd-util.c | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 274bbb32cc0..658b17fed72 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -268,6 +268,21 @@ int close_all_fds(const int except[], size_t n_except) { return -errno; have_close_range = false; + + } else if (n_except == 1) { + + /* Close all but exactly one, then we don't need no sorting. This is a pretty common + * case, hence let's handle it specially. */ + + if ((except[0] <= 3 || close_range(3, except[0]-1, 0) >= 0) && + (except[0] >= INT_MAX || close_range(MAX(3, except[0]+1), -1, 0) >= 0)) + return 0; + + if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) + return -errno; + + have_close_range = false; + } else { _cleanup_free_ int *sorted_malloc = NULL; size_t n_sorted; From c844f0a924f34d0fe84672c08f83eeed6d696b44 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Oct 2021 15:54:54 +0200 Subject: [PATCH 05/10] fd-util: always return 0 on success in close_all_fds() We never make use of the return value, and in case of close_range() we don't even know how many fds got closed, hence don't pretend we knew. --- src/basic/fd-util.c | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 658b17fed72..b9df043aa7d 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -262,7 +262,7 @@ int close_all_fds(const int except[], size_t n_except) { /* Close everything. Yay! */ if (close_range(3, -1, 0) >= 0) - return 1; + return 0; if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) return -errno; @@ -297,8 +297,6 @@ int close_all_fds(const int except[], size_t n_except) { sorted = newa(int, n_sorted); if (sorted) { - int c = 0; - memcpy(sorted, except, n_except * sizeof(int)); /* Let's add fd 2 to the list of fds, to simplify the loop below, as this @@ -326,18 +324,16 @@ int close_all_fds(const int except[], size_t n_except) { have_close_range = false; break; } - - c += end - start - 1; } if (have_close_range) { /* The loop succeeded. Let's now close everything beyond the end */ if (sorted[n_sorted-1] >= INT_MAX) /* Dont let the addition below overflow */ - return c; + return 0; if (close_range(sorted[n_sorted-1] + 1, -1, 0) >= 0) - return c + 1; + return 0; if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) return -errno; From 1f6639eac7e44d55b6a859450ad0cc64b78c0889 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Oct 2021 15:50:39 +0200 Subject: [PATCH 06/10] fd-util: close_all() check d_type Tiny optimization: check dirent's d_type before trying to parse /proc/self/fd/ filenames, given we have that anyway. --- src/basic/fd-util.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index b9df043aa7d..70c036db173 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -353,6 +353,9 @@ int close_all_fds(const int except[], size_t n_except) { FOREACH_DIRENT(de, d, return -errno) { int fd = -1, q; + if (!IN_SET(de->d_type, DT_LNK, DT_UNKNOWN)) + continue; + if (safe_atoi(de->d_name, &fd) < 0) /* Let's better ignore this, just in case */ continue; From 5cfa0798baaf3ba442f8663353e1da2098cc5b87 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Tue, 12 Oct 2021 16:11:46 +0200 Subject: [PATCH 07/10] fd-util: split out close_all_fds() special case handling and call it from close_all_fds_without_malloc(), too The optimization is useful there too. --- src/basic/fd-util.c | 181 +++++++++++++++++++++++++++----------------- 1 file changed, 110 insertions(+), 71 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 70c036db173..50666e6375c 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -208,7 +208,7 @@ static int get_max_fd(void) { return (int) (m - 1); } -int close_all_fds_without_malloc(const int except[], size_t n_except) { +static int close_all_fds_frugal(const int except[], size_t n_except) { int max_fd, r = 0; assert(n_except == 0 || except); @@ -243,104 +243,143 @@ int close_all_fds_without_malloc(const int except[], size_t n_except) { return r; } +static bool have_close_range = true; /* Assume we live in the future */ + +static int close_all_fds_special_case(const int except[], size_t n_except) { + assert(n_except == 0 || except); + + /* Handles a few common special cases separately, since they are common and can be optimized really + * nicely, since we won't need sorting for them. Returns > 0 if the special casing worked, 0 + * otherwise. */ + + if (!have_close_range) + return 0; + + switch (n_except) { + + case 0: + /* Close everything. Yay! */ + + if (close_range(3, -1, 0) >= 0) + return 1; + + if (ERRNO_IS_NOT_SUPPORTED(errno) || ERRNO_IS_PRIVILEGE(errno)) { + have_close_range = false; + return 0; + } + + return -errno; + + case 1: + /* Close all but exactly one, then we don't need no sorting. This is a pretty common + * case, hence let's handle it specially. */ + + if ((except[0] <= 3 || close_range(3, except[0]-1, 0) >= 0) && + (except[0] >= INT_MAX || close_range(MAX(3, except[0]+1), -1, 0) >= 0)) + return 1; + + if (ERRNO_IS_NOT_SUPPORTED(errno) || ERRNO_IS_PRIVILEGE(errno)) { + have_close_range = false; + return 0; + } + + return -errno; + + default: + return 0; + } +} + +int close_all_fds_without_malloc(const int except[], size_t n_except) { + int r; + + assert(n_except == 0 || except); + + r = close_all_fds_special_case(except, n_except); + if (r < 0) + return r; + if (r > 0) /* special case worked! */ + return 0; + + return close_all_fds_frugal(except, n_except); +} + int close_all_fds(const int except[], size_t n_except) { - static bool have_close_range = true; /* Assume we live in the future */ _cleanup_closedir_ DIR *d = NULL; struct dirent *de; int r = 0; assert(n_except == 0 || except); + r = close_all_fds_special_case(except, n_except); + if (r < 0) + return r; + if (r > 0) /* special case worked! */ + return 0; + if (have_close_range) { + _cleanup_free_ int *sorted_malloc = NULL; + size_t n_sorted; + int *sorted; + /* In the best case we have close_range() to close all fds between a start and an end fd, * which we can use on the "inverted" exception array, i.e. all intervals between all * adjacent pairs from the sorted exception array. This changes loop complexity from O(n) * where n is number of open fds to O(m⋅log(m)) where m is the number of fds to keep * open. Given that we assume n ≫ m that's preferable to us. */ - if (n_except == 0) { - /* Close everything. Yay! */ + assert(n_except < SIZE_MAX); + n_sorted = n_except + 1; - if (close_range(3, -1, 0) >= 0) - return 0; + if (n_sorted > 64) /* Use heap for large numbers of fds, stack otherwise */ + sorted = sorted_malloc = new(int, n_sorted); + else + sorted = newa(int, n_sorted); - if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) - return -errno; + if (sorted) { + memcpy(sorted, except, n_except * sizeof(int)); - have_close_range = false; + /* Let's add fd 2 to the list of fds, to simplify the loop below, as this + * allows us to cover the head of the array the same way as the body */ + sorted[n_sorted-1] = 2; - } else if (n_except == 1) { + typesafe_qsort(sorted, n_sorted, cmp_int); - /* Close all but exactly one, then we don't need no sorting. This is a pretty common - * case, hence let's handle it specially. */ + for (size_t i = 0; i < n_sorted-1; i++) { + int start, end; - if ((except[0] <= 3 || close_range(3, except[0]-1, 0) >= 0) && - (except[0] >= INT_MAX || close_range(MAX(3, except[0]+1), -1, 0) >= 0)) - return 0; + start = MAX(sorted[i], 2); /* The first three fds shall always remain open */ + end = MAX(sorted[i+1], 2); - if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) - return -errno; + assert(end >= start); - have_close_range = false; - - } else { - _cleanup_free_ int *sorted_malloc = NULL; - size_t n_sorted; - int *sorted; - - assert(n_except < SIZE_MAX); - n_sorted = n_except + 1; - - if (n_sorted > 64) /* Use heap for large numbers of fds, stack otherwise */ - sorted = sorted_malloc = new(int, n_sorted); - else - sorted = newa(int, n_sorted); - - if (sorted) { - memcpy(sorted, except, n_except * sizeof(int)); - - /* Let's add fd 2 to the list of fds, to simplify the loop below, as this - * allows us to cover the head of the array the same way as the body */ - sorted[n_sorted-1] = 2; - - typesafe_qsort(sorted, n_sorted, cmp_int); - - for (size_t i = 0; i < n_sorted-1; i++) { - int start, end; - - start = MAX(sorted[i], 2); /* The first three fds shall always remain open */ - end = MAX(sorted[i+1], 2); - - assert(end >= start); - - if (end - start <= 1) - continue; - - /* Close everything between the start and end fds (both of which shall stay open) */ - if (close_range(start + 1, end - 1, 0) < 0) { - if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) - return -errno; - - have_close_range = false; - break; - } - } - - if (have_close_range) { - /* The loop succeeded. Let's now close everything beyond the end */ - - if (sorted[n_sorted-1] >= INT_MAX) /* Dont let the addition below overflow */ - return 0; - - if (close_range(sorted[n_sorted-1] + 1, -1, 0) >= 0) - return 0; + if (end - start <= 1) + continue; + /* Close everything between the start and end fds (both of which shall stay open) */ + if (close_range(start + 1, end - 1, 0) < 0) { if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) return -errno; have_close_range = false; + break; } } + + if (have_close_range) { + /* The loop succeeded. Let's now close everything beyond the end */ + + if (sorted[n_sorted-1] >= INT_MAX) /* Dont let the addition below overflow */ + return 0; + + if (close_range(sorted[n_sorted-1] + 1, -1, 0) >= 0) + return 0; + + if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) + return -errno; + + have_close_range = false; + } } /* Fallback on OOM or if close_range() is not supported */ @@ -348,7 +387,7 @@ int close_all_fds(const int except[], size_t n_except) { d = opendir("/proc/self/fd"); if (!d) - return close_all_fds_without_malloc(except, n_except); /* ultimate fallback if /proc/ is not available */ + return close_all_fds_frugal(except, n_except); /* ultimate fallback if /proc/ is not available */ FOREACH_DIRENT(de, d, return -errno) { int fd = -1, q; From ab27b2fe56c6c4bd0295b248448adb1c698e9284 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 29 Jul 2021 16:50:44 +0200 Subject: [PATCH 08/10] exec-util: use close_all_fds_without_malloc() from freeze() --- 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 5c56a59aabe..06493f77995 100644 --- a/src/basic/process-util.c +++ b/src/basic/process-util.c @@ -1620,8 +1620,10 @@ bool invoked_as(char *argv[], const char *token) { _noreturn_ void freeze(void) { log_close(); - /* Make sure nobody waits for us on a socket anymore */ - (void) 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); /* Let's not freeze right away, but keep reaping zombies. */ for (;;) { From 73fc0cbc87131f9a034d3a348bcbb606ccebecbc Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 12 Aug 2021 10:46:10 +0200 Subject: [PATCH 09/10] fd-util: export get_max_fd() so that we can use it in tests --- src/basic/fd-util.c | 2 +- src/basic/fd-util.h | 2 ++ src/test/test-fd-util.c | 8 ++++---- 3 files changed, 7 insertions(+), 5 deletions(-) diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index 50666e6375c..63c37fec4d8 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -187,7 +187,7 @@ _pure_ static bool fd_in_set(int fd, const int fdset[], size_t n_fdset) { return false; } -static int get_max_fd(void) { +int get_max_fd(void) { struct rlimit rl; rlim_t m; diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index e929386b539..dd5207bd88a 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -57,6 +57,8 @@ DEFINE_TRIVIAL_CLEANUP_FUNC_FULL(DIR*, closedir, NULL); int fd_nonblock(int fd, bool nonblock); int fd_cloexec(int fd, bool cloexec); +int get_max_fd(void); + int close_all_fds(const int except[], size_t n_except); int close_all_fds_without_malloc(const int except[], size_t n_except); diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index 4c51592c263..6001f8e057a 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -215,18 +215,18 @@ static size_t validate_fds( static void test_close_all_fds(void) { _cleanup_free_ int *fds = NULL, *keep = NULL; - struct rlimit rl; size_t n_fds, n_keep; + int max_fd; log_info("/* %s */", __func__); rlimit_nofile_bump(-1); - assert_se(getrlimit(RLIMIT_NOFILE, &rl) >= 0); - assert_se(rl.rlim_cur > 10); + max_fd = get_max_fd(); + assert_se(max_fd > 10); /* Try to use 5000 fds, but when we can't bump the rlimit to make that happen use the whole limit minus 10 */ - n_fds = MIN((rl.rlim_cur & ~1U) - 10U, 5000U); + n_fds = MIN(((size_t) max_fd & ~1U) - 10U, 5000U); assert_se((n_fds & 1U) == 0U); /* make sure even number of fds */ /* Allocate the determined number of fds, always two at a time */ From b68919724118e6fa4b29e7b7f8ceb54a127cca5d Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Thu, 12 Aug 2021 11:22:50 +0200 Subject: [PATCH 10/10] test-fd-util: extend close_all_fds() test to trigger all fallback codepaths This extends the close_all_fds() logic to overmount /proc with an empty tmpfs, and/or to block close_range() via seccomp, so that we run the test case for the function with the fallback paths. This should make sure that we don't regress in limited environments or older kernels. --- src/test/meson.build | 4 +- src/test/test-fd-util.c | 107 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/src/test/meson.build b/src/test/meson.build index 292b6329e46..7b4df45aac9 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -227,7 +227,9 @@ tests += [ [['src/test/test-proc-cmdline.c']], - [['src/test/test-fd-util.c']], + [['src/test/test-fd-util.c'], + [], + [libseccomp]], [['src/test/test-web-util.c']], diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index 6001f8e057a..0aa229fbc9a 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -9,10 +9,13 @@ #include "fileio.h" #include "macro.h" #include "memory-util.h" +#include "missing_syscall.h" +#include "mount-util.h" #include "path-util.h" #include "process-util.h" #include "random-util.h" #include "rlimit-util.h" +#include "seccomp-util.h" #include "serialize.h" #include "string-util.h" #include "tests.h" @@ -213,7 +216,7 @@ static size_t validate_fds( return c; /* Return number of fds >= 0 in the array */ } -static void test_close_all_fds(void) { +static void test_close_all_fds_inner(void) { _cleanup_free_ int *fds = NULL, *keep = NULL; size_t n_fds, n_keep; int max_fd; @@ -225,6 +228,15 @@ static void test_close_all_fds(void) { max_fd = get_max_fd(); assert_se(max_fd > 10); + if (max_fd > 7000) { + /* If the worst fallback is activated we need to iterate through all possible fds, hence, + * let's lower the limit a small bit, so that we don't run for too long. Yes, this undoes the + * rlimit_nofile_bump() call above partially. */ + + (void) setrlimit_closest(RLIMIT_NOFILE, &(struct rlimit) { 7000, 7000 }); + max_fd = 7000; + } + /* Try to use 5000 fds, but when we can't bump the rlimit to make that happen use the whole limit minus 10 */ n_fds = MIN(((size_t) max_fd & ~1U) - 10U, 5000U); assert_se((n_fds & 1U) == 0U); /* make sure even number of fds */ @@ -278,6 +290,99 @@ static void test_close_all_fds(void) { log_open(); } +static int seccomp_prohibit_close_range(void) { +#if defined(HAVE_SECCOMP) && defined(__SNR_close_range) + _cleanup_(seccomp_releasep) scmp_filter_ctx seccomp = NULL; + int r; + + r = seccomp_init_for_arch(&seccomp, SCMP_ARCH_NATIVE, SCMP_ACT_ALLOW); + if (r < 0) + return log_warning_errno(r, "Failed to acquire seccomp context, ignoring: %m"); + + r = seccomp_rule_add_exact( + seccomp, + SCMP_ACT_ERRNO(EPERM), + SCMP_SYS(close_range), + 0); + if (r < 0) + return log_warning_errno(r, "Failed to add close_range() rule, ignoring: %m"); + + r = seccomp_load(seccomp); + if (r < 0) + return log_warning_errno(r, "Failed to apply close_range() restrictions, ignoring: %m"); + + return 0; +#else + return log_warning_errno(SYNTHETIC_ERRNO(EOPNOTSUPP), "Seccomp support or close_range() syscall definition not availeble."); +#endif +} + +static void test_close_all_fds(void) { + int r; + + /* Runs the test four times. Once as is. Once with close_range() syscall blocked via seccomp, once + * with /proc overmounted, and once with the combination of both. This should trigger all fallbacks in + * the close_range_all() function. */ + + r = safe_fork("(caf-plain)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); + if (r == 0) { + test_close_all_fds_inner(); + _exit(EXIT_SUCCESS); + } + assert_se(r >= 0); + + if (geteuid() != 0) { + log_notice("Lacking privileges, skipping running tests with blocked close_range() and with /proc/ overnmounted."); + return; + } + + r = safe_fork("(caf-noproc)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_LOG|FORK_WAIT|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE, NULL); + if (r == 0) { + r = mount_nofollow_verbose(LOG_WARNING, "tmpfs", "/proc", "tmpfs", 0, NULL); + if (r < 0) + log_notice("Overmounting /proc didn#t work, skipping close_all_fds() with masked /proc/."); + else + test_close_all_fds_inner(); + _exit(EXIT_SUCCESS); + } + assert_se(r >= 0); + + if (!is_seccomp_available()) { + log_notice("Seccomp not available, skipping seccomp tests in %s", __func__); + return; + } + + r = safe_fork("(caf-seccomp)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_LOG|FORK_WAIT, NULL); + if (r == 0) { + r = seccomp_prohibit_close_range(); + if (r < 0) + log_notice("Applying seccomp filter didn't work, skipping close_all_fds() test with masked close_range()."); + else + test_close_all_fds_inner(); + + _exit(EXIT_SUCCESS); + } + assert_se(r >= 0); + + r = safe_fork("(caf-scnp)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG|FORK_LOG|FORK_WAIT|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE, NULL); + if (r == 0) { + r = seccomp_prohibit_close_range(); + if (r < 0) + log_notice("Applying seccomp filter didn't work, skipping close_all_fds() test with masked close_range()."); + else { + r = mount_nofollow_verbose(LOG_WARNING, "tmpfs", "/proc", "tmpfs", 0, NULL); + if (r < 0) + log_notice("Overmounting /proc didn#t work, skipping close_all_fds() with masked /proc/."); + else + test_close_all_fds_inner(); + } + + test_close_all_fds_inner(); + _exit(EXIT_SUCCESS); + } + assert_se(r >= 0); +} + static void test_format_proc_fd_path(void) { assert_se(streq_ptr(FORMAT_PROC_FD_PATH(0), "/proc/self/fd/0")); assert_se(streq_ptr(FORMAT_PROC_FD_PATH(1), "/proc/self/fd/1"));