diff --git a/NEWS b/NEWS index efa5d46419..b9a1e30a24 100644 --- a/NEWS +++ b/NEWS @@ -4,6 +4,12 @@ CHANGES WITH 262: Feature Removals and Incompatible Changes: + * Services of Type=notify-reload are now required to catch or block + ReloadSignal= when they send READY=1. If they do neither at that + point, the service fails to start with a protocol error. Previously, + such services were allowed to start even though a later reload could + invoke the signal's default action and terminate the service. + * systemd-repart's MakeSymlinks= option now expands "%" specifiers in the symlink target. Previously, these were only expanded in the source. diff --git a/man/systemd.service.xml b/man/systemd.service.xml index 69600b61e2..57bb224c24 100644 --- a/man/systemd.service.xml +++ b/man/systemd.service.xml @@ -256,6 +256,26 @@ command for reloading of the service's configuration. The signal to send can be tweaked via ReloadSignal=, see below. + + For services, systemd verifies that the service's main process + has actually installed a handler for the configured reload signal (by default + SIGHUP). This verification checks for both traditional signal handlers (via + sigaction2) + and blocked signals that are typically handled via + signalfd2. + This verification is performed at two points: + + + When the service first sends READY=1 during initial startup. If no + signal handler is detected at this point, the service startup is aborted. + + Immediately before sending the reload signal. If the handler is no longer present, + systemd logs a warning but the reload signal is still sent. + + + This prevents accidentally starting services that lack the required signal handler. Services + using must install their reload signal handler before sending the + initial READY=1 notification. Behavior of is very similar to ; however, diff --git a/src/core/service.c b/src/core/service.c index 1c8460b136..1918147525 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -3137,6 +3137,53 @@ static void service_enter_reload_post(Service *s) { service_reload_finish(s, SERVICE_SUCCESS); } +static int service_check_reload_signal_handler(Service *s, const char *missing_suffix, const char *error_suffix) { + int r; + + assert(s); + + if (!pidref_is_set(&s->main_pid) || IN_SET(s->reload_signal, SIGKILL, SIGSTOP)) + return 0; + + /* Check if the process has a traditional signal handler (SigCgt) */ + r = pidref_has_sigcgt(&s->main_pid, s->reload_signal); + if (r < 0) { + if (r != -ESRCH) + log_unit_warning_errno( + UNIT(s), r, + "Failed to check for reload signal handler%s: %m", + strempty(error_suffix)); + return r; + } + + if (r == 0) { + /* No traditional handler, check if the signal is blocked (SigBlk). A blocked signal is + * typically handled via signalfd, which is a valid way to handle signals (e.g., via + * sd_event_add_signal() with SD_EVENT_SIGNAL_PROCMASK). */ + r = pidref_has_sigblk(&s->main_pid, s->reload_signal); + if (r < 0) { + if (r != -ESRCH) + log_unit_warning_errno( + UNIT(s), r, + "Failed to check for blocked reload signal%s: %m", + strempty(error_suffix)); + return r; + } + } + + if (r == 0) { + log_unit_warning( + UNIT(s), + "Main process " PID_FMT " lacks handler for reload signal %s%s.", + s->main_pid.pid, + signal_to_string(s->reload_signal), + strempty(missing_suffix)); + return -EOPNOTSUPP; + } + + return 0; +} + static void service_enter_reload_signal(Service *s) { int r; @@ -3161,6 +3208,14 @@ static void service_enter_reload_signal(Service *s) { goto fail; } + /* This is naturally racy, but that's fine. The issue we're looking for is almost always a + * static programming error (handler not yet installed), but if a user wants to shoot + * themself in the foot intentionally by racing with us, who are we to stop them :-) */ + (void) service_check_reload_signal_handler( + s, + ", sending reload signal anyway", + ", sending reload signal anyway"); + r = pidref_kill_and_sigcont(&s->main_pid, s->reload_signal); if (r < 0) { log_unit_warning_errno(UNIT(s), r, "Failed to send reload signal: %m"); @@ -5399,6 +5454,14 @@ static void service_notify_message_process_state(Service *s, char * const *tags) if (strv_contains(tags, "READY=1")) { + if (s->type == SERVICE_NOTIFY_RELOAD && s->state == SERVICE_START) { + r = service_check_reload_signal_handler(s, ", refusing service startup", ", ignoring"); + if (r == -EOPNOTSUPP) { + service_enter_signal(s, SERVICE_STOP_SIGTERM, SERVICE_FAILURE_PROTOCOL); + return; + } + } + if (s->notify_state == NOTIFY_RELOADING) s->notify_state = NOTIFY_RELOAD_READY; else