core/service: Prevent accidental service termination during reload (#39692)

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 two new checks for Type=notify-reload
services:

1. On 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 failed with SERVICE_FAILURE_PROTOCOL. This enforces the contract that
services *must* install their handler before signalling readiness.

2. On reload: Immediately before sending the reload signal, we check
again. If the handler is now missing, we log a warning but still send
the signal. This preserves the requested operator action while making
the missing handler visible.

The startup check catches definite static misconfiguration before the
service is accepted as ready. The reload check is advisory because the
service has already started successfully, but warns if its handler later
disappears. Together these provide a best-effort safety net while
preserving the agreed reload semantics.

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.

man/systemd.service.xml

@@ -256,6 +256,26 @@
             command for reloading of the service's configuration.</para>
 
             <para>The signal to send can be tweaked via <varname>ReloadSignal=</varname>, see below.</para>
+
+            <para>For <option>notify-reload</option> services, systemd verifies that the service's main process
+            has actually installed a handler for the configured reload signal (by default
+            <constant>SIGHUP</constant>). This verification checks for both traditional signal handlers (via
+            <citerefentry project="man-pages"><refentrytitle>sigaction</refentrytitle><manvolnum>2</manvolnum></citerefentry>)
+            and blocked signals that are typically handled via
+            <citerefentry project="man-pages"><refentrytitle>signalfd</refentrytitle><manvolnum>2</manvolnum></citerefentry>.
+            This verification is performed at two points:</para>
+
+            <orderedlist>
+              <listitem><para>When the service first sends <literal>READY=1</literal> during initial startup. If no
+              signal handler is detected at this point, the service startup is aborted.</para></listitem>
+
+              <listitem><para>Immediately before sending the reload signal. If the handler is no longer present,
+              systemd logs a warning but the reload signal is still sent.</para></listitem>
+            </orderedlist>
+
+            <para>This prevents accidentally starting services that lack the required signal handler. Services
+            using <option>notify-reload</option> must install their reload signal handler before sending the
+            initial <literal>READY=1</literal> notification.</para>
             </listitem>
 
             <listitem><para>Behavior of <option>idle</option> is very similar to <option>simple</option>; however,

src/basic/process-util.c

@@ -21,6 +21,7 @@
 #include "alloc-util.h"
 #include "architecture.h"
 #include "argv-util.h"
+#include "bitfield.h"
 #include "capability-util.h"
 #include "cgroup-util.h"
 #include "dirent-util.h"
@@ -804,6 +805,53 @@ int get_process_umask(pid_t pid, mode_t *ret) {
         return parse_mode(m, ret);
 }
 
+static int pid_get_sigmask(pid_t pid, const char *field_name, uint64_t *ret) {
+        _cleanup_free_ char *field = NULL;
+        int r;
+
+        assert(pid >= 0);
+        assert(field_name);
+        assert(ret);
+
+        r = procfs_file_get_field(pid, "status", field_name, &field);
+        if (r == -ENOENT)
+                return -ESRCH;
+        if (r < 0)
+                return r;
+
+        return safe_atou64_full(field, 16, ret);
+}
+
+static int pidref_has_sigmask(const PidRef *pidref, int sig, const char *field_name) {
+        uint64_t mask;
+        int r;
+
+        if (!pidref_is_set(pidref))
+                return -ESRCH;
+        if (pidref_is_remote(pidref))
+                return -EREMOTE;
+        if (!SIGNAL_VALID(sig))
+                return -EINVAL;
+
+        r = pid_get_sigmask(pidref->pid, field_name, &mask);
+        if (r < 0)
+                return r;
+
+        r = pidref_verify(pidref);
+        if (r < 0)
+                return r;
+
+        return BIT_SET(mask, sig - 1);
+}
+
+int pidref_has_sigcgt(const PidRef *pidref, int sig) {
+        return pidref_has_sigmask(pidref, sig, "SigCgt");
+}
+
+int pidref_has_sigblk(const PidRef *pidref, int sig) {
+        return pidref_has_sigmask(pidref, sig, "SigBlk");
+}
+
 /*
  * Return values:
  * < 0 : pidref_wait_for_terminate() failed to get the state of the

src/basic/process-util.h

@@ -53,6 +53,8 @@ int pidref_get_ppid_as_pidref(const PidRef *pidref, PidRef *ret);
 int pid_get_start_time(pid_t pid, usec_t *ret);
 int pidref_get_start_time(const PidRef *pid, usec_t *ret);
 int get_process_umask(pid_t pid, mode_t *ret);
+int pidref_has_sigcgt(const PidRef *pidref, int sig);
+int pidref_has_sigblk(const PidRef *pidref, int sig);
 
 static inline bool SIGINFO_CODE_IS_DEAD(int code) {
         return IN_SET(code, CLD_EXITED, CLD_KILLED, CLD_DUMPED);

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

test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/notify-reload-harness.sh

@@ -0,0 +1,44 @@
+#!/usr/bin/env bash
+# SPDX-License-Identifier: LGPL-2.1-or-later
+set -eux
+set -o pipefail
+
+MODE="${1:-well-behaved}"
+FIFO_IN="${2:-/tmp/reload-test-fifo-in}"
+FIFO_OUT="${3:-/tmp/reload-test-fifo-out}"
+
+handle_signal() {
+    echo "reload" >"$FIFO_OUT"
+    systemd-notify --reloading
+    systemd-notify --ready --status="Reloaded"
+}
+
+case "$MODE" in
+    no-handler)
+        ;;
+    toggle-handler|well-behaved)
+        trap 'handle_signal' HUP
+        ;;
+    *)
+        echo "Unknown mode: $MODE" >&2
+        exit 1
+        ;;
+esac
+
+systemd-notify --ready --status="Started with mode=$MODE"
+
+while read -r cmd < "$FIFO_IN"; do
+    case "$cmd" in
+        remove-handler)
+            trap - HUP
+            echo "handler-removed" >"$FIFO_OUT"
+            ;;
+        exit)
+            break
+            ;;
+        *)
+            echo "Unknown command: $cmd" >&2
+            exit 1
+            ;;
+    esac
+done

test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/notify-reload-no-handler.service

@@ -0,0 +1,6 @@
+[Unit]
+Description=Test notify-reload without signal handler
+
+[Service]
+Type=notify-reload
+ExecStart=/usr/lib/systemd/tests/testdata/TEST-80-NOTIFYACCESS.units/notify-reload-harness.sh no-handler /tmp/reload-nohandler-in /tmp/reload-nohandler-out

test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/notify-reload-sigstop.service

@@ -0,0 +1,7 @@
+[Unit]
+Description=Test notify-reload with SIGSTOP
+
+[Service]
+Type=notify-reload
+ReloadSignal=SIGSTOP
+ExecStart=/usr/lib/systemd/tests/testdata/TEST-80-NOTIFYACCESS.units/notify-reload-harness.sh no-handler /tmp/reload-sigstop-in /tmp/reload-sigstop-out

test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/notify-reload-toggle-handler.service

@@ -0,0 +1,6 @@
+[Unit]
+Description=Test notify-reload with handler that can be removed
+
+[Service]
+Type=notify-reload
+ExecStart=/usr/lib/systemd/tests/testdata/TEST-80-NOTIFYACCESS.units/notify-reload-harness.sh toggle-handler /tmp/reload-toggle-in /tmp/reload-toggle-out

test/integration-tests/TEST-80-NOTIFYACCESS/TEST-80-NOTIFYACCESS.units/notify-reload-well-behaved.service

@@ -0,0 +1,6 @@
+[Unit]
+Description=Test notify-reload with proper signal handler
+
+[Service]
+Type=notify-reload
+ExecStart=/usr/lib/systemd/tests/testdata/TEST-80-NOTIFYACCESS.units/notify-reload-harness.sh well-behaved /tmp/reload-wellbehaved-in /tmp/reload-wellbehaved-out

test/units/TEST-80-NOTIFYACCESS.sh

@@ -172,4 +172,85 @@ assert_eq "$(systemctl show fdstore-nopin.service -P NFileDescriptorStore)" 0
 assert_eq "$(systemctl show fdstore-pin.service -P SubState)" dead
 assert_eq "$(systemctl show fdstore-nopin.service -P SubState)" dead
 
+# Test notify-reload signal handler validation
+
+cleanup_notify_reload() {
+    systemctl stop notify-reload-no-handler.service \
+        notify-reload-sigstop.service \
+        notify-reload-toggle-handler.service \
+        notify-reload-well-behaved.service || :
+    systemctl reset-failed notify-reload-no-handler.service \
+        notify-reload-sigstop.service \
+        notify-reload-toggle-handler.service \
+        notify-reload-well-behaved.service || :
+    rm -f /tmp/reload-nohandler-in /tmp/reload-nohandler-out \
+        /tmp/reload-nohandler-cursor \
+        /tmp/reload-sigstop-in /tmp/reload-sigstop-out \
+        /tmp/reload-toggle-in /tmp/reload-toggle-out \
+        /tmp/reload-toggle-cursor \
+        /tmp/reload-wellbehaved-in /tmp/reload-wellbehaved-out
+}
+
+trap cleanup_notify_reload EXIT
+cleanup_notify_reload
+
+# Test 1: Service without signal handler should fail to start
+mkfifo /tmp/reload-nohandler-in /tmp/reload-nohandler-out
+journalctl -q -n 0 --cursor-file=/tmp/reload-nohandler-cursor
+(! systemctl start notify-reload-no-handler.service)
+assert_eq "$(systemctl show notify-reload-no-handler.service -P SubState)" "failed"
+assert_eq "$(systemctl show notify-reload-no-handler.service -P Result)" "protocol"
+# Verify error was logged about missing signal handler
+journalctl --sync
+journalctl -u notify-reload-no-handler.service --cursor-file=/tmp/reload-nohandler-cursor \
+    --grep "lacks handler for reload signal" >/dev/null
+systemctl reset-failed notify-reload-no-handler.service
+rm -f /tmp/reload-nohandler-in /tmp/reload-nohandler-out
+
+# Test 2: SIGSTOP cannot be caught or blocked, so it is exempt from handler validation
+mkfifo /tmp/reload-sigstop-in /tmp/reload-sigstop-out
+systemctl start notify-reload-sigstop.service
+assert_eq "$(systemctl show notify-reload-sigstop.service -P SubState)" "running"
+systemctl stop notify-reload-sigstop.service
+rm -f /tmp/reload-sigstop-in /tmp/reload-sigstop-out
+
+# Test 3: Service that removes handler should warn on reload but still reload
+mkfifo /tmp/reload-toggle-in /tmp/reload-toggle-out
+systemctl start notify-reload-toggle-handler.service
+assert_eq "$(systemctl show notify-reload-toggle-handler.service -P SubState)" "running"
+
+# Command service to remove its signal handler
+timeout 10 bash -c 'echo "remove-handler" >/tmp/reload-toggle-in'
+response="$(timeout 10 cat /tmp/reload-toggle-out)"
+assert_eq "$response" "handler-removed"
+
+# Reload should succeed (with warning) even though handler is gone - service will be killed by SIGHUP
+# but that's intentional after the first start succeeds
+journalctl -q -n 0 --cursor-file=/tmp/reload-toggle-cursor
+systemctl reload --no-block notify-reload-toggle-handler.service
+# The service will die from SIGHUP since the handler is removed
+timeout 10 bash -c 'while systemctl is-active --quiet notify-reload-toggle-handler.service; do sleep .5; done'
+# Verify warning was logged
+journalctl --sync
+journalctl -u notify-reload-toggle-handler.service --cursor-file=/tmp/reload-toggle-cursor \
+    --grep "lacks handler for reload signal" >/dev/null
+rm -f /tmp/reload-toggle-in /tmp/reload-toggle-out
+
+# Test 4: Well-behaved service with handler should work correctly
+mkfifo /tmp/reload-wellbehaved-in /tmp/reload-wellbehaved-out
+systemctl start notify-reload-well-behaved.service
+assert_eq "$(systemctl show notify-reload-well-behaved.service -P SubState)" "running"
+
+# Reload should succeed
+systemctl reload --no-block notify-reload-well-behaved.service
+timeout 10 bash -c 'until [[ $(systemctl show notify-reload-well-behaved.service -P SubState) == "reload-signal" ]]; do sleep .5; done'
+response="$(timeout 10 cat /tmp/reload-wellbehaved-out)"
+assert_eq "$response" "reload"
+timeout 10 bash -c 'until [[ $(systemctl show notify-reload-well-behaved.service -P SubState) == "running" ]]; do sleep .5; done'
+assert_eq "$(systemctl show notify-reload-well-behaved.service -P ReloadResult)" "success"
+
+timeout 10 bash -c 'echo "exit" >/tmp/reload-wellbehaved-in'
+systemctl stop notify-reload-well-behaved.service
+rm -f /tmp/reload-wellbehaved-in /tmp/reload-wellbehaved-out
+
 touch /testok