From e23d911664b4fd86eb2c24b64233cb9f23cffdd1 Mon Sep 17 00:00:00 2001 From: Anita Zhang Date: Fri, 4 Oct 2019 16:03:04 -0700 Subject: [PATCH 1/3] core: disallow using '-.service' as a service name -.service.d will become a special top level drop in so don't let it be a usable service name (otherwise the interaction gets complicated). --- src/basic/special.h | 4 ++++ src/basic/unit-name.c | 25 +++++++++++++++++++++++++ src/basic/unit-name.h | 2 ++ src/core/service.c | 5 +++++ src/test/test-unit-name.c | 19 +++++++++++++++++++ 5 files changed, 55 insertions(+) diff --git a/src/basic/special.h b/src/basic/special.h index add1c1d507..6475501078 100644 --- a/src/basic/special.h +++ b/src/basic/special.h @@ -105,3 +105,7 @@ /* The root directory. */ #define SPECIAL_ROOT_MOUNT "-.mount" + +/* Used to apply settings to all services through drop-ins. + * Should not exist as an actual service. */ +#define SPECIAL_ROOT_SERVICE "-.service" diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index 4226f3014d..ecbf5ae7f5 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -665,6 +665,31 @@ good: return 0; } +bool service_unit_name_is_valid(const char *name) { + _cleanup_free_ char *prefix = NULL, *s = NULL; + const char *e, *service_name = name; + + if (!unit_name_is_valid(name, UNIT_NAME_ANY)) + return false; + + e = endswith(name, ".service"); + if (!e) + return false; + + /* If it's a template or instance, get the prefix as a service name. */ + if (unit_name_is_valid(name, UNIT_NAME_INSTANCE|UNIT_NAME_TEMPLATE)) { + assert_se(unit_name_to_prefix(name, &prefix) == 0); + assert_se(s = strjoin(prefix, ".service")); + service_name = s; + } + + /* Reject reserved service name(s). */ + if (streq(service_name, SPECIAL_ROOT_SERVICE)) + return false; + + return true; +} + int slice_build_parent_slice(const char *slice, char **ret) { char *s, *dash; int r; diff --git a/src/basic/unit-name.h b/src/basic/unit-name.h index 2e060ff3e8..ddcfc1b349 100644 --- a/src/basic/unit-name.h +++ b/src/basic/unit-name.h @@ -58,6 +58,8 @@ static inline int unit_name_mangle(const char *name, UnitNameMangle flags, char return unit_name_mangle_with_suffix(name, flags, ".service", ret); } +bool service_unit_name_is_valid(const char *name); + int slice_build_parent_slice(const char *slice, char **ret); int slice_build_subslice(const char *slice, const char *name, char **subslice); bool slice_name_is_valid(const char *name); diff --git a/src/core/service.c b/src/core/service.c index ada25e634a..6880b24535 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -552,6 +552,11 @@ static int service_verify(Service *s) { if (UNIT(s)->load_state != UNIT_LOADED) return 0; + if (!service_unit_name_is_valid(UNIT(s)->id)) { + log_unit_error(UNIT(s), "Service name is invalid or reserved. Refusing."); + return -ENOEXEC; + } + if (!s->exec_command[SERVICE_EXEC_START] && !s->exec_command[SERVICE_EXEC_STOP] && UNIT(s)->success_action == EMERGENCY_ACTION_NONE) { /* FailureAction= only makes sense if one of the start or stop commands is specified. diff --git a/src/test/test-unit-name.c b/src/test/test-unit-name.c index 25c649828e..aa072c4ca8 100644 --- a/src/test/test-unit-name.c +++ b/src/test/test-unit-name.c @@ -355,6 +355,24 @@ static void test_unit_name_build(void) { free(t); } +static void test_service_unit_name_is_valid(void) { + assert_se(service_unit_name_is_valid("foo.service")); + assert_se(service_unit_name_is_valid("foo@bar.service")); + assert_se(service_unit_name_is_valid("foo@bar@bar.service")); + assert_se(service_unit_name_is_valid("--.service")); + assert_se(service_unit_name_is_valid(".-.service")); + assert_se(service_unit_name_is_valid("-foo-bar.service")); + assert_se(service_unit_name_is_valid("-foo-bar-.service")); + assert_se(service_unit_name_is_valid("foo-bar-.service")); + + assert_se(!service_unit_name_is_valid("-.service")); + assert_se(!service_unit_name_is_valid("")); + assert_se(!service_unit_name_is_valid("foo.slice")); + assert_se(!service_unit_name_is_valid("@.service")); + assert_se(!service_unit_name_is_valid("@bar.service")); + assert_se(!service_unit_name_is_valid("-@.service")); +} + static void test_slice_name_is_valid(void) { assert_se( slice_name_is_valid(SPECIAL_ROOT_SLICE)); assert_se( slice_name_is_valid("foo.slice")); @@ -840,6 +858,7 @@ int main(int argc, char* argv[]) { test_unit_prefix_is_valid(); test_unit_name_change_suffix(); test_unit_name_build(); + test_service_unit_name_is_valid(); test_slice_name_is_valid(); test_build_subslice(); test_build_parent_slice(); From f18f809c07ec082a3006666c7c7e20f7045c4d87 Mon Sep 17 00:00:00 2001 From: Anita Zhang Date: Fri, 4 Oct 2019 18:14:19 -0700 Subject: [PATCH 2/3] basic/set: const-ify set_first() set_steal_first() is the version that modifies Set so this one should be const. --- src/basic/set.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/basic/set.h b/src/basic/set.h index 2bb26c68b8..5f1956177e 100644 --- a/src/basic/set.h +++ b/src/basic/set.h @@ -102,8 +102,8 @@ static inline void *set_steal_first(Set *s) { /* no set_steal_first_key */ /* no set_first_key */ -static inline void *set_first(Set *s) { - return internal_hashmap_first_key_and_value(HASHMAP_BASE(s), false, NULL); +static inline void *set_first(const Set *s) { + return internal_hashmap_first_key_and_value(HASHMAP_BASE((Set *) s), false, NULL); } /* no set_next */ From d272467882c9c3c3d4faca5fd7a1f44c5ef2f064 Mon Sep 17 00:00:00 2001 From: Anita Zhang Date: Fri, 4 Oct 2019 17:39:34 -0700 Subject: [PATCH 3/3] shared/dropin: support -.service.d/ top level drop-in for service units Closes #12830 --- man/systemd.service.xml | 13 +++++++++++++ man/systemd.special.xml | 9 +++++++++ man/systemd.unit.xml | 4 ++++ src/basic/unit-name.c | 9 +++++++-- src/core/service.c | 2 +- src/shared/dropin.c | 23 +++++++++++++++++++++++ test/TEST-15-DROPIN/test-dropin.sh | 13 +++++++++++++ 7 files changed, 70 insertions(+), 3 deletions(-) diff --git a/man/systemd.service.xml b/man/systemd.service.xml index af14eedfcc..7d120c3243 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -63,6 +63,19 @@ The systemd-run1 command allows creating .service and .scope units dynamically and transiently from the command line. + + In addition to the various drop-in behaviors described in + systemd.unit5, + services also support a top-level drop-in with -.service.d/ that allows + altering or adding to the settings of all services on the system. + The formatting and precedence of applying drop-in configurations follow what is defined in + systemd.unit5. + However, configurations in -.service.d/ have the lowest precedence compared to settings + in the service specific override directories. For example, for foo-bar-baz.service, + drop-ins in foo-bar-baz.service.d/ override the ones in + foo-bar-.service.d/, which override the ones foo-.service.d/, + which override the ones in -.service.d/. + diff --git a/man/systemd.special.xml b/man/systemd.special.xml index 248fb924db..afd14b977c 100644 --- a/man/systemd.special.xml +++ b/man/systemd.special.xml @@ -119,6 +119,15 @@ + + -.service + + This is a reserved unit name used to support top-level drop-ins for services. See + systemd.service5 + for details. + + + basic.target diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index d812108d3c..f6e9d2a17c 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -192,6 +192,10 @@ over unit files wherever located. Multiple drop-in files with different names are applied in lexicographic order, regardless of which of the directories they reside in. + Service units also support a top-level drop-in directory for modifying the settings of all service units. See + systemd.service5 + for details. + diff --git a/src/basic/unit-name.c b/src/basic/unit-name.c index ecbf5ae7f5..bcd01f8515 100644 --- a/src/basic/unit-name.c +++ b/src/basic/unit-name.c @@ -678,8 +678,13 @@ bool service_unit_name_is_valid(const char *name) { /* If it's a template or instance, get the prefix as a service name. */ if (unit_name_is_valid(name, UNIT_NAME_INSTANCE|UNIT_NAME_TEMPLATE)) { - assert_se(unit_name_to_prefix(name, &prefix) == 0); - assert_se(s = strjoin(prefix, ".service")); + if (unit_name_to_prefix(name, &prefix) < 0) + return false; + + s = strjoin(prefix, ".service"); + if (!s) + return false; + service_name = s; } diff --git a/src/core/service.c b/src/core/service.c index 6880b24535..24ad1e77fa 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -554,7 +554,7 @@ static int service_verify(Service *s) { if (!service_unit_name_is_valid(UNIT(s)->id)) { log_unit_error(UNIT(s), "Service name is invalid or reserved. Refusing."); - return -ENOEXEC; + return -EINVAL; } if (!s->exec_command[SERVICE_EXEC_START] && !s->exec_command[SERVICE_EXEC_STOP] diff --git a/src/shared/dropin.c b/src/shared/dropin.c index 4a29bd09c5..2f67a44bf0 100644 --- a/src/shared/dropin.c +++ b/src/shared/dropin.c @@ -19,6 +19,7 @@ #include "mkdir.h" #include "path-util.h" #include "set.h" +#include "special.h" #include "string-util.h" #include "strv.h" #include "unit-name.h" @@ -226,12 +227,34 @@ int unit_file_find_dropin_paths( char ***ret) { _cleanup_strv_free_ char **dirs = NULL; + UnitType type = _UNIT_TYPE_INVALID; char *name, **p; Iterator i; int r; assert(ret); + /* All the names in the unit are of the same type so just grab one. */ + name = (char*) set_first(names); + if (name) { + type = unit_name_to_type(name); + if (type < 0) + return log_error_errno(SYNTHETIC_ERRNO(EINVAL), + "Failed to to derive unit type from unit name: %s", + name); + } + + /* Special drop in for -.service. Add this first as it's the most generic + * and should be able to be overridden by more specific drop-ins. */ + if (type == UNIT_SERVICE) + STRV_FOREACH(p, lookup_path) + (void) unit_file_find_dirs(original_root, + unit_path_cache, + *p, + SPECIAL_ROOT_SERVICE, + dir_suffix, + &dirs); + SET_FOREACH(name, names, i) STRV_FOREACH(p, lookup_path) (void) unit_file_find_dirs(original_root, unit_path_cache, *p, name, dir_suffix, &dirs); diff --git a/test/TEST-15-DROPIN/test-dropin.sh b/test/TEST-15-DROPIN/test-dropin.sh index 2cef5a3c5b..75c36df2b8 100755 --- a/test/TEST-15-DROPIN/test-dropin.sh +++ b/test/TEST-15-DROPIN/test-dropin.sh @@ -101,6 +101,19 @@ test_basic_dropins () { check_ok b Wants c.service systemctl stop a c + echo "*** test -.service.d/ top level drop-in" + create_services a b + check_ko a ExecCondition "/bin/echo a" + check_ko b ExecCondition "/bin/echo b" + mkdir -p /usr/lib/systemd/system/-.service.d + cat >/usr/lib/systemd/system/-.service.d/override.conf <