From: Yu Watanabe Date: Tue, 19 Aug 2025 20:38:30 +0000 (+0900) Subject: fd-util: split out fallback logic for close_all_fds() X-Git-Tag: v259-rc1~562^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=93d037dbd09bc95126008fb19e9b129b8236d7a0;p=thirdparty%2Fsystemd.git fd-util: split out fallback logic for close_all_fds() No functional change. Just refactoring. With this change, we can test each logic directly without seccomp or hiding procfs. --- diff --git a/src/basic/fd-util.c b/src/basic/fd-util.c index a6118b4247a..281cdfcc038 100644 --- a/src/basic/fd-util.c +++ b/src/basic/fd-util.c @@ -248,7 +248,7 @@ int get_max_fd(void) { return (int) (m - 1); } -static int close_all_fds_frugal(const int except[], size_t n_except) { +int close_all_fds_frugal(const int except[], size_t n_except) { int max_fd, r = 0; assert(except || n_except == 0); @@ -282,6 +282,42 @@ static int close_all_fds_frugal(const int except[], size_t n_except) { return r; } +int close_all_fds_by_proc(const int except[], size_t n_except) { + _cleanup_closedir_ DIR *d = NULL; + int r = 0; + + d = opendir("/proc/self/fd"); + if (!d) + return close_all_fds_frugal(except, n_except); /* ultimate fallback if /proc/ is not available */ + + FOREACH_DIRENT(de, d, return -errno) { + int fd = -EBADF, q; + + if (!IN_SET(de->d_type, DT_LNK, DT_UNKNOWN)) + continue; + + fd = parse_fd(de->d_name); + if (fd < 0) + /* Let's better ignore this, just in case */ + continue; + + if (fd < 3) + continue; + + if (fd == dirfd(d)) + continue; + + if (fd_in_set(fd, except, n_except)) + continue; + + q = close_nointr(fd); + if (q != -EBADF) /* Valgrind has its own FD and doesn't want to have it closed */ + RET_GATHER(r, q); + } + + 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) { @@ -348,8 +384,7 @@ int close_all_fds_without_malloc(const int except[], size_t n_except) { } int close_all_fds(const int except[], size_t n_except) { - _cleanup_closedir_ DIR *d = NULL; - int r = 0; + int r; assert(n_except == 0 || except); @@ -359,104 +394,73 @@ int close_all_fds(const int except[], size_t n_except) { 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. */ - - 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 (!have_close_range) + return close_all_fds_by_proc(except, n_except); - if (sorted) { - memcpy(sorted, except, n_except * sizeof(int)); + _cleanup_free_ int *sorted_malloc = NULL; + size_t n_sorted; + int *sorted; - /* 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; + /* 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. */ - typesafe_qsort(sorted, n_sorted, cmp_int); + assert(n_except < SIZE_MAX); + n_sorted = n_except + 1; - for (size_t i = 0; i < n_sorted-1; i++) { - int start, end; + 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); - start = MAX(sorted[i], 2); /* The first three fds shall always remain open */ - end = MAX(sorted[i+1], 2); + if (!sorted) /* Fallback on OOM. */ + return close_all_fds_by_proc(except, n_except); - assert(end >= start); + memcpy(sorted, except, n_except * sizeof(int)); - if (end - start <= 1) - continue; + /* 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; - /* 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; + typesafe_qsort(sorted, n_sorted, cmp_int); - have_close_range = false; - break; - } - } + for (size_t i = 0; i < n_sorted-1; i++) { + int start, end; - if (have_close_range) { - /* The loop succeeded. Let's now close everything beyond the end */ + start = MAX(sorted[i], 2); /* The first three fds shall always remain open */ + end = MAX(sorted[i+1], 2); - if (sorted[n_sorted-1] >= INT_MAX) /* Dont let the addition below overflow */ - return 0; + assert(end >= start); - if (close_range(sorted[n_sorted-1] + 1, INT_MAX, 0) >= 0) - return 0; + if (end - start <= 1) + continue; - 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)) + return -errno; - have_close_range = false; - } + have_close_range = false; + return close_all_fds_by_proc(except, n_except); } - - /* Fallback on OOM or if close_range() is not supported */ } - d = opendir("/proc/self/fd"); - if (!d) - return close_all_fds_frugal(except, n_except); /* ultimate fallback if /proc/ is not available */ + /* The loop succeeded. Let's now close everything beyond the end */ - FOREACH_DIRENT(de, d, return -errno) { - int fd = -EBADF, q; - - if (!IN_SET(de->d_type, DT_LNK, DT_UNKNOWN)) - continue; - - fd = parse_fd(de->d_name); - if (fd < 0) - /* Let's better ignore this, just in case */ - continue; - - if (fd < 3) - continue; - - if (fd == dirfd(d)) - continue; + if (sorted[n_sorted-1] >= INT_MAX) /* Dont let the addition below overflow */ + return 0; - if (fd_in_set(fd, except, n_except)) - continue; + if (close_range(sorted[n_sorted-1] + 1, INT_MAX, 0) < 0) { + if (!ERRNO_IS_NOT_SUPPORTED(errno) && !ERRNO_IS_PRIVILEGE(errno)) + return -errno; - q = close_nointr(fd); - if (q < 0 && q != -EBADF && r >= 0) /* Valgrind has its own FD and doesn't want to have it closed */ - r = q; + have_close_range = false; + return close_all_fds_by_proc(except, n_except); } - return r; + return 0; } int pack_fds(int fds[], size_t n_fds) { diff --git a/src/basic/fd-util.h b/src/basic/fd-util.h index d759300e4ff..94caf06e2fc 100644 --- a/src/basic/fd-util.h +++ b/src/basic/fd-util.h @@ -112,6 +112,8 @@ 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); +int close_all_fds_by_proc(const int except[], size_t n_except); +int close_all_fds_frugal(const int except[], size_t n_except); int pack_fds(int fds[], size_t n); diff --git a/src/test/test-fd-util.c b/src/test/test-fd-util.c index 621e611de3c..f6ab57f58a8 100644 --- a/src/test/test-fd-util.c +++ b/src/test/test-fd-util.c @@ -260,7 +260,7 @@ static size_t validate_fds( return c; /* Return number of fds >= 0 in the array */ } -static void test_close_all_fds_inner(void) { +static void test_close_all_fds_inner(int (*func)(const int except[], size_t n_except)) { _cleanup_free_ int *fds = NULL, *keep = NULL; size_t n_fds, n_keep; int max_fd; @@ -320,13 +320,13 @@ static void test_close_all_fds_inner(void) { log_settle_target(); /* Close all but the ones to keep */ - assert_se(close_all_fds(keep, n_keep) >= 0); + ASSERT_OK(func(keep, n_keep)); assert_se(validate_fds(false, fds, n_fds) == n_fds - n_keep); assert_se(validate_fds(true, keep, n_keep) == n_keep); /* Close everything else too! */ - assert_se(close_all_fds(NULL, 0) >= 0); + ASSERT_OK(func(NULL, 0)); assert_se(validate_fds(false, fds, n_fds) == n_fds - n_keep); assert_se(validate_fds(false, keep, n_keep) == n_keep); @@ -335,95 +335,32 @@ static void test_close_all_fds_inner(void) { log_open(); } -static int seccomp_prohibit_close_range(void) { -#if 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 available."); -#endif -} - TEST(close_all_fds) { 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_SIGTERM|FORK_LOG|FORK_WAIT, NULL); + ASSERT_OK(r = safe_fork("(caf-plain)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_WAIT, NULL)); if (r == 0) { - test_close_all_fds_inner(); + test_close_all_fds_inner(close_all_fds); _exit(EXIT_SUCCESS); } - assert_se(r >= 0); - - if (geteuid() != 0) - return (void) log_tests_skipped("Lacking privileges for test with close_range() blocked and /proc/ overmounted"); - r = safe_fork("(caf-noproc)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_WAIT|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE, NULL); + ASSERT_OK(r = safe_fork("(caf-nomalloc)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_WAIT, 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(); + test_close_all_fds_inner(close_all_fds_without_malloc); _exit(EXIT_SUCCESS); } - if (ERRNO_IS_NEG_PRIVILEGE(r)) - return (void) log_tests_skipped("Lacking privileges for test in namespace with /proc/ overmounted"); - assert_se(r >= 0); - - if (!is_seccomp_available()) - return (void) log_tests_skipped("Seccomp not available"); - r = safe_fork("(caf-seccomp)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_WAIT, NULL); + ASSERT_OK(r = safe_fork("(caf-proc)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|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(); - + test_close_all_fds_inner(close_all_fds_by_proc); _exit(EXIT_SUCCESS); } - assert_se(r >= 0); - r = safe_fork("(caf-scnp)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|FORK_LOG|FORK_WAIT|FORK_NEW_MOUNTNS|FORK_MOUNTNS_SLAVE, NULL); + ASSERT_OK(r = safe_fork("(caf-frugal)", FORK_CLOSE_ALL_FDS|FORK_DEATHSIG_SIGTERM|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 { - 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(); + test_close_all_fds_inner(close_all_fds_frugal); _exit(EXIT_SUCCESS); } - assert_se(r >= 0); } TEST(format_proc_fd_path) {