ptyfwd: Forward various signals to forked process (#36345)

TODO

@@ -125,6 +125,10 @@ Deprecations and removals:
 
 Features:
 
+* Make run0 forward various signals to the forked process so that sending
+  signals to a child process works rougly the same regardless of whether the
+  child process is spawned via run0 or not.
+
 * write a document explaining how to write correct udev rules. Mention things
   such as:
   1. do not do lists of vid/pid matches, use hwdb for that

src/libsystemd/sd-event/event-util.c

@@ -8,6 +8,9 @@
 #include "log.h"
 #include "string-util.h"
 
+#define SI_FLAG_FORWARD  (INT32_C(1) << 30)
+#define SI_FLAG_POSITIVE (INT32_C(1) << 29)
+
 int event_reset_time(
                 sd_event *e,
                 sd_event_source **s,
@@ -177,3 +180,89 @@ dual_timestamp* event_dual_timestamp_now(sd_event *e, dual_timestamp *ts) {
         assert_se(sd_event_now(e, CLOCK_MONOTONIC, &ts->monotonic) >= 0);
         return ts;
 }
+
+void event_source_unref_many(sd_event_source **array, size_t n) {
+        FOREACH_ARRAY(v, array, n)
+                sd_event_source_unref(*v);
+
+        free(array);
+}
+
+static int event_forward_signal_callback(sd_event_source *s, const struct signalfd_siginfo *ssi, void *userdata) {
+        sd_event_source *child = ASSERT_PTR(userdata);
+
+        assert(ssi);
+
+        siginfo_t si = {
+                .si_signo = ssi->ssi_signo,
+                /* We include some extra information to indicate the signal was forwarded and originally a positive
+                 * value since we can only set negative values ourselves as positive values are prohibited by the
+                 * kernel. */
+                .si_code = (ssi->ssi_code & (SI_FLAG_FORWARD|SI_FLAG_POSITIVE)) ? INT_MIN :
+                           (ssi->ssi_code >= 0 ? (-ssi->ssi_code - 1) | SI_FLAG_POSITIVE | SI_FLAG_FORWARD : ssi->ssi_code | SI_FLAG_FORWARD),
+                .si_errno = ssi->ssi_errno,
+        };
+
+        /* The following fields are implemented as macros, hence we cannot use compound initialization for them. */
+        si.si_pid = ssi->ssi_pid;
+        si.si_uid = ssi->ssi_uid;
+        si.si_int = ssi->ssi_int;
+        si.si_ptr = UINT64_TO_PTR(ssi->ssi_ptr);
+
+        return sd_event_source_send_child_signal(child, ssi->ssi_signo, &si, /* flags = */ 0);
+}
+
+static void event_forward_signal_destroy(void *userdata) {
+        sd_event_source *child = ASSERT_PTR(userdata);
+        sd_event_source_unref(child);
+}
+
+int event_forward_signals(
+                sd_event *e,
+                sd_event_source *child,
+                const int *signals,
+                size_t n_signals,
+                sd_event_source ***ret_sources,
+                size_t *ret_n_sources) {
+
+        sd_event_source **sources = NULL;
+        size_t n_sources = 0;
+        int r;
+
+        CLEANUP_ARRAY(sources, n_sources, event_source_unref_many);
+
+        assert(e);
+        assert(child);
+        assert(child->type == SOURCE_CHILD);
+        assert(signals || n_signals == 0);
+        assert(ret_sources);
+        assert(ret_n_sources);
+
+        if (n_signals == 0) {
+                *ret_sources = NULL;
+                *ret_n_sources = 0;
+                return 0;
+        }
+
+        sources = new0(sd_event_source*, n_signals);
+        if (!sources)
+                return -ENOMEM;
+
+        FOREACH_ARRAY(sig, signals, n_signals) {
+                r = sd_event_add_signal(e, &sources[n_sources], *sig | SD_EVENT_SIGNAL_PROCMASK, event_forward_signal_callback, child);
+                if (r < 0)
+                        return r;
+
+                r = sd_event_source_set_destroy_callback(sources[n_sources], event_forward_signal_destroy);
+                if (r < 0)
+                        return r;
+
+                sd_event_source_ref(child);
+                n_sources++;
+        }
+
+        *ret_sources = TAKE_PTR(sources);
+        *ret_n_sources = n_sources;
+
+        return 0;
+}

src/libsystemd/sd-event/event-util.h

@@ -38,3 +38,7 @@ int event_add_time_change(sd_event *e, sd_event_source **ret, sd_event_io_handle
 int event_add_child_pidref(sd_event *e, sd_event_source **s, const PidRef *pid, int options, sd_event_child_handler_t callback, void *userdata);
 
 dual_timestamp* event_dual_timestamp_now(sd_event *e, dual_timestamp *ts);
+
+void event_source_unref_many(sd_event_source **array, size_t n);
+
+int event_forward_signals(sd_event *e, sd_event_source *child, const int *signals, size_t n_signals, sd_event_source ***ret_sources, size_t *ret_n_sources);

src/ptyfwd/ptyfwd-tool.c

@@ -137,9 +137,13 @@ static int run(int argc, char *argv[]) {
         _cleanup_close_ int pty_fd = -EBADF, peer_fd = -EBADF;
         _cleanup_(pty_forward_freep) PTYForward *forward = NULL;
         _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL;
-        _cleanup_(sd_event_source_unrefp) sd_event_source *exit_source = NULL;
+        _cleanup_(sd_event_source_unrefp) sd_event_source *child = NULL;
+        sd_event_source **forward_signal_sources = NULL;
+        size_t n_forward_signal_sources = 0;
         int r;
 
+        CLEANUP_ARRAY(forward_signal_sources, n_forward_signal_sources, event_source_unref_many);
+
         log_setup();
 
         assert_se(sigprocmask_many(SIG_BLOCK, /*ret_old_mask=*/ NULL, SIGCHLD) >= 0);
@@ -156,8 +160,6 @@ static int run(int argc, char *argv[]) {
         if (r < 0)
                 return log_error_errno(r, "Failed to get event loop: %m");
 
-        (void) sd_event_set_signal_exit(event, true);
-
         pty_fd = openpt_allocate(O_RDWR|O_NOCTTY|O_NONBLOCK|O_CLOEXEC, /*ret_peer=*/ NULL);
         if (pty_fd < 0)
                 return log_error_errno(pty_fd, "Failed to acquire pseudo tty: %m");
@@ -208,14 +210,28 @@ static int run(int argc, char *argv[]) {
 
         peer_fd = safe_close(peer_fd);
 
-        r = event_add_child_pidref(event, &exit_source, &pidref, WEXITED, helper_on_exit, NULL);
+        r = event_add_child_pidref(event, &child, &pidref, WEXITED, helper_on_exit, NULL);
         if (r < 0)
                 return log_error_errno(r, "Failed to add child event source: %m");
 
-        r = sd_event_source_set_child_process_own(exit_source, true);
+        r = sd_event_source_set_child_process_own(child, true);
         if (r < 0)
                 return log_error_errno(r, "Failed to take ownership of child process: %m");
 
+        /* Make sure we don't forward signals to a dead child process by increasing the priority of the child
+         * process event source. */
+        r = sd_event_source_set_priority(child, SD_EVENT_PRIORITY_IMPORTANT);
+        if (r < 0)
+                return log_error_errno(r, "Failed to set child event source priority: %m");
+
+        r = event_forward_signals(
+                        event,
+                        child,
+                        pty_forward_signals, ELEMENTSOF(pty_forward_signals),
+                        &forward_signal_sources, &n_forward_signal_sources);
+        if (r < 0)
+                return log_error_errno(r, "Failed to set up signal forwarding: %m");
+
         return sd_event_loop(event);
 }
 

src/shared/ptyfwd.c

@@ -46,6 +46,11 @@ typedef enum AnsiColorState  {
 
 assert_cc(ANSI_SEQUENCE_LENGTH_MAX > ANSI_SEQUENCE_WINDOW_TITLE_MAX);
 
+/* We don't list SIGHUP here because that's what you get when your tty has a hangup, and if we do we'll close
+ * the pty too which already generates a hangup, and thus a SIGHUP, which means we'd generate SIGHUP twice,
+ * once by us, and once by the kernel. */
+const int pty_forward_signals[N_PTY_FORWARD_SIGNALS] = { SIGTERM, SIGINT, SIGQUIT, SIGTSTP, SIGCONT, SIGUSR1, SIGUSR2 };
+
 struct PTYForward {
         sd_event *event;
 

src/shared/ptyfwd.h

@@ -25,6 +25,9 @@ typedef enum PTYForwardFlags {
 
 typedef int (*PTYForwardHandler)(PTYForward *f, int rcode, void *userdata);
 
+#define N_PTY_FORWARD_SIGNALS 7
+extern const int pty_forward_signals[N_PTY_FORWARD_SIGNALS];
+
 int pty_forward_new(sd_event *event, int master, PTYForwardFlags flags, PTYForward **ret);
 PTYForward* pty_forward_free(PTYForward *f);
 

test/units/TEST-74-AUX-UTILS.pty-forward.sh

@@ -4,3 +4,36 @@ set -eux
 set -o pipefail
 
 systemd-pty-forward --background 41 --title test echo foobar
+
+# Test that signals are forwarded to the systemd-pty-forward child process.
+cat >/tmp/child <<\EOF
+#!/usr/bin/bash
+set -x
+
+trap 'touch /tmp/int' INT
+
+# We need to wait for the sleep process asynchronously in order to allow
+# bash to process signals
+sleep infinity &
+
+# notify that the process is ready
+touch /tmp/ready
+
+PID=$!
+while :; do
+    wait || :
+done
+EOF
+
+chmod +x /tmp/child
+
+systemd-pty-forward /tmp/child &
+PID=$!
+
+timeout 5 bash -c "until test -e /tmp/ready; do sleep .5; done"
+
+kill -INT "$PID"
+
+timeout 5 bash -c "until test -e /tmp/int; do sleep .5; done"
+
+kill "$PID"