From acafd7d8a6297fe40960d12dd86a974a53072cfb Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Fri, 19 Jul 2019 11:54:15 +0100 Subject: [PATCH 1/2] core: add KExecWatchdogSec option Rather than always enabling the shutdown WD on kexec, which might be dangerous in case the kernel driver and/or the hardware implementation does not reset the wd on kexec, add a new timer, disabled by default, to let users optionally enable the shutdown WD on kexec separately from the runtime and reboot ones. Advise in the documentation to also use the runtime WD in conjunction with it. Fixes: a637d0f9ecbe ("core: set shutdown watchdog on kexec too") --- man/systemd-system.conf.xml | 9 ++++++++- src/core/dbus-manager.c | 1 + src/core/main.c | 21 +++++++++++++++------ src/core/manager.h | 1 + src/core/system.conf.in | 1 + test/fuzz/fuzz-unit-file/directives.service | 1 + 6 files changed, 27 insertions(+), 7 deletions(-) diff --git a/man/systemd-system.conf.xml b/man/systemd-system.conf.xml index 5b80479a0a1..4dd4bf0ca8b 100644 --- a/man/systemd-system.conf.xml +++ b/man/systemd-system.conf.xml @@ -128,6 +128,7 @@ RuntimeWatchdogSec= ShutdownWatchdogSec= + KExecWatchdogSec= Configure the hardware watchdog at runtime and at reboot. Takes a timeout value in seconds (or in other time units if suffixed with ms, min, h, @@ -149,7 +150,13 @@ phase of system shutdown, configure JobTimeoutSec= and JobTimeoutAction= in the [Unit] section of the shutdown.target unit. By default RuntimeWatchdogSec= defaults to 0 (off), and ShutdownWatchdogSec= to - 10min. These settings have no effect if a hardware watchdog is not available. + 10min. KExecWatchdogSec= may be used to additionally enable the watchdog when kexec + is being executed rather than when rebooting. Note that if the kernel does not reset the watchdog on kexec (depending + on the specific hardware and/or driver), in this case the watchdog might not get disabled after kexec succeeds + and thus the system might get rebooted, unless RuntimeWatchdogSec= is also enabled at the same time. + For this reason it is recommended to enable KExecWatchdogSec= only if + RuntimeWatchdogSec= is also enabled. + These settings have no effect if a hardware watchdog is not available. diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 9fb3ed516a9..e70c1e132a7 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -2410,6 +2410,7 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_PROPERTY("DefaultStandardError", "s", bus_property_get_exec_output, offsetof(Manager, default_std_output), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_WRITABLE_PROPERTY("RuntimeWatchdogUSec", "t", bus_property_get_usec, property_set_runtime_watchdog, offsetof(Manager, runtime_watchdog), 0), SD_BUS_WRITABLE_PROPERTY("ShutdownWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, shutdown_watchdog), 0), + SD_BUS_WRITABLE_PROPERTY("KExecWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, kexec_watchdog), 0), SD_BUS_WRITABLE_PROPERTY("ServiceWatchdogs", "b", bus_property_get_bool, bus_property_set_bool, offsetof(Manager, service_watchdogs), 0), SD_BUS_PROPERTY("ControlGroup", "s", NULL, offsetof(Manager, cgroup_root), 0), SD_BUS_PROPERTY("SystemState", "s", property_get_system_state, 0, 0), diff --git a/src/core/main.c b/src/core/main.c index 8de3d4753d6..ccfa22c814d 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -122,6 +122,7 @@ static usec_t arg_default_start_limit_interval; static unsigned arg_default_start_limit_burst; static usec_t arg_runtime_watchdog; static usec_t arg_shutdown_watchdog; +static usec_t arg_kexec_watchdog; static char *arg_early_core_pattern; static char *arg_watchdog_device; static char **arg_default_environment; @@ -555,6 +556,7 @@ static int parse_config_file(void) { { "Manager", "JoinControllers", config_parse_warn_compat, DISABLED_CONFIGURATION, NULL }, { "Manager", "RuntimeWatchdogSec", config_parse_sec, 0, &arg_runtime_watchdog }, { "Manager", "ShutdownWatchdogSec", config_parse_sec, 0, &arg_shutdown_watchdog }, + { "Manager", "KExecWatchdogSec", config_parse_sec, 0, &arg_kexec_watchdog }, { "Manager", "WatchdogDevice", config_parse_path, 0, &arg_watchdog_device }, { "Manager", "CapabilityBoundingSet", config_parse_capability_set, 0, &arg_capability_bounding_set }, { "Manager", "NoNewPrivileges", config_parse_bool, 0, &arg_no_new_privs }, @@ -674,6 +676,7 @@ static void set_manager_settings(Manager *m) { m->service_watchdogs = arg_service_watchdogs; m->runtime_watchdog = arg_runtime_watchdog; m->shutdown_watchdog = arg_shutdown_watchdog; + m->kexec_watchdog = arg_kexec_watchdog; m->cad_burst_action = arg_cad_burst_action; manager_set_show_status(m, arg_show_status); @@ -1356,6 +1359,7 @@ static int become_shutdown( _cleanup_strv_free_ char **env_block = NULL; size_t pos = 7; int r; + usec_t watchdog_timer = 0; assert(shutdown_verb); assert(!command_line[pos]); @@ -1396,20 +1400,23 @@ static int become_shutdown( assert(pos < ELEMENTSOF(command_line)); - if (STR_IN_SET(shutdown_verb, "reboot", "kexec") && - arg_shutdown_watchdog > 0 && - arg_shutdown_watchdog != USEC_INFINITY) { + if (streq(shutdown_verb, "reboot")) + watchdog_timer = arg_shutdown_watchdog; + else if (streq(shutdown_verb, "kexec")) + watchdog_timer = arg_kexec_watchdog; + + if (watchdog_timer > 0 && watchdog_timer != USEC_INFINITY) { char *e; - /* If we reboot let's set the shutdown + /* If we reboot or kexec let's set the shutdown * watchdog and tell the shutdown binary to * repeatedly ping it */ - r = watchdog_set_timeout(&arg_shutdown_watchdog); + r = watchdog_set_timeout(&watchdog_timer); watchdog_close(r < 0); /* Tell the binary how often to ping, ignore failure */ - if (asprintf(&e, "WATCHDOG_USEC="USEC_FMT, arg_shutdown_watchdog) > 0) + if (asprintf(&e, "WATCHDOG_USEC="USEC_FMT, watchdog_timer) > 0) (void) strv_consume(&env_block, e); if (arg_watchdog_device && @@ -2100,6 +2107,7 @@ static void reset_arguments(void) { arg_default_start_limit_burst = DEFAULT_START_LIMIT_BURST; arg_runtime_watchdog = 0; arg_shutdown_watchdog = 10 * USEC_PER_MINUTE; + arg_kexec_watchdog = 0; arg_early_core_pattern = NULL; arg_watchdog_device = NULL; @@ -2639,6 +2647,7 @@ finish: if (m) { arg_shutdown_watchdog = m->shutdown_watchdog; + arg_kexec_watchdog = m->kexec_watchdog; m = manager_free(m); } diff --git a/src/core/manager.h b/src/core/manager.h index 9f2b5a0eb0b..9600ab51446 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -229,6 +229,7 @@ struct Manager { usec_t runtime_watchdog; usec_t shutdown_watchdog; + usec_t kexec_watchdog; dual_timestamp timestamps[_MANAGER_TIMESTAMP_MAX]; diff --git a/src/core/system.conf.in b/src/core/system.conf.in index 8617ec20aa8..af6e5d9f62a 100644 --- a/src/core/system.conf.in +++ b/src/core/system.conf.in @@ -27,6 +27,7 @@ #NUMAMask= #RuntimeWatchdogSec=0 #ShutdownWatchdogSec=10min +#KExecWatchdogSec=0 #WatchdogDevice= #CapabilityBoundingSet= #NoNewPrivileges=no diff --git a/test/fuzz/fuzz-unit-file/directives.service b/test/fuzz/fuzz-unit-file/directives.service index 2f59585671f..d19610fc55c 100644 --- a/test/fuzz/fuzz-unit-file/directives.service +++ b/test/fuzz/fuzz-unit-file/directives.service @@ -703,6 +703,7 @@ LogTarget= RuntimeWatchdogSec= ShowStatus= ShutdownWatchdogSec= +KExecWatchdogSec= SuspendMode= SuspendState= SystemCallArchitectures= From 65224c1d0e50667a87c2c4f840c49d4918718f80 Mon Sep 17 00:00:00 2001 From: Luca Boccassi Date: Mon, 22 Jul 2019 11:39:25 +0100 Subject: [PATCH 2/2] core: rename ShutdownWatchdogSec to RebootWatchdogSec This option is only used on reboot, not on other types of shutdown modes, so it is misleading. Keep the old name working for backward compatibility, but remove it from the documentation. --- man/systemd-system.conf.xml | 8 ++++---- src/core/dbus-manager.c | 4 +++- src/core/main.c | 13 +++++++------ src/core/manager.h | 2 +- src/core/system.conf.in | 1 + test/fuzz/fuzz-unit-file/directives.service | 1 + 6 files changed, 17 insertions(+), 12 deletions(-) diff --git a/man/systemd-system.conf.xml b/man/systemd-system.conf.xml index 4dd4bf0ca8b..e403fa53086 100644 --- a/man/systemd-system.conf.xml +++ b/man/systemd-system.conf.xml @@ -127,7 +127,7 @@ RuntimeWatchdogSec= - ShutdownWatchdogSec= + RebootWatchdogSec= KExecWatchdogSec= Configure the hardware watchdog at runtime and at reboot. Takes a timeout value in seconds (or @@ -139,9 +139,9 @@ system manager will ensure to contact it at least once in half the specified timeout interval. This feature requires a hardware watchdog device to be present, as it is commonly the case in embedded and server systems. Not all hardware watchdogs allow configuration of all possible reboot timeout values, in which case - the closest available timeout is picked. ShutdownWatchdogSec= may be used to configure the + the closest available timeout is picked. RebootWatchdogSec= may be used to configure the hardware watchdog when the system is asked to reboot. It works as a safety net to ensure that the reboot takes - place even if a clean reboot attempt times out. Note that the ShutdownWatchdogSec= timeout + place even if a clean reboot attempt times out. Note that the RebootWatchdogSec= timeout applies only to the second phase of the reboot, i.e. after all regular services are already terminated, and after the system and service manager process (PID 1) got replaced by the systemd-shutdown binary, see system bootup7 @@ -149,7 +149,7 @@ and hence RuntimeWatchdogSec= is still honoured. In order to define a timeout on this first phase of system shutdown, configure JobTimeoutSec= and JobTimeoutAction= in the [Unit] section of the shutdown.target unit. By default - RuntimeWatchdogSec= defaults to 0 (off), and ShutdownWatchdogSec= to + RuntimeWatchdogSec= defaults to 0 (off), and RebootWatchdogSec= to 10min. KExecWatchdogSec= may be used to additionally enable the watchdog when kexec is being executed rather than when rebooting. Note that if the kernel does not reset the watchdog on kexec (depending on the specific hardware and/or driver), in this case the watchdog might not get disabled after kexec succeeds diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index e70c1e132a7..035011e34f9 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -2409,7 +2409,9 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_PROPERTY("DefaultStandardOutput", "s", bus_property_get_exec_output, offsetof(Manager, default_std_output), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_PROPERTY("DefaultStandardError", "s", bus_property_get_exec_output, offsetof(Manager, default_std_output), SD_BUS_VTABLE_PROPERTY_CONST), SD_BUS_WRITABLE_PROPERTY("RuntimeWatchdogUSec", "t", bus_property_get_usec, property_set_runtime_watchdog, offsetof(Manager, runtime_watchdog), 0), - SD_BUS_WRITABLE_PROPERTY("ShutdownWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, shutdown_watchdog), 0), + SD_BUS_WRITABLE_PROPERTY("RebootWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, reboot_watchdog), 0), + /* The following item is an obsolete alias */ + SD_BUS_WRITABLE_PROPERTY("ShutdownWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, reboot_watchdog), SD_BUS_VTABLE_HIDDEN), SD_BUS_WRITABLE_PROPERTY("KExecWatchdogUSec", "t", bus_property_get_usec, bus_property_set_usec, offsetof(Manager, kexec_watchdog), 0), SD_BUS_WRITABLE_PROPERTY("ServiceWatchdogs", "b", bus_property_get_bool, bus_property_set_bool, offsetof(Manager, service_watchdogs), 0), SD_BUS_PROPERTY("ControlGroup", "s", NULL, offsetof(Manager, cgroup_root), 0), diff --git a/src/core/main.c b/src/core/main.c index ccfa22c814d..187513bffe8 100644 --- a/src/core/main.c +++ b/src/core/main.c @@ -121,7 +121,7 @@ static bool arg_default_timeout_abort_set; static usec_t arg_default_start_limit_interval; static unsigned arg_default_start_limit_burst; static usec_t arg_runtime_watchdog; -static usec_t arg_shutdown_watchdog; +static usec_t arg_reboot_watchdog; static usec_t arg_kexec_watchdog; static char *arg_early_core_pattern; static char *arg_watchdog_device; @@ -555,7 +555,8 @@ static int parse_config_file(void) { { "Manager", "NUMAMask", config_parse_numa_mask, 0, &arg_numa_policy }, { "Manager", "JoinControllers", config_parse_warn_compat, DISABLED_CONFIGURATION, NULL }, { "Manager", "RuntimeWatchdogSec", config_parse_sec, 0, &arg_runtime_watchdog }, - { "Manager", "ShutdownWatchdogSec", config_parse_sec, 0, &arg_shutdown_watchdog }, + { "Manager", "RebootWatchdogSec", config_parse_sec, 0, &arg_reboot_watchdog }, + { "Manager", "ShutdownWatchdogSec", config_parse_sec, 0, &arg_reboot_watchdog }, /* obsolete alias */ { "Manager", "KExecWatchdogSec", config_parse_sec, 0, &arg_kexec_watchdog }, { "Manager", "WatchdogDevice", config_parse_path, 0, &arg_watchdog_device }, { "Manager", "CapabilityBoundingSet", config_parse_capability_set, 0, &arg_capability_bounding_set }, @@ -675,7 +676,7 @@ static void set_manager_settings(Manager *m) { m->confirm_spawn = arg_confirm_spawn; m->service_watchdogs = arg_service_watchdogs; m->runtime_watchdog = arg_runtime_watchdog; - m->shutdown_watchdog = arg_shutdown_watchdog; + m->reboot_watchdog = arg_reboot_watchdog; m->kexec_watchdog = arg_kexec_watchdog; m->cad_burst_action = arg_cad_burst_action; @@ -1401,7 +1402,7 @@ static int become_shutdown( assert(pos < ELEMENTSOF(command_line)); if (streq(shutdown_verb, "reboot")) - watchdog_timer = arg_shutdown_watchdog; + watchdog_timer = arg_reboot_watchdog; else if (streq(shutdown_verb, "kexec")) watchdog_timer = arg_kexec_watchdog; @@ -2106,7 +2107,7 @@ static void reset_arguments(void) { arg_default_start_limit_interval = DEFAULT_START_LIMIT_INTERVAL; arg_default_start_limit_burst = DEFAULT_START_LIMIT_BURST; arg_runtime_watchdog = 0; - arg_shutdown_watchdog = 10 * USEC_PER_MINUTE; + arg_reboot_watchdog = 10 * USEC_PER_MINUTE; arg_kexec_watchdog = 0; arg_early_core_pattern = NULL; arg_watchdog_device = NULL; @@ -2646,7 +2647,7 @@ finish: pager_close(); if (m) { - arg_shutdown_watchdog = m->shutdown_watchdog; + arg_reboot_watchdog = m->reboot_watchdog; arg_kexec_watchdog = m->kexec_watchdog; m = manager_free(m); } diff --git a/src/core/manager.h b/src/core/manager.h index 9600ab51446..a40626e6162 100644 --- a/src/core/manager.h +++ b/src/core/manager.h @@ -228,7 +228,7 @@ struct Manager { char **client_environment; /* Environment variables created by clients through the bus API */ usec_t runtime_watchdog; - usec_t shutdown_watchdog; + usec_t reboot_watchdog; usec_t kexec_watchdog; dual_timestamp timestamps[_MANAGER_TIMESTAMP_MAX]; diff --git a/src/core/system.conf.in b/src/core/system.conf.in index af6e5d9f62a..8112125468a 100644 --- a/src/core/system.conf.in +++ b/src/core/system.conf.in @@ -26,6 +26,7 @@ #NUMAPolicy=default #NUMAMask= #RuntimeWatchdogSec=0 +#RebootWatchdogSec=10min #ShutdownWatchdogSec=10min #KExecWatchdogSec=0 #WatchdogDevice= diff --git a/test/fuzz/fuzz-unit-file/directives.service b/test/fuzz/fuzz-unit-file/directives.service index d19610fc55c..fe9d451b41a 100644 --- a/test/fuzz/fuzz-unit-file/directives.service +++ b/test/fuzz/fuzz-unit-file/directives.service @@ -702,6 +702,7 @@ LogLocation= LogTarget= RuntimeWatchdogSec= ShowStatus= +RebootWatchdogSec= ShutdownWatchdogSec= KExecWatchdogSec= SuspendMode=