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

@@ -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/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