From fe85ea0d83d43d312878fbfe406068677025c6c6 Mon Sep 17 00:00:00 2001 From: Franck Bui Date: Fri, 27 Jan 2017 16:01:15 +0100 Subject: [PATCH] core: introduce dependency symlink mask This patch allows masking of .wants symlinks in /usr/lib by /dev/null symlinks in /etc. IOW it gives the possibility to mask a dependency symlink (in .wants or .requires), assuming that the later being located in a directory having a 'lower' precedence. As a consequence a dependency symlink mask in /etc/ will mask all equivalent dependency symlinks located in directories such as /run, /usr/lib. And a dependency symlink mask in /usr/lib will has no effect on a dropin symlink located in /run or /etc. Consider the following example: /etc/systemd/system/foo.service.wants/bar.service -> /dev/null /usr/lib/systemd/system/foo.service.wants/bar.service -> ../bar.service service 'foo' was installed during a package installation with a dependency on 'bar' and later sysadmin decided to disable this dependency by adding a mask in /etc/. This results in service 'bar' not being pulled in by 'bar' anymore. The primary use case is to allow a clean separation between service activation/deactivation done during package installations/updates and those done by sysadmin. Assuming that distro dependency symlinks happens in /usr/lib only, the sysadmin is now able to disable persistently a service by creating a mask in /etc/. This mask will take precedence over any distro policies (usually defined by presets) defined in /usr/lib. The distro policies (presets) are now free to be changed without interfering with the configuration done by the sysadmin (if any). They will still be preserved and will still be taken into account. The assumption on distros creating symlinks in /usr/lib only is currently not met since this happens in /etc/. However the dropin symlink mask is the first step to achieve this clean separation. Later changes will ensure that dropin symlinks created during package installations/updates will happen in /usr/lib only. See issue #4830. In order to implement the masking of dependency symlinks properly, this patch defers the handling of those dependencies until all involved units are fully loaded. This way all units and especially their aliases are known and a mask can be effective on a specific unit including all of its aliases. This is due to the way we currently handle the unit aliases (how a unit and its aliases are "lazily" merged) and also due to the fact that we can't remove dependencies added to a unit (until a full daemon-reload happens). As an example: /etc/.../a.service.wants/b1.service -> /dev/null /usr/.../a.service.wants/b.service -> ../b.service and 'b1' is an alias of 'b'. In this example, dropin symlink dependencies pulling 'b' or any of its aliases (including 'b1') will be masked. This patch also changes a rather odd behavior that nobody sane should rely on: a .wants symlink to /dev/null was masking the unit the symlink was refering to. Therefore: /etc/.../a.wants/b.service -> /dev/null would have masked 'b' service. Whereas now, it only masks the corresponding dependency symlinks defined in a.wants/ directories. It also changes the following weird case: /etc/.../a.service.wants/b.service -> ../c.service where 'b' defines an alias for 'c'. I'm not sure why symlinks were used to define dependencies in .wants/.requires in the first place whereas a simple empty file would have been sufficient and unambiguous. With this patch applied, no more aliases are defined this way. --- src/core/load-dropin.c | 220 ++++++++++++++++++++++++++++++++++++++--- src/core/load-dropin.h | 2 + src/core/unit.c | 13 +++ src/core/unit.h | 3 + 4 files changed, 225 insertions(+), 13 deletions(-) diff --git a/src/core/load-dropin.c b/src/core/load-dropin.c index f83fa09301530..75673695b8d55 100644 --- a/src/core/load-dropin.c +++ b/src/core/load-dropin.c @@ -19,51 +19,245 @@ #include "conf-parser.h" +#include "fs-util.h" #include "load-dropin.h" #include "load-fragment.h" #include "log.h" +#include "path-util.h" +#include "stat-util.h" #include "strv.h" #include "unit-name.h" #include "unit.h" + +typedef struct DependencyConsumerData { + Unit *unit; + int depth; +} DependencyConsumerData; + +typedef struct DropinSymlink { + Unit *unit; + UnitDependency dependency; + int depth; /* path depth used when merging 2 dropins */ + bool is_mask; /* does the symlink points to /dev/null ? */ +} DropinSymlink; + +static void dropin_hash_func(const void *p, struct siphash *state) { + const DropinSymlink *ds = p; + + siphash24_compress(&ds->unit, sizeof(ds->unit), state); +} + +static int dropin_compare_func(const void *_a, const void *_b) { + const DropinSymlink *a = _a; + const DropinSymlink *b = _b; + + if (a->unit != b->unit) + return -1; + + if (a->dependency != b->dependency) + return 1; + + return 0; +} + +static const struct hash_ops dropin_hash_ops = { + .hash = dropin_hash_func, + .compare = dropin_compare_func, +}; + +/* Return true only if path is a symlink to /dev/null. Anything + * else (specially dangling symlinks) should return false. */ +static bool symlink_to_devnull(const char *path) { + _cleanup_free_ char *target = NULL; + + assert(path); + + if (readlink_malloc(path, &target) < 0) + return false; + + return path_equal(target, "/dev/null"); +} + +int unit_load_reserve_deferred_dropin_dependencies(Unit *u) { + DropinSymlink *ds; + Iterator i; + int r = 0; + + SET_FOREACH(ds, u->deferred_dropin_dependencies, i) { + r = unit_reserve_dependency(u, ds->dependency, ds->unit, true); + if (r < 0) + break; + } + return r; +} + +static void unit_load_solve_dropin_dependencies(Unit *u) { + DropinSymlink *ds1, *ds2; + Iterator i, j; + + assert(u); + + /* If one of the dropin unit has not been loaded yet, + * we defer the loading of the dropins for 'u' further + * until the last dropin is loaded. */ + SET_FOREACH(ds1, u->deferred_dropin_dependencies, i) + if (ds1->unit->load_state == UNIT_STUB) + return; + + SET_FOREACH(ds1, u->deferred_dropin_dependencies, i) { + DropinSymlink best; + + if (!ds1->unit) + continue; + + best = *ds1; /* struct copy */ + best.unit = unit_follow_merge(best.unit); + ds1->unit = NULL; /* mark this dropin as processed. */ + + if (best.unit->load_state == UNIT_LOADED) + SET_FOREACH(ds2, u->deferred_dropin_dependencies, j) { + + if (!ds2->unit) + continue; + + if (best.dependency != ds2->dependency) + continue; + + if (best.unit != unit_follow_merge(ds2->unit)) + continue; + + /* Same dropin on the same depth is unlikely but if + * it happens and one of them is a mask then gives + * priority to the mask. */ + if (best.depth > ds2->depth || + ((best.depth == ds2->depth && ds2->is_mask))) + best = *ds2; + + /* mark this dropin as processed. */ + ds2->unit = NULL; + } + + if (best.is_mask) + continue; + + /* This can't fail as we did a reservation. */ + assert(unit_add_dependency(u, best.dependency, best.unit, true) >= 0); + } + + u->deferred_dropin_dependencies = set_free_free(u->deferred_dropin_dependencies); +} + +void unit_load_deferred_dropin_dependencies(Unit *u) { + Iterator i; + Unit *o; + + /* Let's find out if we're involved in the deferred dep of + * another unit and the later is waiting for us to solve the + * dropin dep, IOW if we're the last loaded deferred dep. */ + SET_FOREACH(o, u->deferred_dropin_units, i) + unit_load_solve_dropin_dependencies(o); + + /* We shouldn't be called again for this unit as it is going + * to be in loaded state, so 'deferred_dropin_units' set + * shouldn't be needed anymore. */ + u->deferred_dropin_units = set_free(u->deferred_dropin_units); +} + static int add_dependency_consumer( UnitDependency dependency, const char *entry, - const char* filepath, + const char *filepath, void *arg) { - Unit *u = arg; + _cleanup_free_ DropinSymlink *ds = NULL; + DependencyConsumerData *data = arg; + Unit *u, *other; int r; - assert(u); + assert(data); + + u = data->unit; + + /* Here's the deal: the dependency unit needs to be loaded so we can + * deal with aliases (if any) properly. But as we're already in the + * loading path this cannot be achieved here (if it's needed)... + * Therefore we defer the handling of the dropins during their own + * unit loading. */ + other = manager_get_unit(u->manager, entry); + if (!other) { + /* Force 'filepath' to NULL because the symlink is not supposed + * to define any aliases and a symlink to /dev/null indicates + * a dropin mask, not that the unit should be masked. */ + r = manager_load_unit_prepare(u->manager, entry, NULL, NULL, &other); + if (r < 0) + return r; + } + + r = set_ensure_allocated(&u->deferred_dropin_dependencies, &dropin_hash_ops); + if (r < 0) + return r; - r = unit_add_dependency_by_name(u, dependency, entry, filepath, true); + r = set_ensure_allocated(&other->deferred_dropin_units, NULL); if (r < 0) - log_error_errno(r, "Cannot add dependency %s to %s, ignoring: %m", entry, u->id); + return r; + + ds = new0(DropinSymlink, 1); + if (!ds) + return -ENOMEM; + ds->unit = other; + ds->depth = data->depth; + ds->dependency = dependency; + ds->is_mask = symlink_to_devnull(filepath); + + r = set_put(u->deferred_dropin_dependencies, ds); + if (r <= 0) + /* r == 0 means we already registered the same dependency + * for 'u' but with a higher priority (lower depth). */ + return r; + + r = set_put(other->deferred_dropin_units, u); + if (r < 0) { + set_remove(u->deferred_dropin_dependencies, ds); + return r; + } + + ds = NULL; return 0; } int unit_load_dropin(Unit *u) { + DependencyConsumerData data = { .unit = u, .depth = 0 }; _cleanup_strv_free_ char **l = NULL; Iterator i; - char *t, **f; + char **f, **p; int r; assert(u); /* Load dependencies from supplementary drop-in directories */ - SET_FOREACH(t, u->names, i) { - char **p; + STRV_FOREACH(p, u->manager->lookup_paths.search_path) { + char *t; - STRV_FOREACH(p, u->manager->lookup_paths.search_path) { - unit_file_process_dir(u->manager->unit_path_cache, *p, t, ".wants", UNIT_WANTS, - add_dependency_consumer, u, NULL); - unit_file_process_dir(u->manager->unit_path_cache, *p, t, ".requires", UNIT_REQUIRES, - add_dependency_consumer, u, NULL); + SET_FOREACH(t, u->names, i) { + unit_file_process_dir(u->manager->unit_path_cache, *p, t, ".wants", + UNIT_WANTS, + add_dependency_consumer, &data, NULL); + unit_file_process_dir(u->manager->unit_path_cache, *p, t, ".requires", + UNIT_REQUIRES, + add_dependency_consumer, &data, NULL); } + data.depth++; } + /* Try to load the deferred dependencies now just in + * case all dropins have been already loaded. */ + r = unit_load_reserve_deferred_dropin_dependencies(u); + if (r < 0) + return r; + unit_load_solve_dropin_dependencies(u); + r = unit_find_dropin_paths(u, &l); if (r <= 0) return 0; diff --git a/src/core/load-dropin.h b/src/core/load-dropin.h index 942d26724ed35..5ec91db6524eb 100644 --- a/src/core/load-dropin.h +++ b/src/core/load-dropin.h @@ -32,3 +32,5 @@ static inline int unit_find_dropin_paths(Unit *u, char ***paths) { } int unit_load_dropin(Unit *u); +void unit_load_deferred_dropin_dependencies(Unit *u); +int unit_load_reserve_deferred_dropin_dependencies(Unit *u); diff --git a/src/core/unit.c b/src/core/unit.c index cb65dfceab76c..f3aaf433c027f 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -582,6 +582,9 @@ void unit_free(Unit *u) { (void) manager_update_failed_units(u->manager, u, false); set_remove(u->manager->startup_units, u); + set_free_free(u->deferred_dropin_dependencies); + set_free(u->deferred_dropin_units); + free(u->description); strv_free(u->documentation); free(u->fragment_path); @@ -1305,6 +1308,9 @@ int unit_load(Unit *u) { goto fail; } + /* This can't fail. */ + unit_load_deferred_dropin_dependencies(u); + if (u->load_state == UNIT_LOADED) { r = unit_add_target_dependencies(u); @@ -1330,6 +1336,12 @@ int unit_load(Unit *u) { } unit_update_cgroup_members_masks(u); + + /* This must be kept after adding all dependencies + * so our reservations will be preserved. */ + r = unit_load_reserve_deferred_dropin_dependencies(u); + if (r < 0) + goto fail; } assert((u->load_state != UNIT_MERGED) == !u->merged_into); @@ -1342,6 +1354,7 @@ int unit_load(Unit *u) { fail: u->load_state = u->load_state == UNIT_STUB ? UNIT_NOT_FOUND : UNIT_ERROR; u->load_error = r; + unit_load_deferred_dropin_dependencies(u); unit_add_to_dbus_queue(u); unit_add_to_gc_queue(u); diff --git a/src/core/unit.h b/src/core/unit.h index 05fc33dde0577..1176bc1dca120 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -167,6 +167,9 @@ struct Unit { * process SIGCHLD for */ Set *pids; + Set *deferred_dropin_dependencies; + Set *deferred_dropin_units; + /* Used in sigchld event invocation to avoid repeat events being invoked */ uint64_t sigchldgen;