Revert "basic/fd-util: sort the 'except' array in place"

This reverts commit 9c46228b7deb53d6384545535b37b2844a102b2b.

6 files changed, 91 insertions, 66 deletions

diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c
index 032f303789..dec083daaf 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;
+
+                        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) {
+                                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;
 
-                        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);
+                                typesafe_qsort(sorted, n_sorted, cmp_int);
 
-                                assert(end >= start);
+                                for (size_t i = 0; i < n_sorted-1; i++) {
+                                        int start, end;
 
-                                if (end - start <= 1)
-                                        continue;
+                                        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
-                                                return -errno;
-                                        goto opendir_fallback;
+                                                break;
+                                        }
+
+                                        c += end - start - 1;
                                 }
-                        }
 
-                        /* The loop succeeded. Let's now close everything beyond the end */
+                                if (have_close_range) {
+                                        /* 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;
+                                        if (sorted[n_sorted-1] >= INT_MAX) /* Dont let the addition below overflow */
+                                                return c;
 
-                        int start = MAX(except[n_except-1], 2);
+                                        if (close_range(sorted[n_sorted-1] + 1, -1, 0) >= 0)
+                                                return c + 1;
 
-                        if (close_range(start + 1, -1, 0) >= 0)
-                                return 0;
+                                        if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno))
+                                                return -errno;
 
-                        if (ERRNO_IS_NOT_SUPPORTED(errno) || ERRNO_IS_PRIVILEGE(errno))
-                                have_close_range = false;
-                        else
-                                return -errno;
+                                        have_close_range = false;
+                                }
+                        }
                 }
+
+                /* 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 ab841b67e0..459059c64e 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 fef0c742c7..5c56a59aab 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 451d0a5ff4..c7622c98df 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 fd0d95c530..f0a9ea9e9f 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 21d28608f9..ba4506e5aa 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_;