From d361e5ba34f1829ac2b01843572b403f7cce8277 Mon Sep 17 00:00:00 2001 From: Chris Down Date: Wed, 12 Nov 2025 15:22:28 +0800 Subject: [PATCH] core/service: Prevent accidental service termination during reload We currently blindly send the configured reload signal (e.g. SIGHUP or SIGUSR1) to the main service PID for Type=notify-reload units, even if the service hasn't installed a userspace handler. This can lead to invoking unintended default behavior (typically process termination) in daemons which later deprecate and remove their reload handler. This is a real problem we have seen in production on multiple occasions. In one particularly egregious instance, a production distributed storage service had a large percentage of its nodes all terminate at once when sent a reload signal. In this case a signal handler had been removed, but another place still sending the signal was missed. To mitigate this, introduce signal handler validation for Type=notify-reload services. We check both SigCgt (traditional handlers via sigaction) and SigBlk (blocked signals for signalfd) to detect valid handler configurations: 1. On first READY=1: When the service first sends READY=1 during initial startup, we check for the handler. If it's missing, the service startup is aborted with Result=protocol. 2. On reload: Immediately before sending the reload signal, we check again. If the handler is missing, we log a warning but still send the signal. The rationale is that a missing handler at first startup represents a definite misconfiguration that should be caught early. A missing handler at reload time is warned about, but we defer to the operator's judgement since the service already started successfully. These checks are a best-effort, pragmatic safety net. They are naturally racy, but in practice they prevent the common class of bugs from static misconfiguration or software updates. --- NEWS | 6 ++++ man/systemd.service.xml | 20 +++++++++++++ src/core/service.c | 63 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 89 insertions(+) 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