summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorLuca Boccassi <bluca@debian.org>2023-10-27 17:33:49 +0200
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>2023-10-28 16:56:25 +0200
commit7b6d3dcdd202ab8ae0478edef6eb6fa9a5437a4b (patch)
tree914972424d5cc4255023dd0a7c6247803e3a0b73
parentmkosi: Use cache and build subdirectories (diff)
downloadsystemd-7b6d3dcdd202ab8ae0478edef6eb6fa9a5437a4b.tar.xz
systemd-7b6d3dcdd202ab8ae0478edef6eb6fa9a5437a4b.zip
exec-invoke: don't double-close FDs on error
When a late error occurs in sd-executor, the cleanup-on-close of the context structs happen, but at that time all FDs might have already been closed via close_all_fds(), so a double-close happens. This can be seen when DynamicUser is enabled, with a non-existing WorkingDirectory. Invalidate the FDs in the context structs if close_all_fds succeeds.
-rw-r--r--src/core/dynamic-user.c11
-rw-r--r--src/core/dynamic-user.h1
-rw-r--r--src/core/exec-invoke.c32
-rwxr-xr-xtest/units/testsuite-07.exec-context.sh3
4 files changed, 46 insertions, 1 deletions
diff --git a/src/core/dynamic-user.c b/src/core/dynamic-user.c
index 76842e5f24..12724c682c 100644
--- a/src/core/dynamic-user.c
+++ b/src/core/dynamic-user.c
@@ -858,3 +858,14 @@ void dynamic_creds_done(DynamicCreds *creds) {
dynamic_user_free(creds->group);
creds->group = creds->user = dynamic_user_free(creds->user);
}
+
+void dynamic_creds_close(DynamicCreds *creds) {
+ if (!creds)
+ return;
+
+ if (creds->user)
+ safe_close_pair(creds->user->storage_socket);
+
+ if (creds->group && creds->group != creds->user)
+ safe_close_pair(creds->group->storage_socket);
+}
diff --git a/src/core/dynamic-user.h b/src/core/dynamic-user.h
index e86ee02796..303a7d081c 100644
--- a/src/core/dynamic-user.h
+++ b/src/core/dynamic-user.h
@@ -41,6 +41,7 @@ int dynamic_creds_realize(DynamicCreds *creds, char **suggested_paths, uid_t *ui
DynamicCreds *dynamic_creds_unref(DynamicCreds *creds);
DynamicCreds *dynamic_creds_destroy(DynamicCreds *creds);
void dynamic_creds_done(DynamicCreds *creds);
+void dynamic_creds_close(DynamicCreds *creds);
DEFINE_TRIVIAL_CLEANUP_FUNC(DynamicCreds*, dynamic_creds_unref);
DEFINE_TRIVIAL_CLEANUP_FUNC(DynamicCreds*, dynamic_creds_destroy);
diff --git a/src/core/exec-invoke.c b/src/core/exec-invoke.c
index 9be39a5610..886e087bf0 100644
--- a/src/core/exec-invoke.c
+++ b/src/core/exec-invoke.c
@@ -3862,6 +3862,33 @@ static int exec_context_named_iofds(
return targets == 0 ? 0 : -ENOENT;
}
+static void exec_shared_runtime_close(ExecSharedRuntime *shared) {
+ if (!shared)
+ return;
+
+ safe_close_pair(shared->netns_storage_socket);
+ safe_close_pair(shared->ipcns_storage_socket);
+}
+
+static void exec_runtime_close(ExecRuntime *rt) {
+ if (!rt)
+ return;
+
+ safe_close_pair(rt->ephemeral_storage_socket);
+
+ exec_shared_runtime_close(rt->shared);
+ dynamic_creds_close(rt->dynamic_creds);
+}
+
+static void exec_params_close(ExecParameters *p) {
+ if (!p)
+ return;
+
+ p->stdin_fd = safe_close(p->stdin_fd);
+ p->stdout_fd = safe_close(p->stdout_fd);
+ p->stderr_fd = safe_close(p->stderr_fd);
+}
+
int exec_invoke(
const ExecCommand *command,
const ExecContext *context,
@@ -4734,7 +4761,10 @@ int exec_invoke(
/* We repeat the fd closing here, to make sure that nothing is leaked from the PAM modules. Note that
* we are more aggressive this time, since we don't need socket_fd and the netns and ipcns fds any
* more. We do keep exec_fd however, if we have it, since we need to keep it open until the final
- * execve(). */
+ * execve(). But first, close the remaining sockets in the context objects. */
+
+ exec_runtime_close(runtime);
+ exec_params_close(params);
r = close_all_fds(keep_fds, n_keep_fds);
if (r >= 0)
diff --git a/test/units/testsuite-07.exec-context.sh b/test/units/testsuite-07.exec-context.sh
index 477b9e84da..0943300424 100755
--- a/test/units/testsuite-07.exec-context.sh
+++ b/test/units/testsuite-07.exec-context.sh
@@ -186,3 +186,6 @@ systemd-run --wait --pipe -p BindPaths="/etc /home:/mnt:norbind -/foo/bar/baz:/u
bash -xec "mountpoint /etc; test -d /etc/systemd; mountpoint /mnt; ! mountpoint /usr"
systemd-run --wait --pipe -p BindReadOnlyPaths="/etc /home:/mnt:norbind -/foo/bar/baz:/usr:rbind" \
bash -xec "test ! -w /etc; test ! -w /mnt; ! mountpoint /usr"
+
+# Ensure that clean-up codepaths work correctly if activation ultimately fails
+(! systemd-run --wait --pipe -p DynamicUser=yes -p WorkingDirectory=/nonexistent echo hello)