diff options
author | Luca Boccassi <bluca@debian.org> | 2023-10-27 17:33:49 +0200 |
---|---|---|
committer | Zbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl> | 2023-10-28 16:56:25 +0200 |
commit | 7b6d3dcdd202ab8ae0478edef6eb6fa9a5437a4b (patch) | |
tree | 914972424d5cc4255023dd0a7c6247803e3a0b73 | |
parent | mkosi: Use cache and build subdirectories (diff) | |
download | systemd-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.c | 11 | ||||
-rw-r--r-- | src/core/dynamic-user.h | 1 | ||||
-rw-r--r-- | src/core/exec-invoke.c | 32 | ||||
-rwxr-xr-x | test/units/testsuite-07.exec-context.sh | 3 |
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) |