From 19d91eef7f15b654cd96ad5350385e535fab9e2a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Tue, 16 Oct 2018 13:28:39 +0200 Subject: [PATCH] core: define "exit" and "exit-force" actions for user units and only accept that We would accept e.g. FailureAction=reboot-force in user units and then do an exit in the user manager. Let's be stricter, and define "exit"/"exit-force" as the only supported actions in user units. v2: - rename 'exit' to 'exit-force' and add new 'exit' - add test for the parsing function (cherry picked from commit 54fcb6192c618726d11404b24b1a1e9ec3169ee1) Related: #1860899 --- TODO | 4 +++ man/systemd.unit.xml | 26 +++++++++------- src/core/dbus-unit.c | 37 ++++++++++++++++++++++- src/core/emergency-action.c | 47 ++++++++++++++++++++++------- src/core/emergency-action.h | 5 ++++ src/core/load-fragment.c | 42 +++++++++++++++++++++++++- src/test/meson.build | 5 ++++ src/test/test-emergency-action.c | 51 ++++++++++++++++++++++++++++++++ 8 files changed, 195 insertions(+), 22 deletions(-) create mode 100644 src/test/test-emergency-action.c diff --git a/TODO b/TODO index 3100e067d6..0705b6b08e 100644 --- a/TODO +++ b/TODO @@ -4,6 +4,10 @@ Bugfixes: * copy.c: set the right chattrs before copying files and others after +* Many manager configuration settings that are only applicable to user + manager or system manager can be always set. It would be better to reject + them when parsing config. + External: * Fedora: add an rpmlint check that verifies that all unit files in the RPM are listed in %systemd_post macros. diff --git a/man/systemd.unit.xml b/man/systemd.unit.xml index 802db453a4..5772a6684e 100644 --- a/man/systemd.unit.xml +++ b/man/systemd.unit.xml @@ -877,18 +877,24 @@ FailureAction= SuccessAction= - Configure the action to take when the unit stops and enters a failed state or inactive - state. Takes one of , , , - , , or - . If is set, no action will be triggered. - causes a reboot following the normal shutdown procedure (i.e. equivalent to - systemctl reboot). causes a forced reboot which will - terminate all processes forcibly but should cause no dirty file systems on reboot (i.e. equivalent to - systemctl reboot -f) and causes immediate execution of the + Configure the action to take when the unit stops and enters a failed state or inactive state. + Takes one of , , , + , , , + , , and . In system mode, + all options except and are allowed. In user mode, only + , , and are allowed. Both options default + to . + + If is set, no action will be triggered. causes a reboot + following the normal shutdown procedure (i.e. equivalent to systemctl reboot). + causes a forced reboot which will terminate all processes forcibly but should + cause no dirty file systems on reboot (i.e. equivalent to systemctl reboot -f) and + causes immediate execution of the reboot2 system call, which might result in data loss. Similarly, , , - have the effect of powering down the system with similar semantics. Both - options default to . + have the effect of powering down the system with similar + semantics. causes the user manager to exit following the normal shutdown procedure, and + causes it terminate without shutting down services. diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 549a166abc..e7ea9db3ac 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1564,8 +1564,43 @@ static int bus_unit_set_live_property( return 0; } +static int bus_set_transient_emergency_action( + Unit *u, + const char *name, + EmergencyAction *p, + sd_bus_message *message, + UnitWriteFlags flags, + sd_bus_error *error) { + + const char *s; + EmergencyAction v; + int r; + bool system; + + assert(p); + + r = sd_bus_message_read(message, "s", &s); + if (r < 0) + return r; + + system = MANAGER_IS_SYSTEM(u->manager); + r = parse_emergency_action(s, system, &v); + if (v < 0) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + v == -EOPNOTSUPP ? "EmergencyAction setting invalid for manager type: %s" + : "Invalid %s setting: %s", + name, s); + + if (!UNIT_WRITE_FLAGS_NOOP(flags)) { + *p = v; + unit_write_settingf(u, flags, name, + "%s=%s", name, s); + } + + return 1; +} + static BUS_DEFINE_SET_TRANSIENT_PARSE(collect_mode, CollectMode, collect_mode_from_string); -static BUS_DEFINE_SET_TRANSIENT_PARSE(emergency_action, EmergencyAction, emergency_action_from_string); static BUS_DEFINE_SET_TRANSIENT_PARSE(job_mode, JobMode, job_mode_from_string); static int bus_set_transient_conditions( diff --git a/src/core/emergency-action.c b/src/core/emergency-action.c index 766a3b4d2b..00f5996317 100644 --- a/src/core/emergency-action.c +++ b/src/core/emergency-action.c @@ -39,15 +39,6 @@ int emergency_action( return -ECANCELED; } - if (!MANAGER_IS_SYSTEM(m)) { - /* Downgrade all options to simply exiting if we run - * in user mode */ - - log_warning("Exiting: %s", reason); - m->exit_code = MANAGER_EXIT; - return -ECANCELED; - } - switch (action) { case EMERGENCY_ACTION_REBOOT: @@ -80,11 +71,26 @@ int emergency_action( (void) reboot(RB_AUTOBOOT); break; + case EMERGENCY_ACTION_EXIT: + assert(MANAGER_IS_USER(m)); + + log_and_status(m, "Exiting", reason); + + (void) manager_add_job_by_name_and_warn(m, JOB_START, SPECIAL_EXIT_TARGET, JOB_REPLACE_IRREVERSIBLY, NULL, NULL); + break; + case EMERGENCY_ACTION_POWEROFF: log_and_status(m, "Powering off", reason); (void) manager_add_job_by_name_and_warn(m, JOB_START, SPECIAL_POWEROFF_TARGET, JOB_REPLACE_IRREVERSIBLY, NULL, NULL); break; + case EMERGENCY_ACTION_EXIT_FORCE: + assert(MANAGER_IS_USER(m)); + + log_and_status(m, "Exiting immediately", reason); + m->exit_code = MANAGER_EXIT; + break; + case EMERGENCY_ACTION_POWEROFF_FORCE: log_and_status(m, "Forcibly powering off", reason); m->exit_code = MANAGER_POWEROFF; @@ -113,6 +119,27 @@ static const char* const emergency_action_table[_EMERGENCY_ACTION_MAX] = { [EMERGENCY_ACTION_REBOOT_IMMEDIATE] = "reboot-immediate", [EMERGENCY_ACTION_POWEROFF] = "poweroff", [EMERGENCY_ACTION_POWEROFF_FORCE] = "poweroff-force", - [EMERGENCY_ACTION_POWEROFF_IMMEDIATE] = "poweroff-immediate" + [EMERGENCY_ACTION_POWEROFF_IMMEDIATE] = "poweroff-immediate", + [EMERGENCY_ACTION_EXIT] = "exit", + [EMERGENCY_ACTION_EXIT_FORCE] = "exit-force", }; DEFINE_STRING_TABLE_LOOKUP(emergency_action, EmergencyAction); + +int parse_emergency_action( + const char *value, + bool system, + EmergencyAction *ret) { + + EmergencyAction x; + + x = emergency_action_from_string(value); + if (x < 0) + return -EINVAL; + + if ((system && x >= _EMERGENCY_ACTION_FIRST_USER_ACTION) || + (!system && x != EMERGENCY_ACTION_NONE && x < _EMERGENCY_ACTION_FIRST_USER_ACTION)) + return -EOPNOTSUPP; + + *ret = x; + return 0; +} diff --git a/src/core/emergency-action.h b/src/core/emergency-action.h index 61791f176f..646ccc4e6b 100644 --- a/src/core/emergency-action.h +++ b/src/core/emergency-action.h @@ -13,6 +13,9 @@ typedef enum EmergencyAction { EMERGENCY_ACTION_POWEROFF, EMERGENCY_ACTION_POWEROFF_FORCE, EMERGENCY_ACTION_POWEROFF_IMMEDIATE, + EMERGENCY_ACTION_EXIT, + _EMERGENCY_ACTION_FIRST_USER_ACTION = EMERGENCY_ACTION_EXIT, + EMERGENCY_ACTION_EXIT_FORCE, _EMERGENCY_ACTION_MAX, _EMERGENCY_ACTION_INVALID = -1 } EmergencyAction; @@ -24,3 +27,5 @@ int emergency_action(Manager *m, EmergencyAction action, const char *reboot_arg, const char* emergency_action_to_string(EmergencyAction i) _const_; EmergencyAction emergency_action_from_string(const char *s) _pure_; + +int parse_emergency_action(const char *value, bool system, EmergencyAction *ret); diff --git a/src/core/load-fragment.c b/src/core/load-fragment.c index e0d7b8f7f8..c102ffb9f0 100644 --- a/src/core/load-fragment.c +++ b/src/core/load-fragment.c @@ -77,7 +77,6 @@ DEFINE_CONFIG_PARSE(config_parse_socket_protocol, supported_socket_protocol_from DEFINE_CONFIG_PARSE(config_parse_exec_secure_bits, secure_bits_from_string, "Failed to parse secure bits"); DEFINE_CONFIG_PARSE_ENUM(config_parse_collect_mode, collect_mode, CollectMode, "Failed to parse garbage collection mode"); DEFINE_CONFIG_PARSE_ENUM(config_parse_device_policy, cgroup_device_policy, CGroupDevicePolicy, "Failed to parse device policy"); -DEFINE_CONFIG_PARSE_ENUM(config_parse_emergency_action, emergency_action, EmergencyAction, "Failed to parse failure action specifier"); DEFINE_CONFIG_PARSE_ENUM(config_parse_exec_keyring_mode, exec_keyring_mode, ExecKeyringMode, "Failed to parse keyring mode"); DEFINE_CONFIG_PARSE_ENUM(config_parse_exec_utmp_mode, exec_utmp_mode, ExecUtmpMode, "Failed to parse utmp mode"); DEFINE_CONFIG_PARSE_ENUM(config_parse_job_mode, job_mode, JobMode, "Failed to parse job mode"); @@ -4253,6 +4252,47 @@ int config_parse_job_running_timeout_sec( return 0; } +int config_parse_emergency_action( + const char* unit, + const char *filename, + unsigned line, + const char *section, + unsigned section_line, + const char *lvalue, + int ltype, + const char *rvalue, + void *data, + void *userdata) { + + Manager *m = NULL; + EmergencyAction *x = data; + int r; + + assert(filename); + assert(lvalue); + assert(rvalue); + assert(data); + + if (unit) + m = ((Unit*) userdata)->manager; + else + m = data; + + r = parse_emergency_action(rvalue, MANAGER_IS_SYSTEM(m), x); + if (r < 0) { + if (r == -EOPNOTSUPP) + log_syntax(unit, LOG_ERR, filename, line, r, + "%s= specified as %s mode action, ignoring: %s", + lvalue, MANAGER_IS_SYSTEM(m) ? "user" : "system", rvalue); + else + log_syntax(unit, LOG_ERR, filename, line, r, + "Failed to parse %s=, ignoring: %s", lvalue, rvalue); + return 0; + } + + return 0; +} + #define FOLLOW_MAX 8 static int open_follow(char **filename, FILE **_f, Set *names, char **_final) { diff --git a/src/test/meson.build b/src/test/meson.build index 7b310d4ec7..40cf56d73d 100644 --- a/src/test/meson.build +++ b/src/test/meson.build @@ -65,6 +65,11 @@ tests += [ libshared], []], + [['src/test/test-emergency-action.c'], + [libcore, + libshared], + []], + [['src/test/test-job-type.c'], [libcore, libshared], diff --git a/src/test/test-emergency-action.c b/src/test/test-emergency-action.c new file mode 100644 index 0000000000..493b23227e --- /dev/null +++ b/src/test/test-emergency-action.c @@ -0,0 +1,51 @@ +/* SPDX-License-Identifier: LGPL-2.1+ */ + +#include "emergency-action.h" +#include "tests.h" + +static void test_parse_emergency_action(void) { + EmergencyAction x; + + log_info("/* %s */", __func__); + + assert_se(parse_emergency_action("none", false, &x) == 0); + assert_se(x == EMERGENCY_ACTION_NONE); + assert_se(parse_emergency_action("reboot", false, &x) == -EOPNOTSUPP); + assert_se(parse_emergency_action("reboot-force", false, &x) == -EOPNOTSUPP); + assert_se(parse_emergency_action("reboot-immediate", false, &x) == -EOPNOTSUPP); + assert_se(parse_emergency_action("poweroff", false, &x) == -EOPNOTSUPP); + assert_se(parse_emergency_action("poweroff-force", false, &x) == -EOPNOTSUPP); + assert_se(parse_emergency_action("poweroff-immediate", false, &x) == -EOPNOTSUPP); + assert_se(x == EMERGENCY_ACTION_NONE); + assert_se(parse_emergency_action("exit", false, &x) == 0); + assert_se(x == EMERGENCY_ACTION_EXIT); + assert_se(parse_emergency_action("exit-force", false, &x) == 0); + assert_se(x == EMERGENCY_ACTION_EXIT_FORCE); + assert_se(parse_emergency_action("exit-forcee", false, &x) == -EINVAL); + + assert_se(parse_emergency_action("none", true, &x) == 0); + assert_se(x == EMERGENCY_ACTION_NONE); + assert_se(parse_emergency_action("reboot", true, &x) == 0); + assert_se(x == EMERGENCY_ACTION_REBOOT); + assert_se(parse_emergency_action("reboot-force", true, &x) == 0); + assert_se(x == EMERGENCY_ACTION_REBOOT_FORCE); + assert_se(parse_emergency_action("reboot-immediate", true, &x) == 0); + assert_se(x == EMERGENCY_ACTION_REBOOT_IMMEDIATE); + assert_se(parse_emergency_action("poweroff", true, &x) == 0); + assert_se(x == EMERGENCY_ACTION_POWEROFF); + assert_se(parse_emergency_action("poweroff-force", true, &x) == 0); + assert_se(x == EMERGENCY_ACTION_POWEROFF_FORCE); + assert_se(parse_emergency_action("poweroff-immediate", true, &x) == 0); + assert_se(parse_emergency_action("exit", true, &x) == -EOPNOTSUPP); + assert_se(parse_emergency_action("exit-force", true, &x) == -EOPNOTSUPP); + assert_se(parse_emergency_action("exit-forcee", true, &x) == -EINVAL); + assert_se(x == EMERGENCY_ACTION_POWEROFF_IMMEDIATE); +} + +int main(int argc, char **argv) { + test_setup_logging(LOG_INFO); + + test_parse_emergency_action(); + + return EXIT_SUCCESS; +}