Merge pull request #30271 from YHNdnzj/executor-cloexec

fdset,core/executor: ocloexecification ™️
This commit is contained in:
Lennart Poettering
2023-12-06 22:26:40 +01:00
committed by GitHub
7 changed files with 45 additions and 57 deletions

View File

@@ -105,7 +105,7 @@ static int shift_fds(int fds[], size_t n_fds) {
return 0;
}
static int flags_fds(
static int flag_fds(
const int fds[],
size_t n_socket_fds,
size_t n_fds,
@@ -113,10 +113,7 @@ static int flags_fds(
int r;
if (n_fds <= 0)
return 0;
assert(fds);
assert(fds || n_fds == 0);
/* Drops/Sets O_NONBLOCK and FD_CLOEXEC from the file flags.
* O_NONBLOCK only applies to socket activation though. */
@@ -3600,32 +3597,29 @@ static int exec_context_cpu_affinity_from_numa(const ExecContext *c, CPUSet *ret
return cpu_set_add_all(ret, &s);
}
static int add_shifted_fd(int *fds, size_t fds_size, size_t *n_fds, int fd, int *ret_fd) {
static int add_shifted_fd(int *fds, size_t fds_size, size_t *n_fds, int *fd) {
int r;
assert(fds);
assert(n_fds);
assert(*n_fds < fds_size);
assert(ret_fd);
assert(fd);
if (fd < 0) {
*ret_fd = -EBADF;
return 0;
}
if (*fd < 0)
return 0;
if (fd < 3 + (int) *n_fds) {
if (*fd < 3 + (int) *n_fds) {
/* Let's move the fd up, so that it's outside of the fd range we will use to store
* the fds we pass to the process (or which are closed only during execve). */
r = fcntl(fd, F_DUPFD_CLOEXEC, 3 + (int) *n_fds);
r = fcntl(*fd, F_DUPFD_CLOEXEC, 3 + (int) *n_fds);
if (r < 0)
return -errno;
close_and_replace(fd, r);
close_and_replace(*fd, r);
}
*ret_fd = fds[*n_fds] = fd;
(*n_fds) ++;
fds[(*n_fds)++] = *fd;
return 1;
}
@@ -3925,7 +3919,7 @@ int exec_invoke(
int *exit_status) {
_cleanup_strv_free_ char **our_env = NULL, **pass_env = NULL, **joined_exec_search_path = NULL, **accum_env = NULL, **replaced_argv = NULL;
int r, ngids = 0, exec_fd;
int r, ngids = 0;
_cleanup_free_ gid_t *supplementary_gids = NULL;
const char *username = NULL, *groupname = NULL;
_cleanup_free_ char *home_buffer = NULL, *memory_pressure_path = NULL;
@@ -4063,19 +4057,17 @@ int exec_invoke(
memcpy_safe(keep_fds, fds, n_fds * sizeof(int));
n_keep_fds = n_fds;
r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, params->exec_fd, &exec_fd);
r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, &params->exec_fd);
if (r < 0) {
*exit_status = EXIT_FDS;
return log_exec_error_errno(context, params, r, "Failed to shift fd and set FD_CLOEXEC: %m");
return log_exec_error_errno(context, params, r, "Failed to collect shifted fd: %m");
}
#if HAVE_LIBBPF
if (params->bpf_outer_map_fd >= 0) {
r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, params->bpf_outer_map_fd, (int *)&params->bpf_outer_map_fd);
if (r < 0) {
*exit_status = EXIT_FDS;
return log_exec_error_errno(context, params, r, "Failed to shift fd and set FD_CLOEXEC: %m");
}
r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, &params->bpf_outer_map_fd);
if (r < 0) {
*exit_status = EXIT_FDS;
return log_exec_error_errno(context, params, r, "Failed to collect shifted fd: %m");
}
#endif
@@ -4756,10 +4748,10 @@ int exec_invoke(
"EXECUTABLE=%s", command->path);
}
r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, executable_fd, &executable_fd);
r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, &executable_fd);
if (r < 0) {
*exit_status = EXIT_FDS;
return log_exec_error_errno(context, params, r, "Failed to shift fd and set FD_CLOEXEC: %m");
return log_exec_error_errno(context, params, r, "Failed to collect shifted fd: %m");
}
#if HAVE_SELINUX
@@ -4804,7 +4796,7 @@ int exec_invoke(
if (r >= 0)
r = shift_fds(fds, n_fds);
if (r >= 0)
r = flags_fds(fds, n_socket_fds, n_fds, context->non_blocking);
r = flag_fds(fds, n_socket_fds, n_fds, context->non_blocking);
if (r < 0) {
*exit_status = EXIT_FDS;
return log_exec_error_errno(context, params, r, "Failed to adjust passed file descriptors: %m");
@@ -5209,13 +5201,13 @@ int exec_invoke(
log_command_line(context, params, "Executing", executable, final_argv);
if (exec_fd >= 0) {
if (params->exec_fd >= 0) {
uint8_t hot = 1;
/* We have finished with all our initializations. Let's now let the manager know that. From this point
* on, if the manager sees POLLHUP on the exec_fd, then execve() was successful. */
if (write(exec_fd, &hot, sizeof(hot)) < 0) {
if (write(params->exec_fd, &hot, sizeof(hot)) < 0) {
*exit_status = EXIT_EXEC;
return log_exec_error_errno(context, params, errno, "Failed to enable exec_fd: %m");
}
@@ -5223,13 +5215,13 @@ int exec_invoke(
r = fexecve_or_execve(executable_fd, executable, final_argv, accum_env);
if (exec_fd >= 0) {
if (params->exec_fd >= 0) {
uint8_t hot = 0;
/* The execve() failed. This means the exec_fd is still open. Which means we need to tell the manager
* that POLLHUP on it no longer means execve() succeeded. */
if (write(exec_fd, &hot, sizeof(hot)) < 0) {
if (write(params->exec_fd, &hot, sizeof(hot)) < 0) {
*exit_status = EXIT_EXEC;
return log_exec_error_errno(context, params, errno, "Failed to disable exec_fd: %m");
}

View File

@@ -373,8 +373,7 @@ static int exec_cgroup_context_serialize(const CGroupContext *c, FILE *f) {
if (il->limits[type] == cgroup_io_limit_defaults[type])
continue;
key = strjoin("exec-cgroup-context-io-device-limit-",
cgroup_io_limit_type_to_string(type));
key = strjoin("exec-cgroup-context-io-device-limit-", cgroup_io_limit_type_to_string(type));
if (!key)
return -ENOMEM;
@@ -1479,8 +1478,8 @@ static int exec_parameters_deserialize(ExecParameters *p, FILE *f, FDSet *fds) {
return log_oom_debug();
/* Ensure we don't leave any FD uninitialized on error, it makes the fuzzer sad */
for (size_t i = 0; i < p->n_socket_fds + p->n_storage_fds; ++i)
p->fds[i] = -EBADF;
FOREACH_ARRAY(i, p->fds, p->n_socket_fds + p->n_storage_fds)
*i = -EBADF;
r = deserialize_fd_many(fds, val, p->n_socket_fds + p->n_storage_fds, p->fds);
if (r < 0)
@@ -1611,12 +1610,6 @@ static int exec_parameters_deserialize(ExecParameters *p, FILE *f, FDSet *fds) {
if (fd < 0)
continue;
/* This is special and relies on close-on-exec semantics, make sure it's
* there */
r = fd_cloexec(fd, true);
if (r < 0)
return r;
p->exec_fd = fd;
} else if ((val = startswith(l, "exec-parameters-bpf-outer-map-fd="))) {
int fd;
@@ -1625,12 +1618,6 @@ static int exec_parameters_deserialize(ExecParameters *p, FILE *f, FDSet *fds) {
if (fd < 0)
continue;
/* This is special and relies on close-on-exec semantics, make sure it's
* there */
r = fd_cloexec(fd, true);
if (r < 0)
return r;
p->bpf_outer_map_fd = fd;
} else if ((val = startswith(l, "exec-parameters-notify-socket="))) {
r = free_and_strdup(&p->notify_socket, val);

View File

@@ -204,7 +204,9 @@ int main(int argc, char *argv[]) {
log_set_prohibit_ipc(false);
log_open();
/* The serialization fd is set to CLOEXEC in parse_argv, so it's also filtered. */
/* This call would collect all passed fds and enable CLOEXEC. We'll unset it in exec_invoke (flag_fds)
* for fds that shall be passed to the child.
* The serialization fd is set to CLOEXEC in parse_argv, so it's also filtered. */
r = fdset_new_fill(/* filter_cloexec= */ 0, &fdset);
if (r < 0)
return log_error_errno(r, "Failed to create fd set: %m");

View File

@@ -2809,8 +2809,6 @@ static int collect_fds(FDSet **ret_fds, const char **ret_error_message) {
"MESSAGE_ID=" SD_MESSAGE_CORE_FD_SET_FAILED_STR);
}
(void) fdset_cloexec(*ret_fds, true);
/* The serialization fd should have O_CLOEXEC turned on already, let's verify that we didn't pick it up here */
assert_se(!arg_serialization || !fdset_contains(*ret_fds, fileno(arg_serialization)));

View File

@@ -225,10 +225,6 @@ static int parse_argv(int argc, char *argv[]) {
r = fdset_new_fill(/* filter_cloexec= */ 0, &passed);
if (r < 0)
return log_error_errno(r, "Failed to take possession of passed file descriptors: %m");
r = fdset_cloexec(passed, true);
if (r < 0)
return log_error_errno(r, "Failed to enable O_CLOEXEC for passed file descriptors: %m");
}
if (fdnr < 3) {

View File

@@ -150,13 +150,15 @@ int fdset_remove(FDSet *s, int fd) {
int fdset_new_fill(
int filter_cloexec, /* if < 0 takes all fds, otherwise only those with O_CLOEXEC set (1) or unset (0) */
FDSet **ret) {
_cleanup_(fdset_shallow_freep) FDSet *s = NULL;
_cleanup_closedir_ DIR *d = NULL;
int r;
assert(ret);
/* Creates an fdset and fills in all currently open file descriptors. */
/* Creates an fdset and fills in all currently open file descriptors. Also set all collected fds
* to CLOEXEC. */
d = opendir("/proc/self/fd");
if (!d) {
@@ -191,6 +193,7 @@ int fdset_new_fill(
/* If user asked for that filter by O_CLOEXEC. This is useful so that fds that have
* been passed in can be collected and fds which have been created locally can be
* ignored, under the assumption that only the latter have O_CLOEXEC set. */
fl = fcntl(fd, F_GETFD);
if (fl < 0)
return -errno;
@@ -199,6 +202,13 @@ int fdset_new_fill(
continue;
}
/* We need to set CLOEXEC manually only if we're collecting non-CLOEXEC fds. */
if (filter_cloexec <= 0) {
r = fd_cloexec(fd, true);
if (r < 0)
return r;
}
r = fdset_put(s, fd);
if (r < 0)
return r;

View File

@@ -11,8 +11,8 @@
#include "tmpfile-util.h"
TEST(fdset_new_fill) {
int fd = -EBADF;
_cleanup_fdset_free_ FDSet *fdset = NULL;
int fd = -EBADF, flags;
log_close();
log_set_open_when_needed(true);
@@ -50,6 +50,9 @@ TEST(fdset_new_fill) {
assert_se(fdset_new_fill(/* filter_cloexec= */ 0, &fdset) >= 0);
assert_se(fdset_contains(fdset, fd));
flags = fcntl(fd, F_GETFD);
assert_se(flags >= 0);
assert_se(FLAGS_SET(flags, FD_CLOEXEC));
fdset = fdset_free(fdset);
assert_se(fcntl(fd, F_GETFD) < 0);
assert_se(errno == EBADF);