[systemd-devel] [PATCH] systemctl: allow enable/disable/is-enabled on symlink aliases
Andrey Borzenkov
arvidjaar at gmail.com
Sun Mar 20 06:50:24 PDT 2011
Currently handling of unit aliases (symlinks) is inconsistent
between systemd and systemctl. While systemd allows acces unit
under any alias, systemctl will refuse to process them unless
redirecting request to systemd.
Allow exactly one level of indirection to support unit aliases.
Symlink cannot point outside of directory (contain "/") and must
be valid unit name of the same type as original - i.e. either
both must be normal units, or both templates, or both template
instances with the same instance name.
Alias name is resolved and all operations are done on target unit.
This ensures that "systemctl is-enabled" returns the same result
for any alias. Also systemctl disable will correctly remove links
to aliases as well (link tagrgets were already fully resolved so
they always pointed to primary unit).
The change is backward compatible as systemctl refused to operate
on symlinks before, so any existing link is for primary name.
---
I am not sure whether such paranoid checks are needed. Systemd seems
to just follow any symlink it encounters and I do not know whether or
where it checks that all unit symlinks are really compatible (I presume
it happens as side effect of unit parsing). In this case patch could be
simplified to simply remove O_NOFOLLOW from open.
---
src/systemctl.c | 128 ++++++++++++++++++++++++++++++++++++++++--------------
src/unit-name.c | 31 +++++++++++++
src/unit-name.h | 1 +
3 files changed, 127 insertions(+), 33 deletions(-)
diff --git a/src/systemctl.c b/src/systemctl.c
index 5e34d03..d9d3295 100644
--- a/src/systemctl.c
+++ b/src/systemctl.c
@@ -3604,6 +3604,27 @@ fail:
return r;
}
+static int install_info_replace_name(InstallInfo *i, const char *name)
+{
+ assert(i);
+ assert(name);
+
+ if (!unit_name_is_valid_no_type(name, true)) {
+ log_warning("Symbolic link tagret %s is not valid unit name.", name);
+ return -EINVAL;
+ }
+
+ if (!unit_name_is_compatible(i->name, name)) {
+ log_warning("Unit type of symbolic link target %s does not match link %s.", name, i->name);
+ return -EINVAL;
+ }
+
+ free(i->name);
+ if (!(i->name = strdup(name)))
+ return -ENOMEM;
+ return 0;
+}
+
static int config_parse_also(
const char *filename,
unsigned line,
@@ -4020,6 +4041,75 @@ finish:
return r;
}
+static int resolve_unit_alias(InstallInfo *i, char *p, FILE **f)
+{
+ int fd;
+ char *original = NULL;
+ char *resolved = NULL;
+ char *target = NULL;
+ int r = 0;
+
+ if (!(original = path_make_absolute(i->name, p))) {
+ log_error("Out of memory");
+ r = -ENOMEM;
+ goto finish;
+ }
+
+ /* We support exactly one level of indirection from
+ * alias to primary unit
+ */
+ if ((fd = open(original, O_RDONLY|O_CLOEXEC|O_NOFOLLOW|O_NOCTTY)) >= 0)
+ if ((*f = fdopen(fd, "re"))) {
+ i->path = strdup(original);
+ goto finish;
+ }
+
+ if (errno == ELOOP) {
+ if ((r = readlink_malloc(original, &target)) < 0) {
+ log_error("Failed to read link target for %s: %m", original);
+ goto finish;
+ }
+
+ if (strchr(target, '/')) {
+ log_error("Link for %s leaves directory", original);
+ r = -EINVAL;
+ goto finish;
+ }
+
+ if ((r = install_info_replace_name(i, target)) < 0)
+ goto finish;
+
+ if (!(resolved = path_make_absolute(target, p))) {
+ log_error("Out of memory");
+ r = -ENOMEM;
+ goto finish;
+ }
+
+ if ((fd = open(resolved, O_RDONLY|O_CLOEXEC|O_NOFOLLOW|O_NOCTTY)) >= 0)
+ if ((*f = fdopen(fd, "re"))) {
+ i->path = strdup(resolved);
+ goto finish;
+ }
+
+ if (errno == ELOOP) {
+ log_error("Too many levels of indirection for %s", original);
+ r = -errno;
+ goto finish;
+ }
+ }
+
+ if (errno != ENOENT) {
+ log_error("Failed to open %s: %m", original);
+ r = -errno;
+ }
+
+finish:
+ free(original);
+ free(resolved);
+ free(target);
+ return r;
+}
+
static int install_info_apply(const char *verb, LookupPaths *paths, InstallInfo *i, const char *config_path) {
const ConfigItem items[] = {
@@ -4031,41 +4121,15 @@ static int install_info_apply(const char *verb, LookupPaths *paths, InstallInfo
};
char **p;
- char *filename = NULL;
FILE *f = NULL;
int r;
assert(paths);
assert(i);
- STRV_FOREACH(p, paths->unit_path) {
- int fd;
-
- if (!(filename = path_make_absolute(i->name, *p))) {
- log_error("Out of memory");
- return -ENOMEM;
- }
-
- /* Ensure that we don't follow symlinks */
- if ((fd = open(filename, O_RDONLY|O_CLOEXEC|O_NOFOLLOW|O_NOCTTY)) >= 0)
- if ((f = fdopen(fd, "re")))
- break;
-
- if (errno == ELOOP) {
- log_error("Refusing to operate on symlinks, please pass unit names or absolute paths to unit files.");
- free(filename);
- return -errno;
- }
-
- if (errno != ENOENT) {
- log_error("Failed to open %s: %m", filename);
- free(filename);
- return -errno;
- }
-
- free(filename);
- filename = NULL;
- }
+ STRV_FOREACH(p, paths->unit_path)
+ if ((r = resolve_unit_alias(i, *p, &f)) < 0)
+ return r;
if (!f) {
#if (defined(TARGET_FEDORA) || defined(TARGET_MANDRIVA)) && defined (HAVE_SYSV_COMPAT)
@@ -4139,9 +4203,7 @@ static int install_info_apply(const char *verb, LookupPaths *paths, InstallInfo
return -ENOENT;
}
- i->path = filename;
-
- if ((r = config_parse(filename, f, NULL, items, true, i)) < 0) {
+ if ((r = config_parse(i->path, f, NULL, items, true, i)) < 0) {
fclose(f);
return r;
}
@@ -4157,7 +4219,7 @@ static int install_info_apply(const char *verb, LookupPaths *paths, InstallInfo
if ((r = install_info_symlink_wants(verb, i, config_path)) != 0)
return r;
- if ((r = mark_symlink_for_removal(filename)) < 0)
+ if ((r = mark_symlink_for_removal(i->path)) < 0)
return r;
if ((r = remove_marked_symlinks(config_path)) < 0)
diff --git a/src/unit-name.c b/src/unit-name.c
index be4e73e..c1e1a8c 100644
--- a/src/unit-name.c
+++ b/src/unit-name.c
@@ -70,6 +70,37 @@ bool unit_name_is_valid_no_type(const char *n, bool template_ok) {
return true;
}
+/*
+ * Verify that (symlink) alias `a' is compatible with name `n', i.e.
+ * - both have the same suffix
+ * - both are standard units or templates
+ * - if instantiated templates, both have the same unit name
+ */
+bool unit_name_is_compatible(const char *n, const char *a)
+{
+ char *sn, *sa;
+ char *tn, *ta;
+
+ assert(n);
+ assert(a);
+
+ sn = strchr(n, '.');
+ sa = strchr(a, '.');
+ if (!sn || !sa)
+ return false;
+ if (!streq(sn, sa))
+ return false;
+
+ tn = strchr(n, '@');
+ ta = strchr(a, '@');
+ if (!tn && !ta)
+ return true;
+ if (tn && ta && (sn - tn) == (sa - ta) && strneq(tn, ta, sn - tn))
+ return true;
+
+ return false;
+}
+
bool unit_instance_is_valid(const char *i) {
assert(i);
diff --git a/src/unit-name.h b/src/unit-name.h
index e369910..ba1cc63 100644
--- a/src/unit-name.h
+++ b/src/unit-name.h
@@ -31,6 +31,7 @@ char* unit_name_to_prefix(const char *n);
char* unit_name_to_prefix_and_instance(const char *n);
bool unit_name_is_valid_no_type(const char *n, bool template_ok);
+bool unit_name_is_compatible(const char *n, const char *a);
bool unit_prefix_is_valid(const char *p);
bool unit_instance_is_valid(const char *i);
--
tg: (8c94438..) u/systemctl-symlink (depends on: origin/master)
More information about the systemd-devel
mailing list