1
0
mirror of https://github.com/systemd/systemd synced 2026-04-03 21:54:58 +02:00

Compare commits

..

12 Commits

Author SHA1 Message Date
Lennart Poettering
aedec452b9 tree-wide: always use TAKE_FD() when calling rearrange_stdio()
rearrange_stdio() invalidates specified fds even on failure, which means
we should always invalidate the fds we pass in no matter what. Let's
make this explicit by using TAKE_FD() for that everywhere.

Note that in many places we such invalidation doesnt get us much
behaviour-wise, since we don't use the variables anymore later. But
TAKE_FD() in a way is also documentation, it encodes explicitly that the
fds are invalidated here, so I think it's a good thing to always make
this explicit here.
2021-11-03 23:05:26 +00:00
Yu Watanabe
829b86bc0f
Merge pull request #21217 from keszybz/debug-test-process-util
procfs-util: fix confusion wrt. quantity limit and maximum value
2021-11-04 04:03:56 +09:00
Yu Watanabe
3f5e7edbdb
Merge pull request #21216 from poettering/take-fd-tweak
fd-util: make TAKE_FD free of double evaluation
2021-11-04 04:01:45 +09:00
Lennart Poettering
b7759c8f0a macro: make TAKE_PTR() side-effect free 2021-11-03 16:36:19 +01:00
Lennart Poettering
7950211df3 tree-wide: port more code to sigkill_wait() 2021-11-03 16:36:19 +01:00
Lennart Poettering
8f03de5323 tree-wide: port various places to use TAKE_PID() 2021-11-03 16:36:09 +01:00
Lennart Poettering
883946f0d2 process-util: rework TAKE_PID() to be side-effect free 2021-11-03 15:57:02 +01:00
Lennart Poettering
2c1612100d process-util: wait for processes we killed even if killing failed
The processes might be zombies in which case killing will fail, but
reaping them still matters.
2021-11-03 15:57:02 +01:00
Lennart Poettering
a70877d881 test: add test that ensures TAKE_FD() works as it should 2021-11-03 15:57:02 +01:00
Lennart Poettering
90d59676b3 fd-util: make TAKE_FD free of double evaluation
Better be safe than sorry.
2021-11-03 15:22:18 +01:00
Zbigniew Jędrzejewski-Szmek
6434a83d01 test-process-util: also add EROFS to the list of "good" errors
It is only added in the one place where we actually try to set the
setting to a new value. Before we were testing if we can set to it the
existing value, which was a noop. We could still get a permission error,
but this is the first place where we would propagate EROFS.
2021-11-03 09:39:16 +01:00
Zbigniew Jędrzejewski-Szmek
c3dead53d5 procfs-util: fix confusion wrt. quantity limit and maximum value
From packit/rawhide-arm64 logs:
Assertion 'limit >= INT_MAX || get_process_ppid(limit+1, NULL) == -ESRCH' failed at src/test/test-process-util.c:855, function test_get_process_ppid(). Aborting.
――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――――

The kernel has a few different limits. In particular kernel.threads-max can be
set to some lower value, and kernel.pid_max can be set to a higher value. This
is nice because it reduces PID reuse, even if the number of threads that is
allowed is limited. But the tests assumed that we cannot have a thread with
PID above MIN(kernel.threads-max, kernel.pid_max-1), which is not valid.

So let's rework the whole thing: let's expose the helpers to read
kernel.threads-max and kernel.pid_max, and print what they return in tests.
procfs_tasks_get_limit() was something that is only used in tests, and wasn't
very well defined, so let's drop it.

Fixes #21193.
2021-11-03 09:36:08 +01:00
33 changed files with 196 additions and 153 deletions

View File

@ -91,9 +91,10 @@ static inline int make_null_stdio(void) {
/* Like TAKE_PTR() but for file descriptors, resetting them to -1 */ /* Like TAKE_PTR() but for file descriptors, resetting them to -1 */
#define TAKE_FD(fd) \ #define TAKE_FD(fd) \
({ \ ({ \
int _fd_ = (fd); \ int *_fd_ = &(fd); \
(fd) = -1; \ int _ret_ = *_fd_; \
_fd_; \ *_fd_ = -1; \
_ret_; \
}) })
/* Like free_and_replace(), but for file descriptors */ /* Like free_and_replace(), but for file descriptors */

View File

@ -111,35 +111,57 @@ uint64_t physical_memory_scale(uint64_t v, uint64_t max) {
} }
uint64_t system_tasks_max(void) { uint64_t system_tasks_max(void) {
uint64_t a = TASKS_MAX, b = TASKS_MAX; uint64_t a = TASKS_MAX, b = TASKS_MAX, c = TASKS_MAX;
_cleanup_free_ char *root = NULL; _cleanup_free_ char *root = NULL;
int r; int r;
/* Determine the maximum number of tasks that may run on this system. We check three sources to determine this /* Determine the maximum number of tasks that may run on this system. We check three sources to
* limit: * determine this limit:
* *
* a) the maximum tasks value the kernel allows on this architecture * a) kernel.threads-max sysctl: the maximum number of tasks (threads) the kernel allows.
* b) the cgroups pids_max attribute for the system
* c) the kernel's configured maximum PID value
* *
* And then pick the smallest of the three */ * This puts a direct limit on the number of concurrent tasks.
*
* b) kernel.pid_max sysctl: the maximum PID value.
*
* This limits the numeric range PIDs can take, and thus indirectly also limits the number of
* concurrent threads. It's primarily a compatibility concept: some crappy old code used a signed
* 16bit type for PIDs, hence the kernel provides a way to ensure the PIDs never go beyond
* INT16_MAX by default.
*
* Also note the weird definition: PIDs assigned will be kept below this value, which means
* the number of tasks that can be created is one lower, as PID 0 is not a valid process ID.
*
* c) pids.max on the root cgroup: the kernel's configured maximum number of tasks.
*
* and then pick the smallest of the three.
*
* By default pid_max is set to much lower values than threads-max, hence the limit people come into
* contact with first, as it's the lowest boundary they need to bump when they want higher number of
* processes.
*/
r = procfs_tasks_get_limit(&a); r = procfs_get_threads_max(&a);
if (r < 0) if (r < 0)
log_debug_errno(r, "Failed to read maximum number of tasks from /proc, ignoring: %m"); log_debug_errno(r, "Failed to read kernel.threads-max, ignoring: %m");
r = procfs_get_pid_max(&b);
if (r < 0)
log_debug_errno(r, "Failed to read kernel.pid_max, ignoring: %m");
else if (b > 0)
/* Subtract one from pid_max, since PID 0 is not a valid PID */
b--;
r = cg_get_root_path(&root); r = cg_get_root_path(&root);
if (r < 0) if (r < 0)
log_debug_errno(r, "Failed to determine cgroup root path, ignoring: %m"); log_debug_errno(r, "Failed to determine cgroup root path, ignoring: %m");
else { else {
r = cg_get_attribute_as_uint64("pids", root, "pids.max", &b); r = cg_get_attribute_as_uint64("pids", root, "pids.max", &c);
if (r < 0) if (r < 0)
log_debug_errno(r, "Failed to read pids.max attribute of cgroup root, ignoring: %m"); log_debug_errno(r, "Failed to read pids.max attribute of root cgroup, ignoring: %m");
} }
return MIN3(TASKS_MAX, return MIN3(a, b, c);
a <= 0 ? TASKS_MAX : a,
b <= 0 ? TASKS_MAX : b);
} }
uint64_t system_tasks_max_scale(uint64_t v, uint64_t max) { uint64_t system_tasks_max_scale(uint64_t v, uint64_t max) {

View File

@ -857,8 +857,8 @@ int wait_for_terminate_with_timeout(pid_t pid, usec_t timeout) {
void sigkill_wait(pid_t pid) { void sigkill_wait(pid_t pid) {
assert(pid > 1); assert(pid > 1);
if (kill(pid, SIGKILL) >= 0) (void) kill(pid, SIGKILL);
(void) wait_for_terminate(pid, NULL); (void) wait_for_terminate(pid, NULL);
} }
void sigkill_waitp(pid_t *pid) { void sigkill_waitp(pid_t *pid) {
@ -875,8 +875,8 @@ void sigkill_waitp(pid_t *pid) {
void sigterm_wait(pid_t pid) { void sigterm_wait(pid_t pid) {
assert(pid > 1); assert(pid > 1);
if (kill_and_sigcont(pid, SIGTERM) >= 0) (void) kill_and_sigcont(pid, SIGTERM);
(void) wait_for_terminate(pid, NULL); (void) wait_for_terminate(pid, NULL);
} }
int kill_and_sigcont(pid_t pid, int sig) { int kill_and_sigcont(pid_t pid, int sig) {

View File

@ -192,8 +192,9 @@ assert_cc(TASKS_MAX <= (unsigned long) PID_T_MAX);
/* Like TAKE_PTR() but for child PIDs, resetting them to 0 */ /* Like TAKE_PTR() but for child PIDs, resetting them to 0 */
#define TAKE_PID(pid) \ #define TAKE_PID(pid) \
({ \ ({ \
pid_t _pid_ = (pid); \ pid_t *_ppid_ = &(pid); \
(pid) = 0; \ pid_t _pid_ = *_ppid_; \
*_ppid_ = 0; \
_pid_; \ _pid_; \
}) })

View File

@ -13,54 +13,34 @@
#include "stdio-util.h" #include "stdio-util.h"
#include "string-util.h" #include "string-util.h"
int procfs_tasks_get_limit(uint64_t *ret) { int procfs_get_pid_max(uint64_t *ret) {
_cleanup_free_ char *value = NULL; _cleanup_free_ char *value = NULL;
uint64_t pid_max, threads_max;
int r; int r;
assert(ret); assert(ret);
/* So there are two sysctl files that control the system limit of processes:
*
* 1. kernel.threads-max: this is probably the sysctl that makes more sense, as it directly puts a limit on
* concurrent tasks.
*
* 2. kernel.pid_max: this limits the numeric range PIDs can take, and thus indirectly also limits the number
* of concurrent threads. AFAICS it's primarily a compatibility concept: some crappy old code used a signed
* 16bit type for PIDs, hence the kernel provides a way to ensure the PIDs never go beyond INT16_MAX by
* default.
*
* By default #2 is set to much lower values than #1, hence the limit people come into contact with first, as
* it's the lowest boundary they need to bump when they want higher number of processes.
*
* Also note the weird definition of #2: PIDs assigned will be kept below this value, which means the number of
* tasks that can be created is one lower, as PID 0 is not a valid process ID. */
r = read_one_line_file("/proc/sys/kernel/pid_max", &value); r = read_one_line_file("/proc/sys/kernel/pid_max", &value);
if (r < 0) if (r < 0)
return r; return r;
r = safe_atou64(value, &pid_max); return safe_atou64(value, ret);
if (r < 0) }
return r;
int procfs_get_threads_max(uint64_t *ret) {
_cleanup_free_ char *value = NULL;
int r;
assert(ret);
value = mfree(value);
r = read_one_line_file("/proc/sys/kernel/threads-max", &value); r = read_one_line_file("/proc/sys/kernel/threads-max", &value);
if (r < 0) if (r < 0)
return r; return r;
r = safe_atou64(value, &threads_max); return safe_atou64(value, ret);
if (r < 0)
return r;
/* Subtract one from pid_max, since PID 0 is not a valid PID */
*ret = MIN(pid_max-1, threads_max);
return 0;
} }
int procfs_tasks_set_limit(uint64_t limit) { int procfs_tasks_set_limit(uint64_t limit) {
char buffer[DECIMAL_STR_MAX(uint64_t)+1]; char buffer[DECIMAL_STR_MAX(uint64_t)+1];
_cleanup_free_ char *value = NULL;
uint64_t pid_max; uint64_t pid_max;
int r; int r;
@ -75,10 +55,7 @@ int procfs_tasks_set_limit(uint64_t limit) {
* set it to the maximum. */ * set it to the maximum. */
limit = CLAMP(limit, 20U, TASKS_MAX); limit = CLAMP(limit, 20U, TASKS_MAX);
r = read_one_line_file("/proc/sys/kernel/pid_max", &value); r = procfs_get_pid_max(&pid_max);
if (r < 0)
return r;
r = safe_atou64(value, &pid_max);
if (r < 0) if (r < 0)
return r; return r;
@ -99,14 +76,10 @@ int procfs_tasks_set_limit(uint64_t limit) {
/* Hmm, we couldn't write this? If so, maybe it was already set properly? In that case let's not /* Hmm, we couldn't write this? If so, maybe it was already set properly? In that case let's not
* generate an error */ * generate an error */
value = mfree(value); if (procfs_get_threads_max(&threads_max) < 0)
if (read_one_line_file("/proc/sys/kernel/threads-max", &value) < 0)
return r; /* return original error */ return r; /* return original error */
if (safe_atou64(value, &threads_max) < 0) if (MIN(pid_max - 1, threads_max) != limit)
return r; /* return original error */
if (MIN(pid_max-1, threads_max) != limit)
return r; /* return original error */ return r; /* return original error */
/* Yay! Value set already matches what we were trying to set, hence consider this a success. */ /* Yay! Value set already matches what we were trying to set, hence consider this a success. */

View File

@ -5,7 +5,9 @@
#include "time-util.h" #include "time-util.h"
int procfs_tasks_get_limit(uint64_t *ret); int procfs_get_pid_max(uint64_t *ret);
int procfs_get_threads_max(uint64_t *ret);
int procfs_tasks_set_limit(uint64_t limit); int procfs_tasks_set_limit(uint64_t limit);
int procfs_tasks_get_current(uint64_t *ret); int procfs_tasks_get_current(uint64_t *ret);

View File

@ -756,12 +756,16 @@ static int chown_terminal(int fd, uid_t uid) {
return 1; return 1;
} }
static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_stdout) { static int setup_confirm_stdio(
const char *vc,
int *ret_saved_stdin,
int *ret_saved_stdout) {
_cleanup_close_ int fd = -1, saved_stdin = -1, saved_stdout = -1; _cleanup_close_ int fd = -1, saved_stdin = -1, saved_stdout = -1;
int r; int r;
assert(_saved_stdin); assert(ret_saved_stdin);
assert(_saved_stdout); assert(ret_saved_stdout);
saved_stdin = fcntl(STDIN_FILENO, F_DUPFD, 3); saved_stdin = fcntl(STDIN_FILENO, F_DUPFD, 3);
if (saved_stdin < 0) if (saved_stdin < 0)
@ -783,16 +787,13 @@ static int setup_confirm_stdio(const char *vc, int *_saved_stdin, int *_saved_st
if (r < 0) if (r < 0)
return r; return r;
r = rearrange_stdio(fd, fd, STDERR_FILENO); r = rearrange_stdio(fd, fd, STDERR_FILENO); /* Invalidates 'fd' also on failure */
fd = -1; TAKE_FD(fd);
if (r < 0) if (r < 0)
return r; return r;
*_saved_stdin = saved_stdin; *ret_saved_stdin = TAKE_FD(saved_stdin);
*_saved_stdout = saved_stdout; *ret_saved_stdout = TAKE_FD(saved_stdout);
saved_stdin = saved_stdout = -1;
return 0; return 0;
} }
@ -2248,8 +2249,7 @@ static int setup_private_users(uid_t ouid, gid_t ogid, uid_t uid, gid_t gid) {
if (n != 0) /* on success we should have read 0 bytes */ if (n != 0) /* on success we should have read 0 bytes */
return -EIO; return -EIO;
r = wait_for_terminate_and_check("(sd-userns)", pid, 0); r = wait_for_terminate_and_check("(sd-userns)", TAKE_PID(pid), 0);
pid = 0;
if (r < 0) if (r < 0)
return r; return r;
if (r != EXIT_SUCCESS) /* If something strange happened with the child, let's consider this fatal, too */ if (r != EXIT_SUCCESS) /* If something strange happened with the child, let's consider this fatal, too */

View File

@ -236,8 +236,7 @@ static void mount_unwatch_control_pid(Mount *m) {
if (m->control_pid <= 0) if (m->control_pid <= 0)
return; return;
unit_unwatch_pid(UNIT(m), m->control_pid); unit_unwatch_pid(UNIT(m), TAKE_PID(m->control_pid));
m->control_pid = 0;
} }
static void mount_parameters_done(MountParameters *p) { static void mount_parameters_done(MountParameters *p) {

View File

@ -130,8 +130,7 @@ static void service_unwatch_control_pid(Service *s) {
if (s->control_pid <= 0) if (s->control_pid <= 0)
return; return;
unit_unwatch_pid(UNIT(s), s->control_pid); unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid));
s->control_pid = 0;
} }
static void service_unwatch_main_pid(Service *s) { static void service_unwatch_main_pid(Service *s) {
@ -140,8 +139,7 @@ static void service_unwatch_main_pid(Service *s) {
if (s->main_pid <= 0) if (s->main_pid <= 0)
return; return;
unit_unwatch_pid(UNIT(s), s->main_pid); unit_unwatch_pid(UNIT(s), TAKE_PID(s->main_pid));
s->main_pid = 0;
} }
static void service_unwatch_pid_file(Service *s) { static void service_unwatch_pid_file(Service *s) {

View File

@ -109,8 +109,7 @@ static void socket_unwatch_control_pid(Socket *s) {
if (s->control_pid <= 0) if (s->control_pid <= 0)
return; return;
unit_unwatch_pid(UNIT(s), s->control_pid); unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid));
s->control_pid = 0;
} }
static void socket_cleanup_fd_list(SocketPort *p) { static void socket_cleanup_fd_list(SocketPort *p) {

View File

@ -155,8 +155,7 @@ static void swap_unwatch_control_pid(Swap *s) {
if (s->control_pid <= 0) if (s->control_pid <= 0)
return; return;
unit_unwatch_pid(UNIT(s), s->control_pid); unit_unwatch_pid(UNIT(s), TAKE_PID(s->control_pid));
s->control_pid = 0;
} }
static void swap_done(Unit *u) { static void swap_done(Unit *u) {

View File

@ -257,8 +257,9 @@
* resets it to NULL. See: https://doc.rust-lang.org/std/option/enum.Option.html#method.take */ * resets it to NULL. See: https://doc.rust-lang.org/std/option/enum.Option.html#method.take */
#define TAKE_PTR(ptr) \ #define TAKE_PTR(ptr) \
({ \ ({ \
typeof(ptr) _ptr_ = (ptr); \ typeof(ptr) *_pptr_ = &(ptr); \
(ptr) = NULL; \ typeof(ptr) _ptr_ = *_pptr_; \
*_pptr_ = NULL; \
_ptr_; \ _ptr_; \
}) })

View File

@ -1202,13 +1202,12 @@ static int home_start_work(Home *h, const char *verb, UserRecord *hr, UserRecord
if (r < 0) if (r < 0)
log_warning_errno(r, "Failed to update $SYSTEMD_EXEC_PID, ignoring: %m"); log_warning_errno(r, "Failed to update $SYSTEMD_EXEC_PID, ignoring: %m");
r = rearrange_stdio(stdin_fd, stdout_fd, STDERR_FILENO); r = rearrange_stdio(TAKE_FD(stdin_fd), TAKE_FD(stdout_fd), STDERR_FILENO); /* fds are invalidated by rearrange_stdio() even on failure */
if (r < 0) { if (r < 0) {
log_error_errno(r, "Failed to rearrange stdin/stdout/stderr: %m"); log_error_errno(r, "Failed to rearrange stdin/stdout/stderr: %m");
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);
} }
stdin_fd = stdout_fd = -1; /* have been invalidated by rearrange_stdio() */
/* Allow overriding the homework path via an environment variable, to make debugging /* Allow overriding the homework path via an environment variable, to make debugging
* easier. */ * easier. */

View File

@ -56,10 +56,8 @@ TarExport *tar_export_unref(TarExport *e) {
sd_event_source_unref(e->output_event_source); sd_event_source_unref(e->output_event_source);
if (e->tar_pid > 1) { if (e->tar_pid > 1)
(void) kill_and_sigcont(e->tar_pid, SIGKILL); sigkill_wait(e->tar_pid);
(void) wait_for_terminate(e->tar_pid, NULL);
}
if (e->temp_path) { if (e->temp_path) {
(void) btrfs_subvol_remove(e->temp_path, BTRFS_REMOVE_QUOTA); (void) btrfs_subvol_remove(e->temp_path, BTRFS_REMOVE_QUOTA);
@ -147,8 +145,7 @@ static int tar_export_finish(TarExport *e) {
assert(e->tar_fd >= 0); assert(e->tar_fd >= 0);
if (e->tar_pid > 0) { if (e->tar_pid > 0) {
r = wait_for_terminate_and_check("tar", e->tar_pid, WAIT_LOG); r = wait_for_terminate_and_check("tar", TAKE_PID(e->tar_pid), WAIT_LOG);
e->tar_pid = 0;
if (r < 0) if (r < 0)
return r; return r;
if (r != EXIT_SUCCESS) if (r != EXIT_SUCCESS)

View File

@ -65,7 +65,7 @@ int import_fork_tar_x(const char *path, pid_t *ret) {
pipefd[1] = safe_close(pipefd[1]); pipefd[1] = safe_close(pipefd[1]);
r = rearrange_stdio(pipefd[0], -1, STDERR_FILENO); r = rearrange_stdio(TAKE_FD(pipefd[0]), -1, STDERR_FILENO);
if (r < 0) { if (r < 0) {
log_error_errno(r, "Failed to rearrange stdin/stdout: %m"); log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);
@ -131,7 +131,7 @@ int import_fork_tar_c(const char *path, pid_t *ret) {
pipefd[0] = safe_close(pipefd[0]); pipefd[0] = safe_close(pipefd[0]);
r = rearrange_stdio(-1, pipefd[1], STDERR_FILENO); r = rearrange_stdio(-1, TAKE_FD(pipefd[1]), STDERR_FILENO);
if (r < 0) { if (r < 0) {
log_error_errno(r, "Failed to rearrange stdin/stdout: %m"); log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);

View File

@ -69,10 +69,8 @@ TarImport* tar_import_unref(TarImport *i) {
sd_event_source_unref(i->input_event_source); sd_event_source_unref(i->input_event_source);
if (i->tar_pid > 1) { if (i->tar_pid > 1)
(void) kill_and_sigcont(i->tar_pid, SIGKILL); sigkill_wait(i->tar_pid);
(void) wait_for_terminate(i->tar_pid, NULL);
}
rm_rf_subvolume_and_free(i->temp_path); rm_rf_subvolume_and_free(i->temp_path);
@ -167,8 +165,7 @@ static int tar_import_finish(TarImport *i) {
i->tar_fd = safe_close(i->tar_fd); i->tar_fd = safe_close(i->tar_fd);
if (i->tar_pid > 0) { if (i->tar_pid > 0) {
r = wait_for_terminate_and_check("tar", i->tar_pid, WAIT_LOG); r = wait_for_terminate_and_check("tar", TAKE_PID(i->tar_pid), WAIT_LOG);
i->tar_pid = 0;
if (r < 0) if (r < 0)
return r; return r;
if (r != EXIT_SUCCESS) if (r != EXIT_SUCCESS)

View File

@ -126,10 +126,8 @@ static Transfer *transfer_unref(Transfer *t) {
free(t->format); free(t->format);
free(t->object_path); free(t->object_path);
if (t->pid > 0) { if (t->pid > 1)
(void) kill_and_sigcont(t->pid, SIGKILL); sigkill_wait(t->pid);
(void) wait_for_terminate(t->pid, NULL);
}
safe_close(t->log_fd); safe_close(t->log_fd);
safe_close(t->stdin_fd); safe_close(t->stdin_fd);
@ -391,9 +389,10 @@ static int transfer_start(Transfer *t) {
pipefd[0] = safe_close(pipefd[0]); pipefd[0] = safe_close(pipefd[0]);
r = rearrange_stdio(t->stdin_fd, r = rearrange_stdio(TAKE_FD(t->stdin_fd),
t->stdout_fd < 0 ? pipefd[1] : t->stdout_fd, t->stdout_fd < 0 ? pipefd[1] : TAKE_FD(t->stdout_fd),
pipefd[1]); pipefd[1]);
TAKE_FD(pipefd[1]);
if (r < 0) { if (r < 0) {
log_error_errno(r, "Failed to set stdin/stdout/stderr: %m"); log_error_errno(r, "Failed to set stdin/stdout/stderr: %m");
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);

View File

@ -442,7 +442,7 @@ static int verify_gpg(
gpg_pipe[1] = safe_close(gpg_pipe[1]); gpg_pipe[1] = safe_close(gpg_pipe[1]);
r = rearrange_stdio(gpg_pipe[0], -1, STDERR_FILENO); r = rearrange_stdio(TAKE_FD(gpg_pipe[0]), -1, STDERR_FILENO);
if (r < 0) { if (r < 0) {
log_error_errno(r, "Failed to rearrange stdin/stdout: %m"); log_error_errno(r, "Failed to rearrange stdin/stdout: %m");
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);
@ -482,8 +482,7 @@ static int verify_gpg(
gpg_pipe[1] = safe_close(gpg_pipe[1]); gpg_pipe[1] = safe_close(gpg_pipe[1]);
r = wait_for_terminate_and_check("gpg", pid, WAIT_LOG_ABNORMAL); r = wait_for_terminate_and_check("gpg", TAKE_PID(pid), WAIT_LOG_ABNORMAL);
pid = 0;
if (r < 0) if (r < 0)
goto finish; goto finish;
if (r != EXIT_SUCCESS) if (r != EXIT_SUCCESS)

View File

@ -71,10 +71,8 @@ TarPull* tar_pull_unref(TarPull *i) {
if (!i) if (!i)
return NULL; return NULL;
if (i->tar_pid > 1) { if (i->tar_pid > 1)
(void) kill_and_sigcont(i->tar_pid, SIGKILL); sigkill_wait(i->tar_pid);
(void) wait_for_terminate(i->tar_pid, NULL);
}
pull_job_unref(i->tar_job); pull_job_unref(i->tar_job);
pull_job_unref(i->checksum_job); pull_job_unref(i->checksum_job);
@ -369,8 +367,7 @@ static void tar_pull_job_on_finished(PullJob *j) {
pull_job_close_disk_fd(i->settings_job); pull_job_close_disk_fd(i->settings_job);
if (i->tar_pid > 0) { if (i->tar_pid > 0) {
r = wait_for_terminate_and_check("tar", i->tar_pid, WAIT_LOG); r = wait_for_terminate_and_check("tar", TAKE_PID(i->tar_pid), WAIT_LOG);
i->tar_pid = 0;
if (r < 0) if (r < 0)
goto finish; goto finish;
if (r != EXIT_SUCCESS) { if (r != EXIT_SUCCESS) {

View File

@ -83,9 +83,9 @@ static int spawn_child(const char* child, char** argv) {
/* In the child */ /* In the child */
if (r == 0) { if (r == 0) {
safe_close(fd[0]); fd[0] = safe_close(fd[0]);
r = rearrange_stdio(STDIN_FILENO, fd[1], STDERR_FILENO); r = rearrange_stdio(STDIN_FILENO, TAKE_FD(fd[1]), STDERR_FILENO);
if (r < 0) { if (r < 0) {
log_error_errno(r, "Failed to dup pipe to stdout: %m"); log_error_errno(r, "Failed to dup pipe to stdout: %m");
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);

View File

@ -988,7 +988,9 @@ int bus_socket_exec(sd_bus *b) {
if (r == 0) { if (r == 0) {
/* Child */ /* Child */
if (rearrange_stdio(s[1], s[1], STDERR_FILENO) < 0) r = rearrange_stdio(s[1], s[1], STDERR_FILENO);
TAKE_FD(s[1]);
if (r < 0)
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);
(void) rlimit_nofile_safe(); (void) rlimit_nofile_safe();

View File

@ -1082,10 +1082,10 @@ static int bus_parse_next_address(sd_bus *b) {
} }
static void bus_kill_exec(sd_bus *bus) { static void bus_kill_exec(sd_bus *bus) {
if (pid_is_valid(bus->busexec_pid) > 0) { if (!pid_is_valid(bus->busexec_pid))
sigterm_wait(bus->busexec_pid); return;
bus->busexec_pid = 0;
} sigterm_wait(TAKE_PID(bus->busexec_pid));
} }
static int bus_start_address(sd_bus *b) { static int bus_start_address(sd_bus *b) {

View File

@ -38,9 +38,9 @@ static int spawn_getent(const char *database, const char *key, pid_t *rpid) {
if (r == 0) { if (r == 0) {
char *empty_env = NULL; char *empty_env = NULL;
safe_close(pipe_fds[0]); pipe_fds[0] = safe_close(pipe_fds[0]);
if (rearrange_stdio(-1, pipe_fds[1], -1) < 0) if (rearrange_stdio(-1, TAKE_FD(pipe_fds[1]), -1) < 0)
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);
(void) close_all_fds(NULL, 0); (void) close_all_fds(NULL, 0);

View File

@ -5248,8 +5248,7 @@ static int run_container(
} }
} }
r = wait_for_container(*pid, &container_status); r = wait_for_container(TAKE_PID(*pid), &container_status);
*pid = 0;
/* Tell machined that we are gone. */ /* Tell machined that we are gone. */
if (bus) if (bus)

View File

@ -50,7 +50,7 @@ static int do_spawn(const char *path, char *argv[], int stdout_fd, pid_t *pid, b
char *_argv[2]; char *_argv[2];
if (stdout_fd >= 0) { if (stdout_fd >= 0) {
r = rearrange_stdio(STDIN_FILENO, stdout_fd, STDERR_FILENO); r = rearrange_stdio(STDIN_FILENO, TAKE_FD(stdout_fd), STDERR_FILENO);
if (r < 0) if (r < 0)
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);
} }

View File

@ -286,7 +286,7 @@ void pager_close(void) {
stdout_redirected = stderr_redirected = false; stdout_redirected = stderr_redirected = false;
(void) kill(pager_pid, SIGCONT); (void) kill(pager_pid, SIGCONT);
(void) wait_for_terminate(pager_pid, NULL); (void) wait_for_terminate(TAKE_PID(pager_pid), NULL);
pager_pid = 0; pager_pid = 0;
} }

View File

@ -43,9 +43,7 @@ void ask_password_agent_close(void) {
return; return;
/* Inform agent that we are done */ /* Inform agent that we are done */
(void) kill_and_sigcont(agent_pid, SIGTERM); sigterm_wait(TAKE_PID(agent_pid));
(void) wait_for_terminate(agent_pid, NULL);
agent_pid = 0;
} }
int ask_password_agent_open_if_enabled(BusTransport transport, bool ask_password) { int ask_password_agent_open_if_enabled(BusTransport transport, bool ask_password) {

View File

@ -69,9 +69,7 @@ void polkit_agent_close(void) {
return; return;
/* Inform agent that we are done */ /* Inform agent that we are done */
(void) kill_and_sigcont(agent_pid, SIGTERM); sigterm_wait(TAKE_PID(agent_pid));
(void) wait_for_terminate(agent_pid, NULL);
agent_pid = 0;
} }
#else #else

View File

@ -579,7 +579,7 @@ static int find_libraries(const char *exec, char ***ret) {
r = safe_fork("(spawn-ldd)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid); r = safe_fork("(spawn-ldd)", FORK_RESET_SIGNALS|FORK_DEATHSIG|FORK_LOG, &pid);
assert_se(r >= 0); assert_se(r >= 0);
if (r == 0) { if (r == 0) {
if (rearrange_stdio(-1, outpipe[1], errpipe[1]) < 0) if (rearrange_stdio(-1, TAKE_FD(outpipe[1]), TAKE_FD(errpipe[1])) < 0)
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);
(void) close_all_fds(NULL, 0); (void) close_all_fds(NULL, 0);

View File

@ -1,6 +1,7 @@
/* SPDX-License-Identifier: LGPL-2.1-or-later */ /* SPDX-License-Identifier: LGPL-2.1-or-later */
#include <fcntl.h> #include <fcntl.h>
#include <sys/eventfd.h>
#include <unistd.h> #include <unistd.h>
#include "alloc-util.h" #include "alloc-util.h"
@ -486,6 +487,47 @@ static void test_fd_reopen(void) {
fd1 = -1; fd1 = -1;
} }
static void test_take_fd(void) {
_cleanup_close_ int fd1 = -1, fd2 = -1;
int array[2] = { -1, -1 }, i = 0;
log_info("/* %s */", __func__);
assert_se(fd1 == -1);
assert_se(fd2 == -1);
fd1 = eventfd(0, EFD_CLOEXEC);
assert_se(fd1 >= 0);
fd2 = TAKE_FD(fd1);
assert_se(fd1 == -1);
assert_se(fd2 >= 0);
assert_se(array[0] == -1);
assert_se(array[1] == -1);
array[0] = TAKE_FD(fd2);
assert_se(fd1 == -1);
assert_se(fd2 == -1);
assert_se(array[0] >= 0);
assert_se(array[1] == -1);
array[1] = TAKE_FD(array[i]);
assert_se(array[0] == -1);
assert_se(array[1] >= 0);
i = 1 - i;
array[0] = TAKE_FD(*(array + i));
assert_se(array[0] >= 0);
assert_se(array[1] == -1);
i = 1 - i;
fd1 = TAKE_FD(array[i]);
assert_se(fd1 >= 0);
assert_se(array[0] == -1);
assert_se(array[1] == -1);
}
int main(int argc, char *argv[]) { int main(int argc, char *argv[]) {
test_setup_logging(LOG_DEBUG); test_setup_logging(LOG_DEBUG);
@ -500,6 +542,7 @@ int main(int argc, char *argv[]) {
test_close_all_fds(); test_close_all_fds();
test_format_proc_fd_path(); test_format_proc_fd_path();
test_fd_reopen(); test_fd_reopen();
test_take_fd();
return 0; return 0;
} }

View File

@ -851,8 +851,14 @@ static void test_get_process_ppid(void) {
assert_se(get_process_ppid(1, NULL) == -EADDRNOTAVAIL); assert_se(get_process_ppid(1, NULL) == -EADDRNOTAVAIL);
/* the process with the PID above the global limit definitely doesn't exist. Verify that */ /* the process with the PID above the global limit definitely doesn't exist. Verify that */
assert_se(procfs_tasks_get_limit(&limit) >= 0); assert_se(procfs_get_pid_max(&limit) >= 0);
assert_se(limit >= INT_MAX || get_process_ppid(limit+1, NULL) == -ESRCH); log_debug("kernel.pid_max = %"PRIu64, limit);
if (limit < INT_MAX) {
r = get_process_ppid(limit + 1, NULL);
log_debug_errno(r, "get_process_limit(%"PRIu64") → %d/%m", limit + 1, r);
assert(r == -ESRCH);
}
for (pid_t pid = 0;;) { for (pid_t pid = 0;;) {
_cleanup_free_ char *c1 = NULL, *c2 = NULL; _cleanup_free_ char *c1 = NULL, *c2 = NULL;

View File

@ -6,11 +6,12 @@
#include "format-util.h" #include "format-util.h"
#include "log.h" #include "log.h"
#include "procfs-util.h" #include "procfs-util.h"
#include "process-util.h"
#include "tests.h" #include "tests.h"
int main(int argc, char *argv[]) { int main(int argc, char *argv[]) {
nsec_t nsec; nsec_t nsec;
uint64_t v; uint64_t v, w;
int r; int r;
log_parse_environment(); log_parse_environment();
@ -25,26 +26,39 @@ int main(int argc, char *argv[]) {
assert_se(procfs_tasks_get_current(&v) >= 0); assert_se(procfs_tasks_get_current(&v) >= 0);
log_info("Current number of tasks: %" PRIu64, v); log_info("Current number of tasks: %" PRIu64, v);
r = procfs_tasks_get_limit(&v); v = TASKS_MAX;
if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r)) r = procfs_get_pid_max(&v);
return log_tests_skipped("can't read /proc/sys/kernel/pid_max"); assert(r >= 0 || r == -ENOENT || ERRNO_IS_PRIVILEGE(r));
log_info("kernel.pid_max: %"PRIu64, v);
w = TASKS_MAX;
r = procfs_get_threads_max(&w);
assert(r >= 0 || r == -ENOENT || ERRNO_IS_PRIVILEGE(r));
log_info("kernel.threads-max: %"PRIu64, w);
v = MIN(v - (v > 0), w);
assert_se(r >= 0); assert_se(r >= 0);
log_info("Limit of tasks: %" PRIu64, v); log_info("Limit of tasks: %" PRIu64, v);
assert_se(v > 0); assert_se(v > 0);
assert_se(procfs_tasks_set_limit(v) >= 0); r = procfs_tasks_set_limit(v);
if (r == -ENOENT || ERRNO_IS_PRIVILEGE(r))
return log_tests_skipped("can't set task limits");
assert(r >= 0);
if (v > 100) { if (v > 100) {
uint64_t w; log_info("Reducing limit by one to %"PRIu64"", v-1);
r = procfs_tasks_set_limit(v-1);
assert_se(IN_SET(r, 0, -EPERM, -EACCES, -EROFS));
assert_se(procfs_tasks_get_limit(&w) >= 0); r = procfs_tasks_set_limit(v-1);
assert_se((r == 0 && w == v - 1) || (r < 0 && w == v)); log_info_errno(r, "procfs_tasks_set_limit: %m");
assert_se(r >= 0 || ERRNO_IS_PRIVILEGE(r) || r == -EROFS);
assert_se(procfs_get_threads_max(&w) >= 0);
assert_se(r >= 0 ? w == v - 1 : w == v);
assert_se(procfs_tasks_set_limit(v) >= 0); assert_se(procfs_tasks_set_limit(v) >= 0);
assert_se(procfs_tasks_get_limit(&w) >= 0); assert_se(procfs_get_threads_max(&w) >= 0);
assert_se(v == w); assert_se(v == w);
} }

View File

@ -783,7 +783,7 @@ int udev_event_spawn(UdevEvent *event,
return log_device_error_errno(event->dev, r, return log_device_error_errno(event->dev, r,
"Failed to fork() to execute command '%s': %m", cmd); "Failed to fork() to execute command '%s': %m", cmd);
if (r == 0) { if (r == 0) {
if (rearrange_stdio(-1, outpipe[WRITE_END], errpipe[WRITE_END]) < 0) if (rearrange_stdio(-1, TAKE_FD(outpipe[WRITE_END]), TAKE_FD(errpipe[WRITE_END])) < 0)
_exit(EXIT_FAILURE); _exit(EXIT_FAILURE);
(void) close_all_fds(NULL, 0); (void) close_all_fds(NULL, 0);