From: Mike Yuan <me@yhndnzj.com>
Date: Wed, 1 May 2024 13:26:33 +0000 (+0800)
Subject: core/service: imply Type=exec if credentials are used
X-Git-Tag: v257-rc1~858^2
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=refs%2Fpull%2F32612%2Fhead;p=thirdparty%2Fsystemd.git

core/service: imply Type=exec if credentials are used

When credentials are used with Type=simple + ExecStartPost=,
i.e. when multiple sd-executor instances are running in parallel
for a single service, the state of final credential dir
might be unexpected wrt path_is_mount_point() and other
steps. So, let's imply Type=exec if not explicitly specified,
and emit a warning otherwise.
---

diff --git a/man/systemd.service.xml b/man/systemd.service.xml
index 6667ac52402..d62d47cf63b 100644
--- a/man/systemd.service.xml
+++ b/man/systemd.service.xml
@@ -162,11 +162,11 @@
 
           <itemizedlist>
             <listitem><para>If set to <option>simple</option> (the default if <varname>ExecStart=</varname>
-            is specified but neither <varname>Type=</varname> nor <varname>BusName=</varname> are), the
-            service manager will consider the unit started immediately after the main service process has
-            been forked off (i.e. immediately after <function>fork()</function>, and before various process
-            attributes have been configured and in particular before the new process has called
-            <function>execve()</function> to invoke the actual service binary). Typically,
+            is specified but neither <varname>Type=</varname> nor <varname>BusName=</varname> are, and
+            credentials are not used), the service manager will consider the unit started immediately after
+            the main service process has been forked off (i.e. immediately after <function>fork()</function>,
+            and before various process attributes have been configured and in particular before the new process
+            has called <function>execve()</function> to invoke the actual service binary). Typically,
             <varname>Type=</varname><option>exec</option> is the better choice, see below.</para>
 
             <para>It is expected that the process configured with <varname>ExecStart=</varname> is the main
@@ -188,7 +188,9 @@
             Note that this means <command>systemctl start</command> command lines for <option>exec</option>
             services will report failure when the service's binary cannot be invoked successfully (for
             example because the selected <varname>User=</varname> doesn't exist, or the service binary is
-            missing).</para></listitem>
+            missing). This type is implied if credentials are used (refer to <varname>LoadCredential=</varname>
+            in <citerefentry><refentrytitle>systemd.exec</refentrytitle><manvolnum>5</manvolnum></citerefentry>
+            for details).</para></listitem>
 
             <listitem><para>If set to <option>forking</option>, the manager will consider the unit started
             immediately after the binary that forked off by the manager exits. <emphasis>The use of this type
diff --git a/src/core/service.c b/src/core/service.c
index 09bb8d61795..92137368678 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -20,6 +20,7 @@
 #include "devnum-util.h"
 #include "env-util.h"
 #include "escape.h"
+#include "exec-credential.h"
 #include "exit-status.h"
 #include "fd-util.h"
 #include "fileio.h"
@@ -697,6 +698,9 @@ static int service_verify(Service *s) {
         if (s->runtime_max_usec == USEC_INFINITY && s->runtime_rand_extra_usec != 0)
                 log_unit_warning(UNIT(s), "Service has RuntimeRandomizedExtraSec= setting, but no RuntimeMaxSec=. Ignoring.");
 
+        if (s->type == SERVICE_SIMPLE && s->exec_command[SERVICE_EXEC_START_POST] && exec_context_has_credentials(&s->exec_context))
+                log_unit_warning(UNIT(s), "Service uses a combination of Type=simple, ExecStartPost=, and credentials. This could lead to race conditions. Continuing.");
+
         if (s->exit_type == SERVICE_EXIT_CGROUP && cg_unified() < CGROUP_UNIFIED_SYSTEMD)
                 log_unit_warning(UNIT(s), "Service has ExitType=cgroup set, but we are running with legacy cgroups v1, which might not work correctly. Continuing.");
 
@@ -825,6 +829,8 @@ static int service_add_extras(Service *s) {
                 /* Figure out a type automatically */
                 if (s->bus_name)
                         s->type = SERVICE_DBUS;
+                else if (exec_context_has_credentials(&s->exec_context))
+                        s->type = SERVICE_EXEC;
                 else if (s->exec_command[SERVICE_EXEC_START])
                         s->type = SERVICE_SIMPLE;
                 else