From 71fad6751434f06485a744d41be2d807303c1184 Mon Sep 17 00:00:00 2001 From: Lennart Poettering Date: Wed, 6 Oct 2010 02:33:40 +0200 Subject: [PATCH] systemctl: require correctly formed unit names when enabling units --- Makefile.am | 3 ++- fixme | 9 ++++----- src/systemctl.c | 38 +++++++++++++++----------------------- src/unit-name.c | 31 ++++++------------------------- src/unit-name.h | 6 +++--- src/unit.c | 22 ++++++++++++++++++++++ src/unit.h | 4 +++- 7 files changed, 55 insertions(+), 58 deletions(-) diff --git a/Makefile.am b/Makefile.am index c06f1ecb..4307db1a 100644 --- a/Makefile.am +++ b/Makefile.am @@ -700,7 +700,8 @@ systemctl_SOURCES = \ src/sd-daemon.c \ src/cgroup-show.c \ src/cgroup-util.c \ - src/exit-status.c + src/exit-status.c \ + src/unit-name.c systemctl_CFLAGS = \ $(AM_CFLAGS) \ diff --git a/fixme b/fixme index fd756312..c4582d3d 100644 --- a/fixme +++ b/fixme @@ -4,9 +4,6 @@ v11: * verify ordering of random-seed-load and base.target! -* systemctl enable /lib/systemd/system/?*.service - creates wrong subdirs in /etc/ssytemd/system/ which prevent further bootup - later: * do not throw error when .service file is linked to /dev/null @@ -97,8 +94,6 @@ later: * document locale.conf, vconsole.conf and possibly the tempfiles.d and modules-load.d mechanism. -* when /proc/self/mountinfo is not parsable, proceed with next line - * beefed up tmpwatch that reads tmpfiles.d * /lib/systemd/system/systemd-readahead-replay.service @@ -109,6 +104,10 @@ later: * Restart=on-failure and Restart=on-abort +* kill sessions on shutdown + +* when processes remain in a service even though the start command failed enter active + External: * place /etc/inittab with explaining blurb. diff --git a/src/systemctl.c b/src/systemctl.c index 1f2b43be..45249aaa 100644 --- a/src/systemctl.c +++ b/src/systemctl.c @@ -53,6 +53,7 @@ #include "exit-status.h" #include "bus-errors.h" #include "build.h" +#include "unit-name.h" static const char *arg_type = NULL; static char **arg_property = NULL; @@ -3237,28 +3238,16 @@ static void install_info_hashmap_free(Hashmap *m) { hashmap_free(m); } -static bool unit_name_valid(const char *name) { - - /* This is a minimal version of unit_name_valid() from - * unit-name.c */ - - if (!*name) - return false; - - if (ignore_file(name)) - return false; - - return true; -} - static int install_info_add(const char *name) { InstallInfo *i; int r; assert(will_install); - if (!unit_name_valid(name)) + if (!unit_name_is_valid_no_type(name)) { + log_warning("Unit name %s is not a valid unit name.", name); return -EINVAL; + } if (hashmap_get(have_installed, name) || hashmap_get(will_install, name)) @@ -3310,11 +3299,13 @@ static int config_parse_also( if (!(n = strndup(w, l))) return -ENOMEM; - r = install_info_add(n); - free(n); - - if (r < 0) + if ((r = install_info_add(n)) < 0) { + log_warning("Cannot install unit %s: %s", n, strerror(-r)); + free(n); return r; + } + + free(n); } return 0; @@ -3639,7 +3630,7 @@ static int install_info_symlink_alias(const char *verb, InstallInfo *i, const ch STRV_FOREACH(s, i->aliases) { - if (!unit_name_valid(*s)) { + if (!unit_name_is_valid_no_type(*s)) { log_error("Invalid name %s.", *s); r = -EINVAL; goto finish; @@ -3658,7 +3649,6 @@ static int install_info_symlink_alias(const char *verb, InstallInfo *i, const ch if (streq(verb, "disable")) rmdir_parents(alias_path, config_path); } - r = 0; finish: @@ -3677,7 +3667,7 @@ static int install_info_symlink_wants(const char *verb, InstallInfo *i, const ch assert(config_path); STRV_FOREACH(s, i->wanted_by) { - if (!unit_name_valid(*s)) { + if (!unit_name_is_valid_no_type(*s)) { log_error("Invalid name %s.", *s); r = -EINVAL; goto finish; @@ -3840,8 +3830,10 @@ static int enable_unit(DBusConnection *bus, char **args, unsigned n) { } for (j = 1; j < n; j++) - if ((r = install_info_add(args[j])) < 0) + if ((r = install_info_add(args[j])) < 0) { + log_warning("Cannot install unit %s: %s", args[j], strerror(-r)); goto finish; + } while ((i = hashmap_first(will_install))) { int q; diff --git a/src/unit-name.c b/src/unit-name.c index 868d13e4..0e86b554 100644 --- a/src/unit-name.c +++ b/src/unit-name.c @@ -21,8 +21,9 @@ #include #include +#include -#include "unit.h" +#include "util.h" #include "unit-name.h" #define VALID_CHARS \ @@ -31,20 +32,7 @@ "ABCDEFGHIJKLMNOPQRSTUVWXYZ" \ ":-_.\\" -UnitType unit_name_to_type(const char *n) { - UnitType t; - - assert(n); - - for (t = 0; t < _UNIT_TYPE_MAX; t++) - if (endswith(n, unit_vtable[t]->suffix)) - return t; - - return _UNIT_TYPE_INVALID; -} - -bool unit_name_is_valid(const char *n) { - UnitType t; +bool unit_name_is_valid_no_type(const char *n) { const char *e, *i, *at; /* Valid formats: @@ -58,13 +46,8 @@ bool unit_name_is_valid(const char *n) { if (strlen(n) >= UNIT_NAME_MAX) return false; - t = unit_name_to_type(n); - if (t < 0 || t >= _UNIT_TYPE_MAX) - return false; - - assert_se(e = strrchr(n, '.')); - - if (e == n) + e = strrchr(n, '.'); + if (!e || e == n) return false; for (i = n, at = NULL; i < e; i++) { @@ -167,7 +150,7 @@ char *unit_name_change_suffix(const char *n, const char *suffix) { size_t a, b; assert(n); - assert(unit_name_is_valid(n)); + assert(unit_name_is_valid_no_type(n)); assert(suffix); assert_se(e = strrchr(n, '.')); @@ -190,7 +173,6 @@ char *unit_name_build(const char *prefix, const char *instance, const char *suff assert(unit_prefix_is_valid(prefix)); assert(!instance || unit_instance_is_valid(instance)); assert(suffix); - assert(unit_name_to_type(suffix) >= 0); if (!instance) return strappend(prefix, suffix); @@ -226,7 +208,6 @@ char *unit_name_build_escape(const char *prefix, const char *instance, const cha assert(prefix); assert(suffix); - assert(unit_name_to_type(suffix) >= 0); /* Takes a arbitrary string for prefix and instance plus a * suffix and makes a nice string suitable as unit name of it, diff --git a/src/unit-name.h b/src/unit-name.h index 3ad25bac..a752f3a2 100644 --- a/src/unit-name.h +++ b/src/unit-name.h @@ -22,15 +22,15 @@ along with systemd; If not, see . ***/ -#include "unit.h" +#include -UnitType unit_name_to_type(const char *n); +#define UNIT_NAME_MAX 256 int unit_name_to_instance(const char *n, char **instance); char* unit_name_to_prefix(const char *n); char* unit_name_to_prefix_and_instance(const char *n); -bool unit_name_is_valid(const char *n); +bool unit_name_is_valid_no_type(const char *n); bool unit_prefix_is_valid(const char *p); bool unit_instance_is_valid(const char *i); diff --git a/src/unit.c b/src/unit.c index fefa0eb5..71ef2a70 100644 --- a/src/unit.c +++ b/src/unit.c @@ -2207,6 +2207,28 @@ bool unit_pending_active(Unit *u) { return false; } +UnitType unit_name_to_type(const char *n) { + UnitType t; + + assert(n); + + for (t = 0; t < _UNIT_TYPE_MAX; t++) + if (endswith(n, unit_vtable[t]->suffix)) + return t; + + return _UNIT_TYPE_INVALID; +} + +bool unit_name_is_valid(const char *n) { + UnitType t; + + t = unit_name_to_type(n); + if (t < 0 || t >= _UNIT_TYPE_MAX) + return false; + + return unit_name_is_valid_no_type(n); +} + static const char* const unit_load_state_table[_UNIT_LOAD_STATE_MAX] = { [UNIT_STUB] = "stub", [UNIT_LOADED] = "loaded", diff --git a/src/unit.h b/src/unit.h index 47a1eb63..afae5804 100644 --- a/src/unit.h +++ b/src/unit.h @@ -39,7 +39,6 @@ typedef enum UnitDependency UnitDependency; #include "socket-util.h" #include "execute.h" -#define UNIT_NAME_MAX 256 #define DEFAULT_TIMEOUT_USEC (60*USEC_PER_SEC) #define DEFAULT_RESTART_USEC (100*USEC_PER_MSEC) @@ -504,6 +503,9 @@ bool unit_pending_active(Unit *u); int unit_add_default_target_dependency(Unit *u, Unit *target); +UnitType unit_name_to_type(const char *n); +bool unit_name_is_valid(const char *n); + const char *unit_load_state_to_string(UnitLoadState i); UnitLoadState unit_load_state_from_string(const char *s); -- 2.39.5