From: Lennart Poettering Date: Wed, 15 Feb 2023 09:25:51 +0000 (+0100) Subject: pid1: add a new D-Bus method for enquing POSIX signals with values to unit processes X-Git-Tag: v254-rc1~1251^2~6 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=a721cd0016fb662fc5888cef959eec19f96b4040;p=thirdparty%2Fsystemd.git pid1: add a new D-Bus method for enquing POSIX signals with values to unit processes This augments the existing KillUnit() + Kill() methods with QueueSignalUnit() + QueueSignal(), which are what sigqueue() is to kill(). This is useful for sending our new SIGRTMIN+18 control signals to system services. --- diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 1592d046ad2..5bae4738563 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -112,6 +112,10 @@ node /org/freedesktop/systemd1 { KillUnit(in s name, in s whom, in i signal); + QueueSignalUnit(in s name, + in s whom, + in i signal, + in i value); CleanUnit(in s name, in as mask); FreezeUnit(in s name); @@ -826,6 +830,8 @@ node /org/freedesktop/systemd1 { + + @@ -1286,6 +1292,13 @@ node /org/freedesktop/systemd1 { ExecStop= and is spawned in parallel to the main daemon process in order to shut it down. + QueueSignalUnit() is similar to KillUnit() but may be + used to enqueue a POSIX Realtime Signal (i.e. SIGRTMIN+… and + SIGRTMAX-…) to the selected process(es). Takes the same paramaters as + KillUnit() with one additional argument: an integer that is passed in the + sival_int value accompanying the queued signal. See sigqueue3 for + details. + GetJob() returns the job object path for a specific job, identified by its id. @@ -1731,7 +1744,8 @@ node /org/freedesktop/systemd1 { Read access is generally granted to all clients. Additionally, for unprivileged clients, some operations are allowed through the polkit privilege system. Operations which modify unit state (StartUnit(), StopUnit(), KillUnit(), - RestartUnit() and similar, SetProperty()) require + QueueSignalUnit(), RestartUnit() and similar, + SetProperty()) require org.freedesktop.systemd1.manage-units. Operations which modify unit file enablement state (EnableUnitFiles(), DisableUnitFiles(), EnableUnitFilesWithFlags(), DisableUnitFilesWithFlags(), @@ -1778,6 +1792,9 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { out a(uosos) affected_jobs); Kill(in s whom, in i signal); + QueueSignal(in s whom, + in i signal, + in i value); ResetFailed(); SetProperties(in b runtime, in a(sv) properties); @@ -2084,6 +2101,8 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { + + @@ -2302,12 +2321,12 @@ node /org/freedesktop/systemd1/unit/avahi_2ddaemon_2eservice { Start(), Stop(), Reload(), Restart(), TryRestart(), ReloadOrRestart(), ReloadOrTryRestart(), - Kill(), ResetFailed(), and - SetProperties() implement the same operation as the respective methods on the + Kill(), QueueSignal(), ResetFailed(), + and SetProperties() implement the same operation as the respective methods on the Manager object (see above). However, these methods operate on the unit - object and hence do not take a unit name parameter. Invoking the methods directly on the Manager - object has the advantage of not requiring a GetUnit() call to get the unit object - for a specific unit name. Calling the methods on the Manager object is hence a round trip + object and hence do not take a unit name parameter. Invoking the methods directly on the Manager object + has the advantage of not requiring a GetUnit() call to get the unit object for a + specific unit name. Calling the methods on the Manager object is hence a round trip optimization. diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index c4f205bc423..7d6f6bfc0fa 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -3034,6 +3034,11 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_NO_RESULT, method_kill_unit, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD_WITH_ARGS("QueueSignalUnit", + SD_BUS_ARGS("s", name, "s", whom, "i", signal, "i", value), + SD_BUS_NO_RESULT, + method_kill_unit, + SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD_WITH_ARGS("CleanUnit", SD_BUS_ARGS("s", name, "as", mask), SD_BUS_NO_RESULT, diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 1ef98da6fd9..8f851cbceb2 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -513,10 +513,11 @@ int bus_unit_method_enqueue_job(sd_bus_message *message, void *userdata, sd_bus_ int bus_unit_method_kill(sd_bus_message *message, void *userdata, sd_bus_error *error) { Unit *u = ASSERT_PTR(userdata); + int32_t value = 0; const char *swho; int32_t signo; KillWho who; - int r; + int r, code; assert(message); @@ -528,17 +529,30 @@ int bus_unit_method_kill(sd_bus_message *message, void *userdata, sd_bus_error * if (r < 0) return r; + if (startswith(sd_bus_message_get_member(message), "QueueSignal")) { + r = sd_bus_message_read(message, "i", &value); + if (r < 0) + return r; + + code = SI_QUEUE; + } else + code = SI_USER; + if (isempty(swho)) who = KILL_ALL; else { who = kill_who_from_string(swho); if (who < 0) - return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid who argument %s", swho); + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, "Invalid who argument: %s", swho); } if (!SIGNAL_VALID(signo)) return sd_bus_error_set(error, SD_BUS_ERROR_INVALID_ARGS, "Signal number out of range."); + if (code == SI_QUEUE && !((signo >= SIGRTMIN) && (signo <= SIGRTMAX))) + return sd_bus_error_setf(error, SD_BUS_ERROR_INVALID_ARGS, + "Value parameter only accepted for realtime signals (SIGRTMIN…SIGRTMAX), refusing for signal SIG%s.", signal_to_string(signo)); + r = bus_verify_manage_units_async_full( u, "kill", @@ -552,7 +566,7 @@ int bus_unit_method_kill(sd_bus_message *message, void *userdata, sd_bus_error * if (r == 0) return 1; /* No authorization for now, but the async polkit stuff will call us again when it has it */ - r = unit_kill(u, who, signo, error); + r = unit_kill(u, who, signo, code, value, error); if (r < 0) return r; @@ -984,6 +998,11 @@ const sd_bus_vtable bus_unit_vtable[] = { SD_BUS_NO_RESULT, bus_unit_method_kill, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD_WITH_ARGS("QueueSignal", + SD_BUS_ARGS("s", whom, "i", signal, "i", value), + SD_BUS_NO_RESULT, + bus_unit_method_kill, + SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD("ResetFailed", NULL, NULL, diff --git a/src/core/mount.c b/src/core/mount.c index 993dce5118e..95bd04f6e9f 100644 --- a/src/core/mount.c +++ b/src/core/mount.c @@ -2158,12 +2158,12 @@ static void mount_reset_failed(Unit *u) { m->clean_result = MOUNT_SUCCESS; } -static int mount_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) { +static int mount_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) { Mount *m = MOUNT(u); assert(m); - return unit_kill_common(u, who, signo, -1, m->control_pid, error); + return unit_kill_common(u, who, signo, code, value, -1, m->control_pid, error); } static int mount_control_pid(Unit *u) { diff --git a/src/core/org.freedesktop.systemd1.conf b/src/core/org.freedesktop.systemd1.conf index 1cef421db81..7f44c32b839 100644 --- a/src/core/org.freedesktop.systemd1.conf +++ b/src/core/org.freedesktop.systemd1.conf @@ -246,6 +246,10 @@ send_interface="org.freedesktop.systemd1.Manager" send_member="KillUnit"/> + + @@ -390,6 +394,10 @@ send_interface="org.freedesktop.systemd1.Unit" send_member="Kill"/> + + diff --git a/src/core/scope.c b/src/core/scope.c index 914d1cc7446..62f23a9e1ec 100644 --- a/src/core/scope.c +++ b/src/core/scope.c @@ -530,8 +530,8 @@ static void scope_reset_failed(Unit *u) { s->result = SCOPE_SUCCESS; } -static int scope_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) { - return unit_kill_common(u, who, signo, -1, -1, error); +static int scope_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) { + return unit_kill_common(u, who, signo, code, value, -1, -1, error); } static int scope_get_timeout(Unit *u, usec_t *timeout) { diff --git a/src/core/service.c b/src/core/service.c index 9ad3c3d9953..dc5ccfd239d 100644 --- a/src/core/service.c +++ b/src/core/service.c @@ -4596,12 +4596,12 @@ static void service_reset_failed(Unit *u) { s->flush_n_restarts = false; } -static int service_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) { +static int service_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) { Service *s = SERVICE(u); assert(s); - return unit_kill_common(u, who, signo, s->main_pid, s->control_pid, error); + return unit_kill_common(u, who, signo, code, value, s->main_pid, s->control_pid, error); } static int service_main_pid(Unit *u) { diff --git a/src/core/slice.c b/src/core/slice.c index 4824a300d07..eb0ba5e763d 100644 --- a/src/core/slice.c +++ b/src/core/slice.c @@ -247,8 +247,8 @@ static int slice_stop(Unit *u) { return 1; } -static int slice_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) { - return unit_kill_common(u, who, signo, -1, -1, error); +static int slice_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) { + return unit_kill_common(u, who, signo, code, value, -1, -1, error); } static int slice_serialize(Unit *u, FILE *f, FDSet *fds) { diff --git a/src/core/socket.c b/src/core/socket.c index 409d415d8d9..8241ba050bf 100644 --- a/src/core/socket.c +++ b/src/core/socket.c @@ -3299,8 +3299,8 @@ static void socket_trigger_notify(Unit *u, Unit *other) { socket_set_state(s, SOCKET_RUNNING); } -static int socket_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) { - return unit_kill_common(u, who, signo, -1, SOCKET(u)->control_pid, error); +static int socket_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) { + return unit_kill_common(u, who, signo, code, value, -1, SOCKET(u)->control_pid, error); } static int socket_get_timeout(Unit *u, usec_t *timeout) { diff --git a/src/core/swap.c b/src/core/swap.c index 2d25014e5f4..ab901a2cd17 100644 --- a/src/core/swap.c +++ b/src/core/swap.c @@ -1480,8 +1480,8 @@ static void swap_reset_failed(Unit *u) { s->clean_result = SWAP_SUCCESS; } -static int swap_kill(Unit *u, KillWho who, int signo, sd_bus_error *error) { - return unit_kill_common(u, who, signo, -1, SWAP(u)->control_pid, error); +static int swap_kill(Unit *u, KillWho who, int signo, int code, int value, sd_bus_error *error) { + return unit_kill_common(u, who, signo, code, value, -1, SWAP(u)->control_pid, error); } static int swap_get_timeout(Unit *u, usec_t *timeout) { diff --git a/src/core/unit.c b/src/core/unit.c index 78a1f72f7fd..be7b19877f4 100644 --- a/src/core/unit.c +++ b/src/core/unit.c @@ -3785,15 +3785,16 @@ bool unit_will_restart(Unit *u) { return UNIT_VTABLE(u)->will_restart(u); } -int unit_kill(Unit *u, KillWho w, int signo, sd_bus_error *error) { +int unit_kill(Unit *u, KillWho w, int signo, int code, int value, sd_bus_error *error) { assert(u); assert(w >= 0 && w < _KILL_WHO_MAX); assert(SIGNAL_VALID(signo)); + assert(IN_SET(code, SI_USER, SI_QUEUE)); if (!UNIT_VTABLE(u)->kill) return -EOPNOTSUPP; - return UNIT_VTABLE(u)->kill(u, w, signo, error); + return UNIT_VTABLE(u)->kill(u, w, signo, code, value, error); } void unit_notify_cgroup_oom(Unit *u, bool managed_oom) { @@ -3838,21 +3839,48 @@ static int kill_common_log(pid_t pid, int signo, void *userdata) { return 1; } +static int kill_or_sigqueue(pid_t pid, int signo, int code, int value) { + assert(pid > 0); + assert(SIGNAL_VALID(signo)); + + switch (code) { + + case SI_USER: + log_debug("Killing " PID_FMT " with signal SIG%s.", pid, signal_to_string(signo)); + return RET_NERRNO(kill(pid, signo)); + + case SI_QUEUE: + log_debug("Enqueuing value %i to " PID_FMT " on signal SIG%s.", value, pid, signal_to_string(signo)); + return RET_NERRNO(sigqueue(pid, signo, (const union sigval) { .sival_int = value })); + + default: + assert_not_reached(); + } +} + int unit_kill_common( Unit *u, KillWho who, int signo, + int code, + int value, pid_t main_pid, pid_t control_pid, sd_bus_error *error) { - int r = 0; bool killed = false; + int ret = 0, r; /* This is the common implementation for explicit user-requested killing of unit processes, shared by * various unit types. Do not confuse with unit_kill_context(), which is what we use when we want to * stop a service ourselves. */ + assert(u); + assert(who >= 0); + assert(who < _KILL_WHO_MAX); + assert(SIGNAL_VALID(signo)); + assert(IN_SET(code, SI_USER, SI_QUEUE)); + if (IN_SET(who, KILL_MAIN, KILL_MAIN_FAIL)) { if (main_pid < 0) return sd_bus_error_setf(error, BUS_ERROR_NO_SUCH_PROCESS, "%s units have no main processes", unit_type_to_string(u->type)); @@ -3867,71 +3895,85 @@ int unit_kill_common( return sd_bus_error_set_const(error, BUS_ERROR_NO_SUCH_PROCESS, "No control process to kill"); } - if (IN_SET(who, KILL_CONTROL, KILL_CONTROL_FAIL, KILL_ALL, KILL_ALL_FAIL)) - if (control_pid > 0) { - _cleanup_free_ char *comm = NULL; - (void) get_process_comm(control_pid, &comm); + if (control_pid > 0 && + IN_SET(who, KILL_CONTROL, KILL_CONTROL_FAIL, KILL_ALL, KILL_ALL_FAIL)) { + _cleanup_free_ char *comm = NULL; + (void) get_process_comm(control_pid, &comm); - if (kill(control_pid, signo) < 0) { - /* Report this failure both to the logs and to the client */ - sd_bus_error_set_errnof( - error, errno, - "Failed to send signal SIG%s to control process " PID_FMT " (%s): %m", - signal_to_string(signo), control_pid, strna(comm)); - r = log_unit_warning_errno( - u, errno, - "Failed to send signal SIG%s to control process " PID_FMT " (%s) on client request: %m", - signal_to_string(signo), control_pid, strna(comm)); - } else { - log_unit_info(u, "Sent signal SIG%s to control process " PID_FMT " (%s) on client request.", - signal_to_string(signo), control_pid, strna(comm)); - killed = true; - } + r = kill_or_sigqueue(control_pid, signo, code, value); + if (r < 0) { + ret = r; + + /* Report this failure both to the logs and to the client */ + sd_bus_error_set_errnof( + error, r, + "Failed to send signal SIG%s to control process " PID_FMT " (%s): %m", + signal_to_string(signo), control_pid, strna(comm)); + log_unit_warning_errno( + u, r, + "Failed to send signal SIG%s to control process " PID_FMT " (%s) on client request: %m", + signal_to_string(signo), control_pid, strna(comm)); + } else { + log_unit_info(u, "Sent signal SIG%s to control process " PID_FMT " (%s) on client request.", + signal_to_string(signo), control_pid, strna(comm)); + killed = true; } + } - if (IN_SET(who, KILL_MAIN, KILL_MAIN_FAIL, KILL_ALL, KILL_ALL_FAIL)) - if (main_pid > 0) { - _cleanup_free_ char *comm = NULL; - (void) get_process_comm(main_pid, &comm); + if (main_pid > 0 && + IN_SET(who, KILL_MAIN, KILL_MAIN_FAIL, KILL_ALL, KILL_ALL_FAIL)) { - if (kill(main_pid, signo) < 0) { - if (r == 0) - sd_bus_error_set_errnof( - error, errno, - "Failed to send signal SIG%s to main process " PID_FMT " (%s): %m", - signal_to_string(signo), main_pid, strna(comm)); + _cleanup_free_ char *comm = NULL; + (void) get_process_comm(main_pid, &comm); + + r = kill_or_sigqueue(main_pid, signo, code, value); + if (r < 0) { + if (ret == 0) { + ret = r; - r = log_unit_warning_errno( - u, errno, - "Failed to send signal SIG%s to main process " PID_FMT " (%s) on client request: %m", + sd_bus_error_set_errnof( + error, r, + "Failed to send signal SIG%s to main process " PID_FMT " (%s): %m", signal_to_string(signo), main_pid, strna(comm)); - } else { - log_unit_info(u, "Sent signal SIG%s to main process " PID_FMT " (%s) on client request.", - signal_to_string(signo), main_pid, strna(comm)); - killed = true; } + + log_unit_warning_errno( + u, r, + "Failed to send signal SIG%s to main process " PID_FMT " (%s) on client request: %m", + signal_to_string(signo), main_pid, strna(comm)); + + } else { + log_unit_info(u, "Sent signal SIG%s to main process " PID_FMT " (%s) on client request.", + signal_to_string(signo), main_pid, strna(comm)); + killed = true; } + } - if (IN_SET(who, KILL_ALL, KILL_ALL_FAIL) && u->cgroup_path) { + /* Note: if we shall enqueue rather than kill we won't do this via the cgroup mechanism, since it + * doesn't really make much sense (and given that enqueued values are a relatively expensive + * resource, and we shouldn't allow us to be subjects for such allocation sprees) */ + if (IN_SET(who, KILL_ALL, KILL_ALL_FAIL) && u->cgroup_path && code == SI_USER) { _cleanup_set_free_ Set *pid_set = NULL; - int q; /* Exclude the main/control pids from being killed via the cgroup */ pid_set = unit_pid_set(main_pid, control_pid); if (!pid_set) return log_oom(); - q = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, signo, 0, pid_set, kill_common_log, u); - if (q < 0) { - if (!IN_SET(q, -ESRCH, -ENOENT)) { - if (r == 0) + r = cg_kill_recursive(SYSTEMD_CGROUP_CONTROLLER, u->cgroup_path, signo, 0, pid_set, kill_common_log, u); + if (r < 0) { + if (!IN_SET(r, -ESRCH, -ENOENT)) { + if (ret == 0) { + ret = r; + sd_bus_error_set_errnof( - error, q, + error, r, "Failed to send signal SIG%s to auxiliary processes: %m", signal_to_string(signo)); + } - r = log_unit_warning_errno( - u, q, + log_unit_warning_errno( + u, r, "Failed to send signal SIG%s to auxiliary processes on client request: %m", signal_to_string(signo)); } @@ -3940,10 +3982,10 @@ int unit_kill_common( } /* If the "fail" versions of the operation are requested, then complain if the set of processes we killed is empty */ - if (r == 0 && !killed && IN_SET(who, KILL_ALL_FAIL, KILL_CONTROL_FAIL, KILL_MAIN_FAIL)) + if (ret == 0 && !killed && IN_SET(who, KILL_ALL_FAIL, KILL_CONTROL_FAIL, KILL_MAIN_FAIL)) return sd_bus_error_set_const(error, BUS_ERROR_NO_SUCH_PROCESS, "No matching processes to kill"); - return r; + return ret; } int unit_following_set(Unit *u, Set **s) { diff --git a/src/core/unit.h b/src/core/unit.h index 58417ebd0ec..3f8377fbf64 100644 --- a/src/core/unit.h +++ b/src/core/unit.h @@ -619,7 +619,7 @@ typedef struct UnitVTable { int (*stop)(Unit *u); int (*reload)(Unit *u); - int (*kill)(Unit *u, KillWho w, int signo, sd_bus_error *error); + int (*kill)(Unit *u, KillWho w, int signo, int code, int value, sd_bus_error *error); /* Clear out the various runtime/state/cache/logs/configuration data */ int (*clean)(Unit *u, ExecCleanMask m); @@ -889,8 +889,8 @@ int unit_start(Unit *u, ActivationDetails *details); int unit_stop(Unit *u); int unit_reload(Unit *u); -int unit_kill(Unit *u, KillWho w, int signo, sd_bus_error *error); -int unit_kill_common(Unit *u, KillWho who, int signo, pid_t main_pid, pid_t control_pid, sd_bus_error *error); +int unit_kill(Unit *u, KillWho w, int signo, int code, int value, sd_bus_error *error); +int unit_kill_common(Unit *u, KillWho who, int signo, int code, int value, pid_t main_pid, pid_t control_pid, sd_bus_error *error); void unit_notify_cgroup_oom(Unit *u, bool managed_oom); diff --git a/src/test/test-execute.c b/src/test/test-execute.c index f8c64b60426..f6d05afc1bd 100644 --- a/src/test/test-execute.c +++ b/src/test/test-execute.c @@ -73,7 +73,7 @@ static void wait_for_service_finish(Manager *m, Unit *unit) { n = now(CLOCK_MONOTONIC); if (ts + timeout < n) { log_error("Test timeout when testing %s", unit->id); - r = unit_kill(unit, KILL_ALL, SIGKILL, NULL); + r = unit_kill(unit, KILL_ALL, SIGKILL, SI_USER, 0, NULL); if (r < 0) log_error_errno(r, "Failed to kill %s: %m", unit->id); exit(EXIT_FAILURE);