From b000e16798022241b334b9f7ae595abb46dc50f2 Mon Sep 17 00:00:00 2001 From: Daan De Meyer Date: Mon, 10 Feb 2025 23:59:04 +0100 Subject: [PATCH] ptyfwd: Forward various signals to forked process We want systemd-pty-forward to be something that can be dropped in somewhere without too much thought. To enable this, let's make sure we forward various signals to the forked process. This makes sure that any signals are delivered to the actual child process regardless of whether it's running within systemd-pty-forward or not. --- src/libsystemd/sd-event/event-util.c | 89 +++++++++++++++++++++ src/libsystemd/sd-event/event-util.h | 4 + src/ptyfwd/ptyfwd-tool.c | 26 ++++-- src/shared/ptyfwd.c | 5 ++ src/shared/ptyfwd.h | 3 + test/units/TEST-74-AUX-UTILS.pty-forward.sh | 33 ++++++++ 6 files changed, 155 insertions(+), 5 deletions(-) diff --git a/src/libsystemd/sd-event/event-util.c b/src/libsystemd/sd-event/event-util.c index 862455e19c3..42e973564e3 100644 --- a/src/libsystemd/sd-event/event-util.c +++ b/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; +} diff --git a/src/libsystemd/sd-event/event-util.h b/src/libsystemd/sd-event/event-util.h index 7002ca37da3..383b4c82b7c 100644 --- a/src/libsystemd/sd-event/event-util.h +++ b/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); diff --git a/src/ptyfwd/ptyfwd-tool.c b/src/ptyfwd/ptyfwd-tool.c index 2e16dfaa441..c6ae362e3d3 100644 --- a/src/ptyfwd/ptyfwd-tool.c +++ b/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); } diff --git a/src/shared/ptyfwd.c b/src/shared/ptyfwd.c index fb7c1c05cf8..c5814ad3c7a 100644 --- a/src/shared/ptyfwd.c +++ b/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; diff --git a/src/shared/ptyfwd.h b/src/shared/ptyfwd.h index a6e729d7bc6..bd33551a6df 100644 --- a/src/shared/ptyfwd.h +++ b/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); diff --git a/test/units/TEST-74-AUX-UTILS.pty-forward.sh b/test/units/TEST-74-AUX-UTILS.pty-forward.sh index 9b9b14a18a8..deb49352628 100755 --- a/test/units/TEST-74-AUX-UTILS.pty-forward.sh +++ b/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" -- 2.47.3