From c0af656c52fa19cf9a46bb5bfe503ea0da99c979 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 12 Apr 2018 16:16:19 +0200 Subject: [PATCH 1/8] test-execute/exec-specifier.service: fix quoting The lines would cause the whole service to fail to be loaded. --- test/test-execute/exec-specifier.service | 4 ++-- test/test-execute/exec-specifier@.service | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/test/test-execute/exec-specifier.service b/test/test-execute/exec-specifier.service index 37852390ac..70227eac68 100644 --- a/test/test-execute/exec-specifier.service +++ b/test/test-execute/exec-specifier.service @@ -16,8 +16,8 @@ ExecStart=/usr/bin/test %C = /var/cache ExecStart=/usr/bin/test %L = /var/log ExecStart=/bin/sh -c 'test %u = $$(id -un 0)' ExecStart=/usr/bin/test %U = 0 -ExecStart=/bin/sh -c 'test %h = $$(getent passwd 0 | cut -d: -f 6) -ExecStart=/bin/sh -c 'test %s = $$(getent passwd 0 | cut -d: -f 7) +ExecStart=/bin/sh -c 'test %h = $$(getent passwd 0 | cut -d: -f 6)' +ExecStart=/bin/sh -c 'test %s = $$(getent passwd 0 | cut -d: -f 7)' ExecStart=/bin/sh -c 'test %m = $$(cat /etc/machine-id)' ExecStart=/bin/sh -c 'test %b = $$(cat /proc/sys/kernel/random/boot_id | sed -e 's/-//g')' ExecStart=/bin/sh -c 'test %H = $$(hostname)' diff --git a/test/test-execute/exec-specifier@.service b/test/test-execute/exec-specifier@.service index 0015dffca6..a90af5aee4 100644 --- a/test/test-execute/exec-specifier@.service +++ b/test/test-execute/exec-specifier@.service @@ -16,8 +16,8 @@ ExecStart=/usr/bin/test %C = /var/cache ExecStart=/usr/bin/test %L = /var/log ExecStart=/bin/sh -c 'test %u = $$(id -un 0)' ExecStart=/usr/bin/test %U = 0 -ExecStart=/bin/sh -c 'test %h = $$(getent passwd 0 | cut -d: -f 6) -ExecStart=/bin/sh -c 'test %s = $$(getent passwd 0 | cut -d: -f 7) +ExecStart=/bin/sh -c 'test %h = $$(getent passwd 0 | cut -d: -f 6)' +ExecStart=/bin/sh -c 'test %s = $$(getent passwd 0 | cut -d: -f 7)' ExecStart=/bin/sh -c 'test %m = $$(cat /etc/machine-id)' ExecStart=/bin/sh -c 'test %b = $$(cat /proc/sys/kernel/random/boot_id | sed -e 's/-//g')' ExecStart=/bin/sh -c 'test %H = $$(hostname)' From 4109ede7788c90e961b08c0da7d4da2d402931d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 12 Apr 2018 15:13:14 +0200 Subject: [PATCH 2/8] core/manager: split out function to verify that unit is loaded and not masked No functional change. --- src/core/main.c | 27 +++++++-------------------- src/core/manager.c | 29 +++++++++++++++++++++++++++++ src/core/manager.h | 1 + 3 files changed, 37 insertions(+), 20 deletions(-) diff --git a/src/core/main.c b/src/core/main.c index 9e95b6f110..4d113bd1d1 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -1906,28 +1906,15 @@ static int do_queue_default_job( log_debug("Activating default unit: %s", arg_default_unit); - r = manager_load_unit(m, arg_default_unit, NULL, &error, &target); - if (r < 0) - log_error("Failed to load default target: %s", bus_error_message(&error, r)); - else if (IN_SET(target->load_state, UNIT_ERROR, UNIT_NOT_FOUND)) - log_error_errno(target->load_error, "Failed to load default target: %m"); - else if (target->load_state == UNIT_MASKED) - log_error("Default target masked."); + r = manager_load_startable_unit_or_warn(m, arg_default_unit, NULL, &target); + if (r < 0) { + log_info("Falling back to rescue target: " SPECIAL_RESCUE_TARGET); - if (!target || target->load_state != UNIT_LOADED) { - log_info("Trying to load rescue target..."); - - r = manager_load_unit(m, SPECIAL_RESCUE_TARGET, NULL, &error, &target); + r = manager_load_startable_unit_or_warn(m, SPECIAL_RESCUE_TARGET, NULL, &target); if (r < 0) { - *ret_error_message = "Failed to load rescue target"; - return log_emergency_errno(r, "Failed to load rescue target: %s", bus_error_message(&error, r)); - } else if (IN_SET(target->load_state, UNIT_ERROR, UNIT_NOT_FOUND)) { - *ret_error_message = "Failed to load rescue target"; - return log_emergency_errno(target->load_error, "Failed to load rescue target: %m"); - } else if (target->load_state == UNIT_MASKED) { - *ret_error_message = "Rescue target masked"; - log_emergency("Rescue target masked."); - return -ERFKILL; + *ret_error_message = r == -ERFKILL ? "Rescue target masked" + : "Failed to load rescue target"; + return r; } } diff --git a/src/core/manager.c b/src/core/manager.c index e67f7446c7..8fccd3b4e9 100644 --- a/src/core/manager.c +++ b/src/core/manager.c @@ -1823,7 +1823,36 @@ int manager_load_unit( manager_dispatch_load_queue(m); *_ret = unit_follow_merge(*_ret); + return 0; +} +int manager_load_startable_unit_or_warn( + Manager *m, + const char *name, + const char *path, + Unit **ret) { + + /* Load a unit, make sure it loaded fully and is not masked. */ + + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + Unit *unit; + int r; + + r = manager_load_unit(m, name, path, &error, &unit); + if (r < 0) + return log_error_errno(r, "Failed to load %s %s: %s", + name ? "unit" : "file", name ?: path, + bus_error_message(&error, r)); + else if (IN_SET(unit->load_state, UNIT_ERROR, UNIT_NOT_FOUND)) + return log_error_errno(unit->load_error, "Failed to load %s %s: %m", + name ? "unit" : "file", name ?: path); + else if (unit->load_state == UNIT_MASKED) { + log_error("%s %s is masked.", + name ? "Unit" : "File", name ?: path); + return -ERFKILL; + } + + *ret = unit; return 0; } diff --git a/src/core/manager.h b/src/core/manager.h index e09e0cdf5e..1a09721f92 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -396,6 +396,7 @@ int manager_get_job_from_dbus_path(Manager *m, const char *s, Job **_j); int manager_load_unit_prepare(Manager *m, const char *name, const char *path, sd_bus_error *e, Unit **_ret); int manager_load_unit(Manager *m, const char *name, const char *path, sd_bus_error *e, Unit **_ret); +int manager_load_startable_unit_or_warn(Manager *m, const char *name, const char *path, Unit **ret); int manager_load_unit_from_dbus_path(Manager *m, const char *s, sd_bus_error *e, Unit **_u); int manager_add_job(Manager *m, JobType type, Unit *unit, JobMode mode, sd_bus_error *e, Job **_ret); From ba412430a97ffbc02b3911c1b34db63e1524f7f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 12 Apr 2018 15:51:39 +0200 Subject: [PATCH 3/8] tests: use manager_load_startable_unit_or_warn() to load units Doing manager_load_unit() followed by UNIT_VTABLE(unit)->start(unit) would result in an assertion failure in ->start() if the unit failed to load properly. Something like this is okey-ish is tests, since the test units are not expected to fail to load, but the reason for failure is clearer if we fail immediately. --- src/test/test-cgroup-mask.c | 15 +++++---------- src/test/test-engine.c | 14 +++++++------- src/test/test-execute.c | 2 +- src/test/test-path.c | 14 +++++++------- src/test/test-sched-prio.c | 15 +++++---------- 5 files changed, 25 insertions(+), 35 deletions(-) diff --git a/src/test/test-cgroup-mask.c b/src/test/test-cgroup-mask.c index 907531b045..c63c4d5a1a 100644 --- a/src/test/test-cgroup-mask.c +++ b/src/test/test-cgroup-mask.c @@ -66,16 +66,11 @@ static int test_cgroup_mask(void) { assert_se(manager_startup(m, serial, fdset) >= 0); /* Load units and verify hierarchy. */ - assert_se(manager_load_unit(m, "parent.slice", NULL, NULL, &parent) >= 0); - assert_se(manager_load_unit(m, "son.service", NULL, NULL, &son) >= 0); - assert_se(manager_load_unit(m, "daughter.service", NULL, NULL, &daughter) >= 0); - assert_se(manager_load_unit(m, "grandchild.service", NULL, NULL, &grandchild) >= 0); - assert_se(manager_load_unit(m, "parent-deep.slice", NULL, NULL, &parent_deep) >= 0); - assert_se(parent->load_state == UNIT_LOADED); - assert_se(son->load_state == UNIT_LOADED); - assert_se(daughter->load_state == UNIT_LOADED); - assert_se(grandchild->load_state == UNIT_LOADED); - assert_se(parent_deep->load_state == UNIT_LOADED); + assert_se(manager_load_startable_unit_or_warn(m, "parent.slice", NULL, &parent) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "son.service", NULL, &son) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "daughter.service", NULL, &daughter) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "grandchild.service", NULL, &grandchild) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "parent-deep.slice", NULL, &parent_deep) >= 0); assert_se(UNIT_DEREF(son->slice) == parent); assert_se(UNIT_DEREF(daughter->slice) == parent); assert_se(UNIT_DEREF(parent_deep->slice) == parent); diff --git a/src/test/test-engine.c b/src/test/test-engine.c index 5d7cd8cfd5..e5ec3972b4 100644 --- a/src/test/test-engine.c +++ b/src/test/test-engine.c @@ -56,9 +56,9 @@ int main(int argc, char *argv[]) { assert_se(manager_startup(m, serial, fdset) >= 0); printf("Load1:\n"); - assert_se(manager_load_unit(m, "a.service", NULL, NULL, &a) >= 0); - assert_se(manager_load_unit(m, "b.service", NULL, NULL, &b) >= 0); - assert_se(manager_load_unit(m, "c.service", NULL, NULL, &c) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "a.service", NULL, &a) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "b.service", NULL, &b) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "c.service", NULL, &c) >= 0); manager_dump_units(m, stdout, "\t"); printf("Test1: (Trivial)\n"); @@ -70,8 +70,8 @@ int main(int argc, char *argv[]) { printf("Load2:\n"); manager_clear_jobs(m); - assert_se(manager_load_unit(m, "d.service", NULL, NULL, &d) >= 0); - assert_se(manager_load_unit(m, "e.service", NULL, NULL, &e) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "d.service", NULL, &d) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "e.service", NULL, &e) >= 0); manager_dump_units(m, stdout, "\t"); printf("Test2: (Cyclic Order, Unfixable)\n"); @@ -87,7 +87,7 @@ int main(int argc, char *argv[]) { manager_dump_jobs(m, stdout, "\t"); printf("Load3:\n"); - assert_se(manager_load_unit(m, "g.service", NULL, NULL, &g) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "g.service", NULL, &g) >= 0); manager_dump_units(m, stdout, "\t"); printf("Test5: (Colliding transaction, fail)\n"); @@ -109,7 +109,7 @@ int main(int argc, char *argv[]) { manager_dump_jobs(m, stdout, "\t"); printf("Load4:\n"); - assert_se(manager_load_unit(m, "h.service", NULL, NULL, &h) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "h.service", NULL, &h) >= 0); manager_dump_units(m, stdout, "\t"); printf("Test10: (Unmergeable job type of auxiliary job, fail)\n"); diff --git a/src/test/test-execute.c b/src/test/test-execute.c index db8a9c75a9..81f9b7c344 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -145,7 +145,7 @@ static void test(Manager *m, const char *unit_name, int status_expected, int cod assert_se(unit_name); - assert_se(manager_load_unit(m, unit_name, NULL, NULL, &unit) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, unit_name, NULL, &unit) >= 0); assert_se(UNIT_VTABLE(unit)->start(unit) >= 0); check(m, unit, status_expected, code_expected); } diff --git a/src/test/test-path.c b/src/test/test-path.c index ec9d2626e2..9eaa61f950 100644 --- a/src/test/test-path.c +++ b/src/test/test-path.c @@ -134,7 +134,7 @@ static void test_path_exists(Manager *m) { assert_se(m); - assert_se(manager_load_unit(m, "path-exists.path", NULL, NULL, &unit) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "path-exists.path", NULL, &unit) >= 0); assert_se(UNIT_VTABLE(unit)->start(unit) >= 0); assert_se(touch(test_path) >= 0); @@ -147,7 +147,7 @@ static void test_path_existsglob(Manager *m) { Unit *unit = NULL; assert_se(m); - assert_se(manager_load_unit(m, "path-existsglob.path", NULL, NULL, &unit) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "path-existsglob.path", NULL, &unit) >= 0); assert_se(UNIT_VTABLE(unit)->start(unit) >= 0); assert_se(touch(test_path) >= 0); @@ -164,7 +164,7 @@ static void test_path_changed(Manager *m) { assert_se(touch(test_path) >= 0); - assert_se(manager_load_unit(m, "path-changed.path", NULL, NULL, &unit) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "path-changed.path", NULL, &unit) >= 0); assert_se(UNIT_VTABLE(unit)->start(unit) >= 0); f = fopen(test_path, "w"); @@ -183,7 +183,7 @@ static void test_path_modified(Manager *m) { assert_se(touch(test_path) >= 0); - assert_se(manager_load_unit(m, "path-modified.path", NULL, NULL, &unit) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "path-modified.path", NULL, &unit) >= 0); assert_se(UNIT_VTABLE(unit)->start(unit) >= 0); f = fopen(test_path, "w"); @@ -199,7 +199,7 @@ static void test_path_unit(Manager *m) { assert_se(m); - assert_se(manager_load_unit(m, "path-unit.path", NULL, NULL, &unit) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "path-unit.path", NULL, &unit) >= 0); assert_se(UNIT_VTABLE(unit)->start(unit) >= 0); assert_se(touch(test_path) >= 0); @@ -215,7 +215,7 @@ static void test_path_directorynotempty(Manager *m) { assert_se(access(test_path, F_OK) < 0); - assert_se(manager_load_unit(m, "path-directorynotempty.path", NULL, NULL, &unit) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "path-directorynotempty.path", NULL, &unit) >= 0); assert_se(UNIT_VTABLE(unit)->start(unit) >= 0); /* MakeDirectory default to no */ @@ -236,7 +236,7 @@ static void test_path_makedirectory_directorymode(Manager *m) { assert_se(access(test_path, F_OK) < 0); - assert_se(manager_load_unit(m, "path-makedirectory.path", NULL, NULL, &unit) >= 0); + assert_se(manager_load_startable_unit_or_warn(m, "path-makedirectory.path", NULL, &unit) >= 0); assert_se(UNIT_VTABLE(unit)->start(unit) >= 0); /* Check if the directory has been created */ diff --git a/src/test/test-sched-prio.c b/src/test/test-sched-prio.c index abcda4dab5..6272505ce7 100644 --- a/src/test/test-sched-prio.c +++ b/src/test/test-sched-prio.c @@ -53,8 +53,7 @@ int main(int argc, char *argv[]) { assert_se(manager_startup(m, serial, fdset) >= 0); /* load idle ok */ - assert_se(manager_load_unit(m, "sched_idle_ok.service", NULL, NULL, &idle_ok) >= 0); - assert_se(idle_ok->load_state == UNIT_LOADED); + assert_se(manager_load_startable_unit_or_warn(m, "sched_idle_ok.service", NULL, &idle_ok) >= 0); ser = SERVICE(idle_ok); assert_se(ser->exec_context.cpu_sched_policy == SCHED_OTHER); assert_se(ser->exec_context.cpu_sched_priority == 0); @@ -62,8 +61,7 @@ int main(int argc, char *argv[]) { /* * load idle bad. This should print a warning but we have no way to look at it. */ - assert_se(manager_load_unit(m, "sched_idle_bad.service", NULL, NULL, &idle_bad) >= 0); - assert_se(idle_bad->load_state == UNIT_LOADED); + assert_se(manager_load_startable_unit_or_warn(m, "sched_idle_bad.service", NULL, &idle_bad) >= 0); ser = SERVICE(idle_ok); assert_se(ser->exec_context.cpu_sched_policy == SCHED_OTHER); assert_se(ser->exec_context.cpu_sched_priority == 0); @@ -72,8 +70,7 @@ int main(int argc, char *argv[]) { * load rr ok. * Test that the default priority is moving from 0 to 1. */ - assert_se(manager_load_unit(m, "sched_rr_ok.service", NULL, NULL, &rr_ok) >= 0); - assert_se(rr_ok->load_state == UNIT_LOADED); + assert_se(manager_load_startable_unit_or_warn(m, "sched_rr_ok.service", NULL, &rr_ok) >= 0); ser = SERVICE(rr_ok); assert_se(ser->exec_context.cpu_sched_policy == SCHED_RR); assert_se(ser->exec_context.cpu_sched_priority == 1); @@ -82,8 +79,7 @@ int main(int argc, char *argv[]) { * load rr bad. * Test that the value of 0 and 100 is ignored. */ - assert_se(manager_load_unit(m, "sched_rr_bad.service", NULL, NULL, &rr_bad) >= 0); - assert_se(rr_bad->load_state == UNIT_LOADED); + assert_se(manager_load_startable_unit_or_warn(m, "sched_rr_bad.service", NULL, &rr_bad) >= 0); ser = SERVICE(rr_bad); assert_se(ser->exec_context.cpu_sched_policy == SCHED_RR); assert_se(ser->exec_context.cpu_sched_priority == 1); @@ -92,8 +88,7 @@ int main(int argc, char *argv[]) { * load rr change. * Test that anything between 1 and 99 can be set. */ - assert_se(manager_load_unit(m, "sched_rr_change.service", NULL, NULL, &rr_sched) >= 0); - assert_se(rr_sched->load_state == UNIT_LOADED); + assert_se(manager_load_startable_unit_or_warn(m, "sched_rr_change.service", NULL, &rr_sched) >= 0); ser = SERVICE(rr_sched); assert_se(ser->exec_context.cpu_sched_policy == SCHED_RR); assert_se(ser->exec_context.cpu_sched_priority == 99); From f79cd1a9b2111a228bbb5b6de6fb836ad515c5c8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Thu, 12 Apr 2018 15:58:45 +0200 Subject: [PATCH 4/8] verify: use manager_load_startable_unit_or_warn() to load units for verification This doesn't change the outcome: (before) /home/zbyszek/src/systemd/test/test-execute/exec-basic.service:6: Executable path specifies a directory: /usr/bin/test/ exec-basic.service: Failed to create exec-basic.service/start: Unit exec-basic.service is not loaded properly: Exec format error. (after) /home/zbyszek/src/systemd/test/test-execute/exec-basic.service:6: Executable path specifies a directory: /usr/bin/test/ Failed to load file /home/zbyszek/src/systemd/test/test-execute/exec-basic.service: Exec format error (before) masked.service: Failed to create masked.service/start: Unit masked.service is masked. (after) File /home/zbyszek/src/systemd/test/test-execute/masked.service is masked. but the failure is immediate and the error messages are more direct. --- src/analyze/analyze-verify.c | 11 ++++------- 1 file changed, 4 insertions(+), 7 deletions(-) diff --git a/src/analyze/analyze-verify.c b/src/analyze/analyze-verify.c index 4cdf632552..35e2b426dd 100644 --- a/src/analyze/analyze-verify.c +++ b/src/analyze/analyze-verify.c @@ -244,7 +244,6 @@ static int verify_unit(Unit *u, bool check_man) { } int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run_generators) { - _cleanup_(sd_bus_error_free) sd_bus_error err = SD_BUS_ERROR_NULL; _cleanup_free_ char *var = NULL; Manager *m = NULL; FILE *serial = NULL; @@ -297,12 +296,10 @@ int verify_units(char **filenames, UnitFileScope scope, bool check_man, bool run continue; } - k = manager_load_unit(m, NULL, prepared, &err, &units[count]); - if (k < 0) { - log_error_errno(k, "Failed to load %s: %m", *filename); - if (r == 0) - r = k; - } else + k = manager_load_startable_unit_or_warn(m, NULL, prepared, &units[count]); + if (k < 0 && r == 0) + r = k; + else count++; } From 5008da1ec1cf2cf8c15b702c4052e3a49583095d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 25 Mar 2018 20:50:15 +0200 Subject: [PATCH 5/8] systemd: do not require absolute paths in ExecStart 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 | 39 ++++++++++++++--------- src/basic/path-util.h | 6 ++++ src/core/load-fragment.c | 46 ++++++++++++++++++++++------ src/test/test-execute.c | 5 +++ test/meson.build | 1 + test/test-execute/exec-basic.service | 13 ++++++++ 6 files changed, 86 insertions(+), 24 deletions(-) create mode 100644 test/test-execute/exec-basic.service diff --git a/man/systemd.service.xml b/man/systemd.service.xml index b68d351dff..e89cfe3f0e 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -301,8 +301,9 @@ ExecStop= line set. (Services lacking both ExecStart= and ExecStop= are not valid.) - 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: + 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: Special executable prefixes @@ -1004,11 +1005,9 @@ &, and other elements of shell syntax are not supported. - The command to execute must be an absolute path name. It may - contain spaces, but control characters are not allowed. + The command to execute may contain spaces, but control characters are not allowed. - The command line accepts % specifiers as - described in + The command line accepts % specifiers as described in systemd.unit5. Basic environment variable substitution is supported. Use @@ -1022,10 +1021,20 @@ For this type of expansion, quotes are respected when splitting into words, and afterwards removed. + 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 + /usr/local/bin/, /usr/bin/, /bin/ + on systems using split /usr/bin/ and /bin/ + directories, and their sbin/ counterparts on systems using split + bin/ and sbin/. 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. + Example: Environment="ONE=one" 'TWO=two two' -ExecStart=/bin/echo $ONE $TWO ${TWO} +ExecStart=echo $ONE $TWO ${TWO} This will execute /bin/echo with four arguments: one, two, @@ -1035,7 +1044,7 @@ ExecStart=/bin/echo $ONE $TWO ${TWO} Environment=ONE='one' "TWO='two two' too" THREE= ExecStart=/bin/echo ${ONE} ${TWO} ${THREE} ExecStart=/bin/echo $ONE $TWO $THREE - This results in echo being + This results in /bin/echo being called twice, the first time with arguments 'one', 'two two' too, , @@ -1061,27 +1070,27 @@ ExecStart=/bin/echo $ONE $TWO $THREE 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: - ExecStart=/bin/sh -c 'dmesg | tac' + ExecStart=sh -c 'dmesg | tac' Example: - ExecStart=/bin/echo one ; /bin/echo "two two" + ExecStart=echo one ; echo "two two" - This will execute /bin/echo two times, + This will execute echo two times, each time with one argument: one and two two, respectively. Because two commands are specified, Type=oneshot must be used. Example: - ExecStart=/bin/echo / >/dev/null & \; \ -/bin/ls + ExecStart=echo / >/dev/null & \; \ +ls - This will execute /bin/echo + This will execute echo with five arguments: /, >/dev/null, &, ;, and - /bin/ls. + ls.
C escapes supported in command lines and environment variables diff --git a/src/basic/path-util.h b/src/basic/path-util.h index 73f1769fd9..d5b62e295e 100644 --- a/src/basic/path-util.h +++ b/src/basic/path-util.h @@ -30,17 +30,23 @@ #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_; diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index 887eb1cf49..f9d340df23 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -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; diff --git a/src/test/test-execute.c b/src/test/test-execute.c index 81f9b7c344..4c15f1acd5 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -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, diff --git a/test/meson.build b/test/meson.build index 809bd44a93..fd4ac11a39 100644 --- a/test/meson.build +++ b/test/meson.build @@ -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 index 0000000000..456f06951a --- /dev/null +++ b/test/test-execute/exec-basic.service @@ -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 From 42345b178da73c454e26645231dbd826e9a501f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Sun, 25 Mar 2018 21:10:50 +0200 Subject: [PATCH 6/8] test: drop the use of /bin/sh in various test services This is not meant to be comprehensive, just the few cases where the /bin/sh -c wrapper is obviously superfluous. --- test/test-execute/exec-bindpaths.service | 8 ++-- ...dynamicuser-statedir-migrate-step1.service | 16 ++++---- ...dynamicuser-statedir-migrate-step2.service | 32 ++++++++-------- .../exec-dynamicuser-statedir.service | 18 ++++----- ...c-restrictnamespaces-mnt-blacklist.service | 2 +- .../exec-restrictnamespaces-mnt.service | 2 +- .../exec-restrictnamespaces-no.service | 2 +- .../exec-restrictnamespaces-yes.service | 2 +- test/test-execute/exec-specifier.service | 38 +++++++++---------- test/test-execute/exec-specifier@.service | 38 +++++++++---------- .../exec-temporaryfilesystem-rw.service | 16 ++++---- 11 files changed, 87 insertions(+), 87 deletions(-) diff --git a/test/test-execute/exec-bindpaths.service b/test/test-execute/exec-bindpaths.service index 7bd8fa7402..edab18bb0f 100644 --- a/test/test-execute/exec-bindpaths.service +++ b/test/test-execute/exec-bindpaths.service @@ -4,14 +4,14 @@ Description=Test for BindPaths= and BindReadOnlyPaths= [Service] Type=oneshot # Create a file in /tmp/test-exec-bindpaths -ExecStart=/bin/sh -c 'touch /tmp/test-exec-bindpaths/thisisasimpletest' +ExecStart=touch /tmp/test-exec-bindpaths/thisisasimpletest # Then, the file can be access through /tmp -ExecStart=/bin/sh -c 'test -f /tmp/thisisasimpletest' +ExecStart=test -f /tmp/thisisasimpletest # Also, through /tmp/test-exec-bindreadonlypaths -ExecStart=/bin/sh -c 'test -f /tmp/test-exec-bindreadonlypaths/thisisasimpletest' +ExecStart=test -f /tmp/test-exec-bindreadonlypaths/thisisasimpletest # The file cannot modify through /tmp/test-exec-bindreadonlypaths ExecStart=/bin/sh -x -c '! touch /tmp/test-exec-bindreadonlypaths/thisisasimpletest' # Cleanup -ExecStart=/bin/sh -c 'rm /tmp/thisisasimpletest' +ExecStart=rm /tmp/thisisasimpletest BindPaths=/tmp:/tmp/test-exec-bindpaths BindReadOnlyPaths=/tmp:/tmp/test-exec-bindreadonlypaths diff --git a/test/test-execute/exec-dynamicuser-statedir-migrate-step1.service b/test/test-execute/exec-dynamicuser-statedir-migrate-step1.service index 83bdfb311a..72e6d7686f 100644 --- a/test/test-execute/exec-dynamicuser-statedir-migrate-step1.service +++ b/test/test-execute/exec-dynamicuser-statedir-migrate-step1.service @@ -2,14 +2,14 @@ Description=Test DynamicUser= migrate StateDirectory= (preparation) [Service] -ExecStart=/bin/sh -c 'test -w /var/lib/test-dynamicuser-migrate' -ExecStart=/bin/sh -c 'test -w /var/lib/test-dynamicuser-migrate2/hoge' -ExecStart=/bin/sh -c 'test ! -L /var/lib/test-dynamicuser-migrate' -ExecStart=/bin/sh -c 'test ! -L /var/lib/test-dynamicuser-migrate2/hoge' -ExecStart=/bin/sh -c 'test -d /var/lib/test-dynamicuser-migrate' -ExecStart=/bin/sh -c 'test -d /var/lib/test-dynamicuser-migrate2/hoge' -ExecStart=/bin/sh -c 'touch /var/lib/test-dynamicuser-migrate/yay' -ExecStart=/bin/sh -c 'touch /var/lib/test-dynamicuser-migrate2/hoge/yayyay' +ExecStart=test -w /var/lib/test-dynamicuser-migrate +ExecStart=test -w /var/lib/test-dynamicuser-migrate2/hoge +ExecStart=test ! -L /var/lib/test-dynamicuser-migrate +ExecStart=test ! -L /var/lib/test-dynamicuser-migrate2/hoge +ExecStart=test -d /var/lib/test-dynamicuser-migrate +ExecStart=test -d /var/lib/test-dynamicuser-migrate2/hoge +ExecStart=touch /var/lib/test-dynamicuser-migrate/yay +ExecStart=touch /var/lib/test-dynamicuser-migrate2/hoge/yayyay Type=oneshot DynamicUser=no diff --git a/test/test-execute/exec-dynamicuser-statedir-migrate-step2.service b/test/test-execute/exec-dynamicuser-statedir-migrate-step2.service index 8154922a2f..5a61228c9f 100644 --- a/test/test-execute/exec-dynamicuser-statedir-migrate-step2.service +++ b/test/test-execute/exec-dynamicuser-statedir-migrate-step2.service @@ -2,22 +2,22 @@ Description=Test DynamicUser= migrate StateDirectory= (preparation) [Service] -ExecStart=/bin/sh -c 'test -w /var/lib/test-dynamicuser-migrate' -ExecStart=/bin/sh -c 'test -w /var/lib/test-dynamicuser-migrate2/hoge' -ExecStart=/bin/sh -c 'test -L /var/lib/test-dynamicuser-migrate' -ExecStart=/bin/sh -c 'test -L /var/lib/test-dynamicuser-migrate2/hoge' -ExecStart=/bin/sh -c 'test -d /var/lib/test-dynamicuser-migrate' -ExecStart=/bin/sh -c 'test -d /var/lib/test-dynamicuser-migrate2/hoge' -ExecStart=/bin/sh -c 'test -f /var/lib/test-dynamicuser-migrate/yay' -ExecStart=/bin/sh -c 'test -f /var/lib/test-dynamicuser-migrate2/hoge/yayyay' -ExecStart=/bin/sh -c 'test -d /var/lib/private/test-dynamicuser-migrate' -ExecStart=/bin/sh -c 'test -d /var/lib/private/test-dynamicuser-migrate2/hoge' -ExecStart=/bin/sh -c 'test -f /var/lib/private/test-dynamicuser-migrate/yay' -ExecStart=/bin/sh -c 'test -f /var/lib/private/test-dynamicuser-migrate2/hoge/yayyay' -ExecStart=/bin/sh -c 'touch /var/lib/test-dynamicuser-migrate/yay' -ExecStart=/bin/sh -c 'touch /var/lib/test-dynamicuser-migrate2/hoge/yayyay' -ExecStart=/bin/sh -c 'touch /var/lib/private/test-dynamicuser-migrate/yay' -ExecStart=/bin/sh -c 'touch /var/lib/private/test-dynamicuser-migrate2/hoge/yayyay' +ExecStart=test -w /var/lib/test-dynamicuser-migrate +ExecStart=test -w /var/lib/test-dynamicuser-migrate2/hoge +ExecStart=test -L /var/lib/test-dynamicuser-migrate +ExecStart=test -L /var/lib/test-dynamicuser-migrate2/hoge +ExecStart=test -d /var/lib/test-dynamicuser-migrate +ExecStart=test -d /var/lib/test-dynamicuser-migrate2/hoge +ExecStart=test -f /var/lib/test-dynamicuser-migrate/yay +ExecStart=test -f /var/lib/test-dynamicuser-migrate2/hoge/yayyay +ExecStart=test -d /var/lib/private/test-dynamicuser-migrate +ExecStart=test -d /var/lib/private/test-dynamicuser-migrate2/hoge +ExecStart=test -f /var/lib/private/test-dynamicuser-migrate/yay +ExecStart=test -f /var/lib/private/test-dynamicuser-migrate2/hoge/yayyay +ExecStart=touch /var/lib/test-dynamicuser-migrate/yay +ExecStart=touch /var/lib/test-dynamicuser-migrate2/hoge/yayyay +ExecStart=touch /var/lib/private/test-dynamicuser-migrate/yay +ExecStart=touch /var/lib/private/test-dynamicuser-migrate2/hoge/yayyay Type=oneshot DynamicUser=yes diff --git a/test/test-execute/exec-dynamicuser-statedir.service b/test/test-execute/exec-dynamicuser-statedir.service index 5ea6d9da42..c771717904 100644 --- a/test/test-execute/exec-dynamicuser-statedir.service +++ b/test/test-execute/exec-dynamicuser-statedir.service @@ -2,17 +2,17 @@ Description=Test DynamicUser= with StateDirectory= [Service] -ExecStart=/bin/sh -c 'test -w /var/lib/waldo' -ExecStart=/bin/sh -c 'test -w /var/lib/quux/pief' -ExecStart=/bin/sh -c 'touch /var/lib/waldo/yay' -ExecStart=/bin/sh -c 'touch /var/lib/quux/pief/yayyay' -ExecStart=/bin/sh -c 'test -f /var/lib/waldo/yay' -ExecStart=/bin/sh -c 'test -f /var/lib/quux/pief/yayyay' -ExecStart=/bin/sh -c 'test -f /var/lib/private/waldo/yay' -ExecStart=/bin/sh -c 'test -f /var/lib/private/quux/pief/yayyay' +ExecStart=test -w /var/lib/waldo +ExecStart=test -w /var/lib/quux/pief +ExecStart=touch /var/lib/waldo/yay +ExecStart=touch /var/lib/quux/pief/yayyay +ExecStart=test -f /var/lib/waldo/yay +ExecStart=test -f /var/lib/quux/pief/yayyay +ExecStart=test -f /var/lib/private/waldo/yay +ExecStart=test -f /var/lib/private/quux/pief/yayyay # Make sure that /var/lib/private/waldo is really the only writable directory besides the obvious candidates -ExecStart=/bin/sh -x -c 'test $$(find / -type d -writable 2> /dev/null | egrep -v -e \'^(/var/tmp$$|/tmp$$|/proc/|/dev/mqueue$$|/dev/shm$$|/sys/fs/bpf$$)\' | sort -u | tr -d '\\\\n') = /var/lib/private/quux/pief/var/lib/private/waldo' +ExecStart=sh -x -c 'test $$(find / -type d -writable 2> /dev/null | egrep -v -e \'^(/var/tmp$$|/tmp$$|/proc/|/dev/mqueue$$|/dev/shm$$|/sys/fs/bpf$$)\' | sort -u | tr -d '\\\\n') = /var/lib/private/quux/pief/var/lib/private/waldo' Type=oneshot DynamicUser=yes diff --git a/test/test-execute/exec-restrictnamespaces-mnt-blacklist.service b/test/test-execute/exec-restrictnamespaces-mnt-blacklist.service index ab909cbd94..7756a2575e 100644 --- a/test/test-execute/exec-restrictnamespaces-mnt-blacklist.service +++ b/test/test-execute/exec-restrictnamespaces-mnt-blacklist.service @@ -3,5 +3,5 @@ Description=Test RestrictNamespaces=~mnt [Service] RestrictNamespaces=~mnt -ExecStart=/bin/sh -x -c 'unshare -m' +ExecStart=unshare -m Type=oneshot diff --git a/test/test-execute/exec-restrictnamespaces-mnt.service b/test/test-execute/exec-restrictnamespaces-mnt.service index 1aeed72717..2c5b942601 100644 --- a/test/test-execute/exec-restrictnamespaces-mnt.service +++ b/test/test-execute/exec-restrictnamespaces-mnt.service @@ -3,5 +3,5 @@ Description=Test RestrictNamespaces=mnt [Service] RestrictNamespaces=mnt -ExecStart=/bin/sh -x -c 'unshare -m' +ExecStart=unshare -m Type=oneshot diff --git a/test/test-execute/exec-restrictnamespaces-no.service b/test/test-execute/exec-restrictnamespaces-no.service index 33500302d2..5ffe081e45 100644 --- a/test/test-execute/exec-restrictnamespaces-no.service +++ b/test/test-execute/exec-restrictnamespaces-no.service @@ -3,5 +3,5 @@ Description=Test RestrictNamespaces=no [Service] RestrictNamespaces=no -ExecStart=/bin/sh -x -c 'unshare -m -u -i -n -p -f' +ExecStart=unshare -m -u -i -n -p -f Type=oneshot diff --git a/test/test-execute/exec-restrictnamespaces-yes.service b/test/test-execute/exec-restrictnamespaces-yes.service index 3fe70e2bea..8e077ed3a0 100644 --- a/test/test-execute/exec-restrictnamespaces-yes.service +++ b/test/test-execute/exec-restrictnamespaces-yes.service @@ -3,5 +3,5 @@ Description=Test RestrictNamespaces=yes [Service] RestrictNamespaces=yes -ExecStart=/bin/sh -x -c 'unshare -m' +ExecStart=unshare -m Type=oneshot diff --git a/test/test-execute/exec-specifier.service b/test/test-execute/exec-specifier.service index 70227eac68..65a64bc95e 100644 --- a/test/test-execute/exec-specifier.service +++ b/test/test-execute/exec-specifier.service @@ -3,22 +3,22 @@ Description=Test for specifiers [Service] Type=oneshot -ExecStart=/usr/bin/test %n = exec-specifier.service -ExecStart=/usr/bin/test %N = exec-specifier -ExecStart=/usr/bin/test %p = exec-specifier -ExecStart=/usr/bin/test %P = exec/specifier -ExecStart=/usr/bin/test %i = "" -ExecStart=/usr/bin/test %I = "" -ExecStart=/usr/bin/test %f = /exec/specifier -ExecStart=/usr/bin/test %t = /run -ExecStart=/usr/bin/test %S = /var/lib -ExecStart=/usr/bin/test %C = /var/cache -ExecStart=/usr/bin/test %L = /var/log -ExecStart=/bin/sh -c 'test %u = $$(id -un 0)' -ExecStart=/usr/bin/test %U = 0 -ExecStart=/bin/sh -c 'test %h = $$(getent passwd 0 | cut -d: -f 6)' -ExecStart=/bin/sh -c 'test %s = $$(getent passwd 0 | cut -d: -f 7)' -ExecStart=/bin/sh -c 'test %m = $$(cat /etc/machine-id)' -ExecStart=/bin/sh -c 'test %b = $$(cat /proc/sys/kernel/random/boot_id | sed -e 's/-//g')' -ExecStart=/bin/sh -c 'test %H = $$(hostname)' -ExecStart=/bin/sh -c 'test %v = $$(uname -r)' +ExecStart=test %n = exec-specifier.service +ExecStart=test %N = exec-specifier +ExecStart=test %p = exec-specifier +ExecStart=test %P = exec/specifier +ExecStart=test %i = "" +ExecStart=test %I = "" +ExecStart=test %f = /exec/specifier +ExecStart=test %t = /run +ExecStart=test %S = /var/lib +ExecStart=test %C = /var/cache +ExecStart=test %L = /var/log +ExecStart=sh -c 'test %u = $$(id -un 0)' +ExecStart=test %U = 0 +ExecStart=sh -c 'test %h = $$(getent passwd 0 | cut -d: -f 6)' +ExecStart=sh -c 'test %s = $$(getent passwd 0 | cut -d: -f 7)' +ExecStart=sh -c 'test %m = $$(cat /etc/machine-id)' +ExecStart=sh -c 'test %b = $$(cat /proc/sys/kernel/random/boot_id | sed -e 's/-//g')' +ExecStart=sh -c 'test %H = $$(hostname)' +ExecStart=sh -c 'test %v = $$(uname -r)' diff --git a/test/test-execute/exec-specifier@.service b/test/test-execute/exec-specifier@.service index a90af5aee4..f8d7dce680 100644 --- a/test/test-execute/exec-specifier@.service +++ b/test/test-execute/exec-specifier@.service @@ -3,22 +3,22 @@ Description=Test for specifiers (template unit) [Service] Type=oneshot -ExecStart=/usr/bin/test %n = exec-specifier@foo-bar.service -ExecStart=/usr/bin/test %N = exec-specifier@foo-bar -ExecStart=/usr/bin/test %p = exec-specifier -ExecStart=/usr/bin/test %P = exec/specifier -ExecStart=/usr/bin/test %i = foo-bar -ExecStart=/usr/bin/test %I = foo/bar -ExecStart=/usr/bin/test %f = /foo/bar -ExecStart=/usr/bin/test %t = /run -ExecStart=/usr/bin/test %S = /var/lib -ExecStart=/usr/bin/test %C = /var/cache -ExecStart=/usr/bin/test %L = /var/log -ExecStart=/bin/sh -c 'test %u = $$(id -un 0)' -ExecStart=/usr/bin/test %U = 0 -ExecStart=/bin/sh -c 'test %h = $$(getent passwd 0 | cut -d: -f 6)' -ExecStart=/bin/sh -c 'test %s = $$(getent passwd 0 | cut -d: -f 7)' -ExecStart=/bin/sh -c 'test %m = $$(cat /etc/machine-id)' -ExecStart=/bin/sh -c 'test %b = $$(cat /proc/sys/kernel/random/boot_id | sed -e 's/-//g')' -ExecStart=/bin/sh -c 'test %H = $$(hostname)' -ExecStart=/bin/sh -c 'test %v = $$(uname -r)' +ExecStart=test %n = exec-specifier@foo-bar.service +ExecStart=test %N = exec-specifier@foo-bar +ExecStart=test %p = exec-specifier +ExecStart=test %P = exec/specifier +ExecStart=test %i = foo-bar +ExecStart=test %I = foo/bar +ExecStart=test %f = /foo/bar +ExecStart=test %t = /run +ExecStart=test %S = /var/lib +ExecStart=test %C = /var/cache +ExecStart=test %L = /var/log +ExecStart=sh -c 'test %u = $$(id -un 0)' +ExecStart=test %U = 0 +ExecStart=sh -c 'test %h = $$(getent passwd 0 | cut -d: -f 6)' +ExecStart=sh -c 'test %s = $$(getent passwd 0 | cut -d: -f 7)' +ExecStart=sh -c 'test %m = $$(cat /etc/machine-id)' +ExecStart=sh -c 'test %b = $$(cat /proc/sys/kernel/random/boot_id | sed -e 's/-//g')' +ExecStart=sh -c 'test %H = $$(hostname)' +ExecStart=sh -c 'test %v = $$(uname -r)' diff --git a/test/test-execute/exec-temporaryfilesystem-rw.service b/test/test-execute/exec-temporaryfilesystem-rw.service index fc02ceab1c..379ad066fb 100644 --- a/test/test-execute/exec-temporaryfilesystem-rw.service +++ b/test/test-execute/exec-temporaryfilesystem-rw.service @@ -5,28 +5,28 @@ Description=Test for TemporaryFileSystem Type=oneshot # Check directories exist -ExecStart=/bin/sh -c 'test -d /var/test-exec-temporaryfilesystem/rw && test -d /var/test-exec-temporaryfilesystem/ro' +ExecStart=test -d /var/test-exec-temporaryfilesystem/rw -a -d /var/test-exec-temporaryfilesystem/ro # Check TemporaryFileSystem= are empty -ExecStart=/bin/sh -c 'for i in $$(ls -A /var); do test $$i = test-exec-temporaryfilesystem || false; done' +ExecStart=sh -c 'for i in $$(ls -A /var); do test $$i = test-exec-temporaryfilesystem || false; done' # Create a file in /var -ExecStart=/bin/sh -c 'touch /var/hoge' +ExecStart=touch /var/hoge # Create a file in /var/test-exec-temporaryfilesystem/rw -ExecStart=/bin/sh -c 'touch /var/test-exec-temporaryfilesystem/rw/thisisasimpletest-temporaryfilesystem' +ExecStart=touch /var/test-exec-temporaryfilesystem/rw/thisisasimpletest-temporaryfilesystem # Then, the file can be access through /tmp -ExecStart=/bin/sh -c 'test -f /tmp/thisisasimpletest-temporaryfilesystem' +ExecStart=test -f /tmp/thisisasimpletest-temporaryfilesystem # Also, through /var/test-exec-temporaryfilesystem/ro -ExecStart=/bin/sh -c 'test -f /var/test-exec-temporaryfilesystem/ro/thisisasimpletest-temporaryfilesystem' +ExecStart=test -f /var/test-exec-temporaryfilesystem/ro/thisisasimpletest-temporaryfilesystem # The file cannot modify through /var/test-exec-temporaryfilesystem/ro -ExecStart=/bin/sh -c '! touch /var/test-exec-temporaryfilesystem/ro/thisisasimpletest-temporaryfilesystem' +ExecStart=sh -c '! touch /var/test-exec-temporaryfilesystem/ro/thisisasimpletest-temporaryfilesystem' # Cleanup -ExecStart=/bin/sh -c 'rm /tmp/thisisasimpletest-temporaryfilesystem' +ExecStart=rm /tmp/thisisasimpletest-temporaryfilesystem TemporaryFileSystem=/var BindPaths=/tmp:/var/test-exec-temporaryfilesystem/rw From e12d446b6623cedaf2b92c5e935312f7ade6cfef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Mon, 26 Mar 2018 09:51:12 +0200 Subject: [PATCH 7/8] systemd-path: allow the default search path to be queried --- man/systemd-path.xml | 7 ++++--- man/systemd.service.xml | 3 ++- src/libsystemd/sd-path/sd-path.c | 14 +++++++++++++- src/path/path.c | 1 + src/systemd/sd-path.h | 1 + 5 files changed, 21 insertions(+), 5 deletions(-) diff --git a/man/systemd-path.xml b/man/systemd-path.xml index 7144569d59..025247c883 100644 --- a/man/systemd-path.xml +++ b/man/systemd-path.xml @@ -52,7 +52,9 @@ - systemd-path OPTIONS NAME + systemd-path + OPTIONS + NAME @@ -81,8 +83,7 @@ - The printed paths are suffixed by the - specified string. + Printed paths are suffixed by the specified string. diff --git a/man/systemd.service.xml b/man/systemd.service.xml index e89cfe3f0e..55a9c6fbdc 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -1029,7 +1029,8 @@ bin/ and sbin/. 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. + ambiguity. Hint: this search path may be queried using + systemd-path search-binaries-default. Example: diff --git a/src/libsystemd/sd-path/sd-path.c b/src/libsystemd/sd-path/sd-path.c index 419c763668..b2e8e63c1b 100644 --- a/src/libsystemd/sd-path/sd-path.c +++ b/src/libsystemd/sd-path/sd-path.c @@ -348,6 +348,7 @@ _public_ int sd_path_home(uint64_t type, const char *suffix, char **path) { if (IN_SET(type, SD_PATH_SEARCH_BINARIES, + SD_PATH_SEARCH_BINARIES_DEFAULT, SD_PATH_SEARCH_LIBRARY_PRIVATE, SD_PATH_SEARCH_LIBRARY_ARCH, SD_PATH_SEARCH_SHARED, @@ -566,7 +567,17 @@ static int get_search(uint64_t type, char ***list) { false, "/etc", NULL); - } + + case SD_PATH_SEARCH_BINARIES_DEFAULT: { + char **t; + + t = strv_split_nulstr(DEFAULT_PATH_NULSTR); + if (!t) + return -ENOMEM; + + *list = t; + return 0; + }} return -EOPNOTSUPP; } @@ -579,6 +590,7 @@ _public_ int sd_path_search(uint64_t type, const char *suffix, char ***paths) { if (!IN_SET(type, SD_PATH_SEARCH_BINARIES, + SD_PATH_SEARCH_BINARIES_DEFAULT, SD_PATH_SEARCH_LIBRARY_PRIVATE, SD_PATH_SEARCH_LIBRARY_ARCH, SD_PATH_SEARCH_SHARED, diff --git a/src/path/path.c b/src/path/path.c index 0f029c4ae6..be17a444d3 100644 --- a/src/path/path.c +++ b/src/path/path.c @@ -67,6 +67,7 @@ static const char* const path_table[_SD_PATH_MAX] = { [SD_PATH_USER_TEMPLATES] = "user-templates", [SD_PATH_USER_DESKTOP] = "user-desktop", [SD_PATH_SEARCH_BINARIES] = "search-binaries", + [SD_PATH_SEARCH_BINARIES_DEFAULT] = "search-binaries-default", [SD_PATH_SEARCH_LIBRARY_PRIVATE] = "search-library-private", [SD_PATH_SEARCH_LIBRARY_ARCH] = "search-library-arch", [SD_PATH_SEARCH_SHARED] = "search-shared", diff --git a/src/systemd/sd-path.h b/src/systemd/sd-path.h index 2dfc8967b4..19f48b73eb 100644 --- a/src/systemd/sd-path.h +++ b/src/systemd/sd-path.h @@ -74,6 +74,7 @@ enum { /* Search paths */ SD_PATH_SEARCH_BINARIES, + SD_PATH_SEARCH_BINARIES_DEFAULT, SD_PATH_SEARCH_LIBRARY_PRIVATE, SD_PATH_SEARCH_LIBRARY_ARCH, SD_PATH_SEARCH_SHARED, From 7e4a49b42b2f4fb3a621dc89a4a76e330a9aebe9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 11 Apr 2018 16:50:48 +0200 Subject: [PATCH 8/8] test-execute: make find invocation a bit more efficent, increase timeout We go through the whole file system, so this test can take arbitrary time. But this test is still quite useful, so let's at least try to make it more efficent by not descending at all into the directories we would filter out later on anyway. Also increase the timeout, in case the previous step doesn't help enough. --- src/test/meson.build | 3 ++- test/test-execute/exec-dynamicuser-statedir.service | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/src/test/meson.build b/src/test/meson.build index 205a09d62a..3b1201f3e9 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -566,7 +566,8 @@ tests += [ libseccomp, libselinux, libmount, - libblkid]], + libblkid], + '', 'timeout=360'], [['src/test/test-siphash24.c'], [], diff --git a/test/test-execute/exec-dynamicuser-statedir.service b/test/test-execute/exec-dynamicuser-statedir.service index c771717904..f459f3c1eb 100644 --- a/test/test-execute/exec-dynamicuser-statedir.service +++ b/test/test-execute/exec-dynamicuser-statedir.service @@ -12,7 +12,7 @@ ExecStart=test -f /var/lib/private/waldo/yay ExecStart=test -f /var/lib/private/quux/pief/yayyay # Make sure that /var/lib/private/waldo is really the only writable directory besides the obvious candidates -ExecStart=sh -x -c 'test $$(find / -type d -writable 2> /dev/null | egrep -v -e \'^(/var/tmp$$|/tmp$$|/proc/|/dev/mqueue$$|/dev/shm$$|/sys/fs/bpf$$)\' | sort -u | tr -d '\\\\n') = /var/lib/private/quux/pief/var/lib/private/waldo' +ExecStart=sh -x -c 'test $$(find / \( -path /var/tmp -o -path /tmp -o -path /proc -o -path /dev/mqueue -o -path /dev/shm -o -path /sys/fs/bpf \) -prune -o -type d -writable -print 2>/dev/null | sort -u | tr -d '\\\\n') = /var/lib/private/quux/pief/var/lib/private/waldo' Type=oneshot DynamicUser=yes