systemd: do not require absolute paths in ExecStart
authorZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Sun, 25 Mar 2018 18:50:15 +0000 (20:50 +0200)
committerZbigniew Jędrzejewski-Szmek <zbyszek@in.waw.pl>
Mon, 16 Apr 2018 14:09:46 +0000 (16:09 +0200)
Absolute paths make everything simple and quick, but sometimes this requirement
can be annoying. A good example is calling 'test', which will be located in
/usr/bin/ or /bin depending on the distro. The need the provide the full path
makes it harder a portable unit file in such cases.

This patch uses a fixed search path (DEFAULT_PATH which was already used as the
default value of $PATH), and if a non-absolute file name is found, it is
immediately resolved to a full path using this search path when the unit is
loaded. After that, everything behaves as if an absolute path was specified. In
particular, the executable must exist when the unit is loaded.

man/systemd.service.xml
src/basic/path-util.h
src/core/load-fragment.c
src/test/test-execute.c
test/meson.build
test/test-execute/exec-basic.service [new file with mode: 0644]

index b68d351..e89cfe3 100644 (file)
         <varname>ExecStop=</varname> line set. (Services lacking both <varname>ExecStart=</varname> and
         <varname>ExecStop=</varname> are not valid.)</para>
 
-        <para>For each of the specified commands, the first argument must be an absolute path to an
-        executable. Optionally, this filename may be prefixed with a number of special characters:</para>
+        <para>For each of the specified commands, the first argument must be either an absolute path to an executable
+        or a simple file name without any slashes. Optionally, this filename may be prefixed with a number of special
+        characters:</para>
 
         <table>
           <title>Special executable prefixes</title>
     <literal>&amp;</literal>, and <emphasis>other elements of shell
     syntax are not supported</emphasis>.</para>
 
-    <para>The command to execute must be an absolute path name. It may
-    contain spaces, but control characters are not allowed.</para>
+    <para>The command to execute may contain spaces, but control characters are not allowed.</para>
 
-    <para>The command line accepts <literal>%</literal> specifiers as
-    described in
+    <para>The command line accepts <literal>%</literal> specifiers as described in
     <citerefentry><refentrytitle>systemd.unit</refentrytitle><manvolnum>5</manvolnum></citerefentry>.</para>
 
     <para>Basic environment variable substitution is supported. Use
     For this type of expansion, quotes are respected when splitting
     into words, and afterwards removed.</para>
 
+    <para>If the command is not a full (absolute) path, it will be resolved to a full path using a
+    fixed search path determinted at compilation time. Searched directories include
+    <filename>/usr/local/bin/</filename>, <filename>/usr/bin/</filename>, <filename>/bin/</filename>
+    on systems using split <filename>/usr/bin/</filename> and <filename>/bin/</filename>
+    directories, and their <filename>sbin/</filename> counterparts on systems using split
+    <filename>bin/</filename> and <filename>sbin/</filename>. It is thus safe to use just the
+    executable name in case of executables located in any of the "standard" directories, and an
+    absolute path must be used in other cases. Using an absolute path is recommended to avoid
+    ambiguity.</para>
+
     <para>Example:</para>
 
     <programlisting>Environment="ONE=one" 'TWO=two two'
-ExecStart=/bin/echo $ONE $TWO ${TWO}</programlisting>
+ExecStart=echo $ONE $TWO ${TWO}</programlisting>
 
     <para>This will execute <command>/bin/echo</command> with four
     arguments: <literal>one</literal>, <literal>two</literal>,
@@ -1035,7 +1044,7 @@ ExecStart=/bin/echo $ONE $TWO ${TWO}</programlisting>
     <programlisting>Environment=ONE='one' "TWO='two two' too" THREE=
 ExecStart=/bin/echo ${ONE} ${TWO} ${THREE}
 ExecStart=/bin/echo $ONE $TWO $THREE</programlisting>
-    <para>This results in <filename>echo</filename> being
+    <para>This results in <filename>/bin/echo</filename> being
     called twice, the first time with arguments
     <literal>'one'</literal>,
     <literal>'two two' too</literal>, <literal></literal>,
@@ -1061,27 +1070,27 @@ ExecStart=/bin/echo $ONE $TWO $THREE</programlisting>
     <para>Note that shell command lines are not directly supported. If
     shell command lines are to be used, they need to be passed
     explicitly to a shell implementation of some kind. Example:</para>
-    <programlisting>ExecStart=/bin/sh -c 'dmesg | tac'</programlisting>
+    <programlisting>ExecStart=sh -c 'dmesg | tac'</programlisting>
 
     <para>Example:</para>
 
-    <programlisting>ExecStart=/bin/echo one ; /bin/echo "two two"</programlisting>
+    <programlisting>ExecStart=echo one ; echo "two two"</programlisting>
 
-    <para>This will execute <command>/bin/echo</command> two times,
+    <para>This will execute <command>echo</command> two times,
     each time with one argument: <literal>one</literal> and
     <literal>two two</literal>, respectively. Because two commands are
     specified, <varname>Type=oneshot</varname> must be used.</para>
 
     <para>Example:</para>
 
-    <programlisting>ExecStart=/bin/echo / &gt;/dev/null &amp; \; \
-/bin/ls</programlisting>
+    <programlisting>ExecStart=echo / &gt;/dev/null &amp; \; \
+ls</programlisting>
 
-    <para>This will execute <command>/bin/echo</command>
+    <para>This will execute <command>echo</command>
     with five arguments: <literal>/</literal>,
     <literal>&gt;/dev/null</literal>,
     <literal>&amp;</literal>, <literal>;</literal>, and
-    <literal>/bin/ls</literal>.</para>
+    <literal>ls</literal>.</para>
 
     <table>
       <title>C escapes supported in command lines and environment variables</title>
index 73f1769..d5b62e2 100644 (file)
 
 #if HAVE_SPLIT_BIN
 #  define PATH_SBIN_BIN(x) x "sbin:" x "bin"
+#  define PATH0_SBIN_BIN(x) x "sbin\0" x "bin"
 #else
+#  define PATH0_SBIN_BIN(x) x "bin"
 #  define PATH_SBIN_BIN(x) x "bin"
 #endif
 
 #define DEFAULT_PATH_NORMAL PATH_SBIN_BIN("/usr/local/") ":" PATH_SBIN_BIN("/usr/")
+#define DEFAULT_PATH0_NORMAL PATH0_SBIN_BIN("/usr/local/") "\0" PATH0_SBIN_BIN("/usr/")
 #define DEFAULT_PATH_SPLIT_USR DEFAULT_PATH_NORMAL ":" PATH_SBIN_BIN("/")
+#define DEFAULT_PATH0_SPLIT_USR DEFAULT_PATH0_NORMAL "\0" PATH0_SBIN_BIN("/")
 
 #if HAVE_SPLIT_USR
 #  define DEFAULT_PATH DEFAULT_PATH_SPLIT_USR
+#  define DEFAULT_PATH_NULSTR DEFAULT_PATH0_SPLIT_USR
 #else
 #  define DEFAULT_PATH DEFAULT_PATH_NORMAL
+#  define DEFAULT_PATH_NULSTR DEFAULT_PATH0_NORMAL
 #endif
 
 bool is_path(const char *p) _pure_;
index 887eb1c..f9d340d 100644 (file)
@@ -652,23 +652,51 @@ int config_parse_exec(
                 }
                 if (!string_is_safe(path)) {
                         log_syntax(unit, LOG_ERR, filename, line, 0,
-                                   "Executable path contains special characters%s: %s",
-                                   ignore ? ", ignoring" : "", rvalue);
-                        return ignore ? 0 : -ENOEXEC;
-                }
-                if (!path_is_absolute(path)) {
-                        log_syntax(unit, LOG_ERR, filename, line, 0,
-                                   "Executable path is not absolute%s: %s",
-                                   ignore ? ", ignoring" : "", rvalue);
+                                   "Executable name contains special characters%s: %s",
+                                   ignore ? ", ignoring" : "", path);
                         return ignore ? 0 : -ENOEXEC;
                 }
                 if (endswith(path, "/")) {
                         log_syntax(unit, LOG_ERR, filename, line, 0,
                                    "Executable path specifies a directory%s: %s",
-                                   ignore ? ", ignoring" : "", rvalue);
+                                   ignore ? ", ignoring" : "", path);
                         return ignore ? 0 : -ENOEXEC;
                 }
 
+                if (!path_is_absolute(path)) {
+                        const char *prefix;
+                        bool found = false;
+
+                        if (!filename_is_valid(path)) {
+                                log_syntax(unit, LOG_ERR, filename, line, 0,
+                                           "Neither a valid executable name nor an absolute path%s: %s",
+                                           ignore ? ", ignoring" : "", path);
+                                return ignore ? 0 : -ENOEXEC;
+                        }
+
+                        /* Resolve a single-component name to a full path */
+                        NULSTR_FOREACH(prefix, DEFAULT_PATH_NULSTR) {
+                                _cleanup_free_ char *fullpath = NULL;
+
+                                fullpath = strjoin(prefix, "/", path);
+                                if (!fullpath)
+                                        return log_oom();
+
+                                if (access(fullpath, F_OK) >= 0) {
+                                        free_and_replace(path, fullpath);
+                                        found = true;
+                                        break;
+                                }
+                        }
+
+                        if (!found) {
+                                log_syntax(unit, LOG_ERR, filename, line, 0,
+                                           "Executable \"%s\" not found in path \"%s\"%s",
+                                           path, DEFAULT_PATH, ignore ? ", ignoring" : "");
+                                return ignore ? 0 : -ENOEXEC;
+                        }
+                }
+
                 if (!separate_argv0) {
                         char *w = NULL;
 
index 81f9b7c..4c15f1a 100644 (file)
@@ -549,6 +549,10 @@ static void test_exec_capabilityboundingset(Manager *m) {
         test(m, "exec-capabilityboundingset-invert.service", 0, CLD_EXITED);
 }
 
+static void test_exec_basic(Manager *m) {
+        test(m, "exec-basic.service", 0, CLD_EXITED);
+}
+
 static void test_exec_ambientcapabilities(Manager *m) {
         int r;
 
@@ -648,6 +652,7 @@ static int run_tests(UnitFileScope scope, const test_function_t *tests) {
 int main(int argc, char *argv[]) {
         _cleanup_(rm_rf_physical_and_freep) char *runtime_dir = NULL;
         static const test_function_t user_tests[] = {
+                test_exec_basic,
                 test_exec_ambientcapabilities,
                 test_exec_bindpaths,
                 test_exec_capabilityboundingset,
index 809bd44..fd4ac11 100644 (file)
@@ -45,6 +45,7 @@ test_data_files = '''
         sockets.target
         son.service
         sysinit.target
+        test-execute/exec-basic.service
         test-execute/exec-ambientcapabilities-merge-nfsnobody.service
         test-execute/exec-ambientcapabilities-merge-nobody.service
         test-execute/exec-ambientcapabilities-merge.service
diff --git a/test/test-execute/exec-basic.service b/test/test-execute/exec-basic.service
new file mode 100644 (file)
index 0000000..456f069
--- /dev/null
@@ -0,0 +1,13 @@
+[Unit]
+Description=Test for basic execution
+
+[Service]
+ExecStart=touch /tmp/a ; /bin/touch /tmp/b ; touch /tmp/c
+ExecStart=test -f /tmp/a
+ExecStart=!test -f /tmp/b
+ExecStart=!!test -f /tmp/c
+ExecStart=+test -f /tmp/c
+ExecStartPost=rm /tmp/a /tmp/b /tmp/c
+
+PrivateTmp=true
+Type=oneshot