mirror of
https://github.com/systemd/systemd.git
synced 2025-01-10 05:18:17 +03:00
executor: don't duplicate FD array to avoid double closing
Just use ExecParam directly, as these are all internal to sd-exec now anyway. Avoids double close when execution fails after FDs are set up for inheritance and were already re-arranged. Fixes https://github.com/systemd/systemd/issues/30412
This commit is contained in:
parent
a9235a9488
commit
1eeaa93de3
@ -1807,7 +1807,6 @@ static int build_environment(
|
||||
const ExecParameters *p,
|
||||
const CGroupContext *cgroup_context,
|
||||
size_t n_fds,
|
||||
char **fdnames,
|
||||
const char *home,
|
||||
const char *username,
|
||||
const char *shell,
|
||||
@ -1841,7 +1840,7 @@ static int build_environment(
|
||||
return -ENOMEM;
|
||||
our_env[n_env++] = x;
|
||||
|
||||
joined = strv_join(fdnames, ":");
|
||||
joined = strv_join(p->fd_names, ":");
|
||||
if (!joined)
|
||||
return -ENOMEM;
|
||||
|
||||
@ -3706,18 +3705,11 @@ static int get_open_file_fd(const ExecContext *c, const ExecParameters *p, const
|
||||
return TAKE_FD(fd);
|
||||
}
|
||||
|
||||
static int collect_open_file_fds(
|
||||
const ExecContext *c,
|
||||
const ExecParameters *p,
|
||||
int **fds,
|
||||
char ***fdnames,
|
||||
size_t *n_fds) {
|
||||
static int collect_open_file_fds(const ExecContext *c, ExecParameters *p, size_t *n_fds) {
|
||||
int r;
|
||||
|
||||
assert(c);
|
||||
assert(p);
|
||||
assert(fds);
|
||||
assert(fdnames);
|
||||
assert(n_fds);
|
||||
|
||||
LIST_FOREACH(open_files, of, p->open_files) {
|
||||
@ -3733,14 +3725,14 @@ static int collect_open_file_fds(
|
||||
return fd;
|
||||
}
|
||||
|
||||
if (!GREEDY_REALLOC(*fds, *n_fds + 1))
|
||||
if (!GREEDY_REALLOC(p->fds, *n_fds + 1))
|
||||
return -ENOMEM;
|
||||
|
||||
r = strv_extend(fdnames, of->fdname);
|
||||
r = strv_extend(&p->fd_names, of->fdname);
|
||||
if (r < 0)
|
||||
return r;
|
||||
|
||||
(*fds)[*n_fds] = TAKE_FD(fd);
|
||||
p->fds[*n_fds] = TAKE_FD(fd);
|
||||
|
||||
(*n_fds)++;
|
||||
}
|
||||
@ -3947,11 +3939,9 @@ int exec_invoke(
|
||||
int secure_bits;
|
||||
_cleanup_free_ gid_t *gids_after_pam = NULL;
|
||||
int ngids_after_pam = 0;
|
||||
_cleanup_free_ int *fds = NULL;
|
||||
_cleanup_strv_free_ char **fdnames = NULL;
|
||||
|
||||
int socket_fd = -EBADF, named_iofds[3] = EBADF_TRIPLET, *params_fds = NULL;
|
||||
size_t n_storage_fds = 0, n_socket_fds = 0;
|
||||
int socket_fd = -EBADF, named_iofds[3] = EBADF_TRIPLET;
|
||||
size_t n_storage_fds, n_socket_fds;
|
||||
|
||||
assert(command);
|
||||
assert(context);
|
||||
@ -3984,8 +3974,8 @@ int exec_invoke(
|
||||
return log_exec_error_errno(context, params, SYNTHETIC_ERRNO(EINVAL), "Got no socket.");
|
||||
|
||||
socket_fd = params->fds[0];
|
||||
n_storage_fds = n_socket_fds = 0;
|
||||
} else {
|
||||
params_fds = params->fds;
|
||||
n_socket_fds = params->n_socket_fds;
|
||||
n_storage_fds = params->n_storage_fds;
|
||||
}
|
||||
@ -4027,26 +4017,14 @@ int exec_invoke(
|
||||
/* In case anything used libc syslog(), close this here, too */
|
||||
closelog();
|
||||
|
||||
fds = newdup(int, params_fds, n_fds);
|
||||
if (!fds) {
|
||||
*exit_status = EXIT_MEMORY;
|
||||
return log_oom();
|
||||
}
|
||||
|
||||
fdnames = strv_copy((char**) params->fd_names);
|
||||
if (!fdnames) {
|
||||
*exit_status = EXIT_MEMORY;
|
||||
return log_oom();
|
||||
}
|
||||
|
||||
r = collect_open_file_fds(context, params, &fds, &fdnames, &n_fds);
|
||||
r = collect_open_file_fds(context, params, &n_fds);
|
||||
if (r < 0) {
|
||||
*exit_status = EXIT_FDS;
|
||||
return log_exec_error_errno(context, params, r, "Failed to get OpenFile= file descriptors: %m");
|
||||
}
|
||||
|
||||
int keep_fds[n_fds + 3];
|
||||
memcpy_safe(keep_fds, fds, n_fds * sizeof(int));
|
||||
memcpy_safe(keep_fds, params->fds, n_fds * sizeof(int));
|
||||
n_keep_fds = n_fds;
|
||||
|
||||
r = add_shifted_fd(keep_fds, ELEMENTSOF(keep_fds), &n_keep_fds, ¶ms->exec_fd);
|
||||
@ -4444,7 +4422,6 @@ int exec_invoke(
|
||||
params,
|
||||
cgroup_context,
|
||||
n_fds,
|
||||
fdnames,
|
||||
home,
|
||||
username,
|
||||
shell,
|
||||
@ -4554,7 +4531,7 @@ int exec_invoke(
|
||||
* wins here. (See above.) */
|
||||
|
||||
/* All fds passed in the fds array will be closed in the pam child process. */
|
||||
r = setup_pam(context->pam_name, username, uid, gid, context->tty_path, &accum_env, fds, n_fds);
|
||||
r = setup_pam(context->pam_name, username, uid, gid, context->tty_path, &accum_env, params->fds, n_fds);
|
||||
if (r < 0) {
|
||||
*exit_status = EXIT_PAM;
|
||||
return log_exec_error_errno(context, params, r, "Failed to set up PAM session: %m");
|
||||
@ -4786,9 +4763,9 @@ int exec_invoke(
|
||||
|
||||
r = close_all_fds(keep_fds, n_keep_fds);
|
||||
if (r >= 0)
|
||||
r = shift_fds(fds, n_fds);
|
||||
r = shift_fds(params->fds, n_fds);
|
||||
if (r >= 0)
|
||||
r = flag_fds(fds, n_socket_fds, n_fds, context->non_blocking);
|
||||
r = flag_fds(params->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");
|
||||
|
@ -36,7 +36,7 @@ EOF
|
||||
"${SYSTEMCTL:?}" enable --root="$workspace" issue2730.mount
|
||||
ln -svrf "$workspace/etc/systemd/system/issue2730.mount" "$workspace/etc/systemd/system/issue2730-alias.mount"
|
||||
|
||||
image_install logger
|
||||
image_install logger socat
|
||||
}
|
||||
|
||||
do_test "$@"
|
||||
|
31
test/units/testsuite-07.issue-30412.sh
Executable file
31
test/units/testsuite-07.issue-30412.sh
Executable file
@ -0,0 +1,31 @@
|
||||
#!/usr/bin/env bash
|
||||
# SPDX-License-Identifier: LGPL-2.1-or-later
|
||||
set -eux
|
||||
set -o pipefail
|
||||
|
||||
# Check that socket FDs are not double closed on error: https://github.com/systemd/systemd/issues/30412
|
||||
|
||||
mkdir -p /run/systemd/system
|
||||
|
||||
rm -f /tmp/badbin
|
||||
touch /tmp/badbin
|
||||
chmod 744 /tmp/badbin
|
||||
|
||||
cat >/run/systemd/system/badbin_assert.service <<EOF
|
||||
[Service]
|
||||
ExecStart=/tmp/badbin
|
||||
Restart=never
|
||||
EOF
|
||||
|
||||
cat >/run/systemd/system/badbin_assert.socket <<EOF
|
||||
[Socket]
|
||||
ListenStream=@badbin_assert.socket
|
||||
EOF
|
||||
|
||||
systemctl daemon-reload
|
||||
systemctl start badbin_assert.socket
|
||||
|
||||
socat - ABSTRACT-CONNECT:badbin_assert.socket
|
||||
|
||||
timeout 10 sh -c 'while systemctl is-active badbin_assert.service; do sleep .5; done'
|
||||
[[ "$(systemctl show -P ExecMainStatus badbin_assert.service)" == 203 ]]
|
Loading…
Reference in New Issue
Block a user