diff options
author | Luca Boccassi <bluca@debian.org> | 2023-01-17 00:46:01 +0100 |
---|---|---|
committer | Lennart Poettering <lennart@poettering.net> | 2023-01-18 10:58:46 +0100 |
commit | e0e7bc8223c3f28fcb48db9f0f003d9f03ca46d7 (patch) | |
tree | b4cd11a3508c2ec92080472aa7ca4391c8ff138a | |
parent | hwdb: Add mic mute key mappings for Dell G16 Series (diff) | |
download | systemd-e0e7bc8223c3f28fcb48db9f0f003d9f03ca46d7.tar.xz systemd-e0e7bc8223c3f28fcb48db9f0f003d9f03ca46d7.zip |
core: add GetUnitByPIDFD method and use it in systemctl
A pid can be recycled, but a pidfd is pinned. Add a new method that is safer
as it takes a pidfd as input.
Return not only the D-Bus object path, but also the unit id and the last
recorded invocation id, as they are both useful (especially the id, as
converting from a path object to a unit id from a script requires another
round-trip via D-Bus).
Note that the manager still tracks processes by pid, so theorethically this
is not fully error-proof, but on the other hand the method response is
synchronous and the manager is single-threaded, so once a call is being
processed the unit database will not change anyway. Once the manager
switches to use pidfds everywhere, this can be further hardened.
-rw-r--r-- | man/org.freedesktop.systemd1.xml | 12 | ||||
-rw-r--r-- | src/core/dbus-manager.c | 62 | ||||
-rw-r--r-- | src/core/org.freedesktop.systemd1.conf | 4 | ||||
-rw-r--r-- | src/systemctl/systemctl-show.c | 85 | ||||
-rwxr-xr-x | test/units/testsuite-26.sh | 1 |
5 files changed, 151 insertions, 13 deletions
diff --git a/man/org.freedesktop.systemd1.xml b/man/org.freedesktop.systemd1.xml index 8eb30afd85..bbb6b14339 100644 --- a/man/org.freedesktop.systemd1.xml +++ b/man/org.freedesktop.systemd1.xml @@ -65,6 +65,10 @@ node /org/freedesktop/systemd1 { out o unit); GetUnitByControlGroup(in s cgroup, out o unit); + GetUnitByPIDFD(in h pidfd, + out o unit, + out s unit_id, + out ay invocation_id); LoadUnit(in s name, out o unit); StartUnit(in s name, @@ -796,6 +800,8 @@ node /org/freedesktop/systemd1 { <variablelist class="dbus-method" generated="True" extra-ref="GetUnitByControlGroup()"/> + <variablelist class="dbus-method" generated="True" extra-ref="GetUnitByPIDFD()"/> + <variablelist class="dbus-method" generated="True" extra-ref="LoadUnit()"/> <variablelist class="dbus-method" generated="True" extra-ref="StartUnit()"/> @@ -1219,7 +1225,11 @@ node /org/freedesktop/systemd1 { will fail.</para> <para><function>GetUnitByPID()</function> may be used to get the unit object path of the unit a process - ID belongs to. It takes a UNIX PID and returns the object path. The PID must refer to an existing system process.</para> + ID belongs to. It takes a UNIX PID and returns the object path. The PID must refer to an existing system process. + <function>GetUnitByPIDFD()</function> may be used to query with a Linux PIDFD (see: + <citerefentry><refentrytitle>pidfd_open</refentrytitle><manvolnum>2</manvolnum></citerefentry>) instead + of a PID, which is safer as UNIX PIDs can be recycled. The latter method returns the unit id and the + invocation id together with the unit object path.</para> <para><function>LoadUnit()</function> is similar to <function>GetUnit()</function> but will load the unit from disk if possible.</para> diff --git a/src/core/dbus-manager.c b/src/core/dbus-manager.c index 6109ab6ce0..e4a4b69541 100644 --- a/src/core/dbus-manager.c +++ b/src/core/dbus-manager.c @@ -653,6 +653,63 @@ static int method_get_unit_by_control_group(sd_bus_message *message, void *userd return reply_unit_path(u, message, error); } +static int method_get_unit_by_pidfd(sd_bus_message *message, void *userdata, sd_bus_error *error) { + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + Manager *m = ASSERT_PTR(userdata); + _cleanup_free_ char *path = NULL; + int r, pidfd; + pid_t pid_early, pid_late; + Unit *u; + + assert(message); + + r = sd_bus_message_read(message, "h", &pidfd); + if (r < 0) + return r; + + r = pidfd_get_pid(pidfd, &pid_early); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to get PID from PIDFD: %m"); + + u = manager_get_unit_by_pid(m, pid_early); + if (!u) + return sd_bus_error_setf(error, BUS_ERROR_NO_UNIT_FOR_PID, "PID "PID_FMT" does not belong to any loaded unit.", pid_early); + + r = mac_selinux_unit_access_check(u, message, "status", error); + if (r < 0) + return r; + + path = unit_dbus_path(u); + if (!path) + return log_oom(); + + r = sd_bus_message_new_method_return(message, &reply); + if (r < 0) + return r; + + r = sd_bus_message_append(reply, "os", path, u->id); + if (r < 0) + return r; + + r = sd_bus_message_append_array(reply, 'y', u->invocation_id.bytes, sizeof(u->invocation_id.bytes)); + if (r < 0) + return r; + + /* Double-check that the process is still alive and that the PID did not change before returning the + * answer. */ + r = pidfd_get_pid(pidfd, &pid_late); + if (r < 0) + return sd_bus_error_set_errnof(error, r, "Failed to get PID from PIDFD: %m"); + if (pid_early != pid_late) + return sd_bus_error_setf(error, + BUS_ERROR_NO_SUCH_PROCESS, + "The PIDFD's PID "PID_FMT" changed to "PID_FMT" during the lookup operation.", + pid_early, + pid_late); + + return sd_bus_send(NULL, reply, NULL); +} + static int method_load_unit(sd_bus_message *message, void *userdata, sd_bus_error *error) { Manager *m = ASSERT_PTR(userdata); const char *name; @@ -2911,6 +2968,11 @@ const sd_bus_vtable bus_manager_vtable[] = { SD_BUS_RESULT("o", unit), method_get_unit_by_control_group, SD_BUS_VTABLE_UNPRIVILEGED), + SD_BUS_METHOD_WITH_ARGS("GetUnitByPIDFD", + SD_BUS_ARGS("h", pidfd), + SD_BUS_RESULT("o", unit, "s", unit_id, "ay", invocation_id), + method_get_unit_by_pidfd, + SD_BUS_VTABLE_UNPRIVILEGED), SD_BUS_METHOD_WITH_ARGS("LoadUnit", SD_BUS_ARGS("s", name), SD_BUS_RESULT("o", unit), diff --git a/src/core/org.freedesktop.systemd1.conf b/src/core/org.freedesktop.systemd1.conf index fe9de012b6..1cef421db8 100644 --- a/src/core/org.freedesktop.systemd1.conf +++ b/src/core/org.freedesktop.systemd1.conf @@ -66,6 +66,10 @@ <allow send_destination="org.freedesktop.systemd1" send_interface="org.freedesktop.systemd1.Manager" + send_member="GetUnitByPIDFD"/> + + <allow send_destination="org.freedesktop.systemd1" + send_interface="org.freedesktop.systemd1.Manager" send_member="LoadUnit"/> <allow send_destination="org.freedesktop.systemd1" diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index 4166b361dd..db489a671e 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -13,6 +13,7 @@ #include "errno-util.h" #include "exec-util.h" #include "exit-status.h" +#include "fd-util.h" #include "format-util.h" #include "hexdecoct.h" #include "hostname-util.h" @@ -2100,29 +2101,93 @@ static int show_one( return 0; } -static int get_unit_dbus_path_by_pid( +static int get_unit_dbus_path_by_pid_fallback( sd_bus *bus, uint32_t pid, - char **unit) { + char **ret_path, + char **ret_unit) { _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - char *u; + _cleanup_free_ char *path = NULL, *unit = NULL; + char *p; int r; + assert(bus); + assert(ret_path); + assert(ret_unit); + r = bus_call_method(bus, bus_systemd_mgr, "GetUnitByPID", &error, &reply, "u", pid); if (r < 0) return log_error_errno(r, "Failed to get unit for PID %"PRIu32": %s", pid, bus_error_message(&error, r)); - r = sd_bus_message_read(reply, "o", &u); + r = sd_bus_message_read(reply, "o", &p); + if (r < 0) + return bus_log_parse_error(r); + + path = strdup(p); + if (!path) + return log_oom(); + + r = unit_name_from_dbus_path(path, &unit); + if (r < 0) + return log_oom(); + + *ret_unit = TAKE_PTR(unit); + *ret_path = TAKE_PTR(path); + + return 0; +} + +static int get_unit_dbus_path_by_pid( + sd_bus *bus, + uint32_t pid, + char **ret_path, + char **ret_unit) { + + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + _cleanup_free_ char *path = NULL, *unit = NULL; + _cleanup_close_ int pidfd = -EBADFD; + char *p, *u; + int r; + + assert(bus); + assert(ret_path); + assert(ret_unit); + + /* First, try to send a PIDFD across the wire, so that we can pin the process and there's no race + * condition possible while we wait for the D-Bus reply. If we either don't have PIDFD support in + * the kernel or the new D-Bus method is not available, then fallback to the older method that + * sends the numeric PID. */ + + pidfd = pidfd_open(pid, 0); + if (pidfd < 0 && ERRNO_IS_NOT_SUPPORTED(errno)) + return get_unit_dbus_path_by_pid_fallback(bus, pid, ret_path, ret_unit); + if (pidfd < 0) + return log_error_errno(errno, "Failed to open PID %"PRIu32": %m", pid); + + r = bus_call_method(bus, bus_systemd_mgr, "GetUnitByPIDFD", &error, &reply, "h", pidfd); + if (r < 0 && sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_METHOD)) + return get_unit_dbus_path_by_pid_fallback(bus, pid, ret_path, ret_unit); + if (r < 0) + return log_error_errno(r, "Failed to get unit for PID %"PRIu32": %s", pid, bus_error_message(&error, r)); + + r = sd_bus_message_read(reply, "os", &p, &u); if (r < 0) return bus_log_parse_error(r); - u = strdup(u); - if (!u) + path = strdup(p); + if (!path) + return log_oom(); + + unit = strdup(u); + if (!unit) return log_oom(); - *unit = u; + *ret_unit = TAKE_PTR(unit); + *ret_path = TAKE_PTR(path); + return 0; } @@ -2297,15 +2362,11 @@ int verb_show(int argc, char *argv[], void *userdata) { } else { /* Interpret as PID */ - r = get_unit_dbus_path_by_pid(bus, id, &path); + r = get_unit_dbus_path_by_pid(bus, id, &path, &unit); if (r < 0) { ret = r; continue; } - - r = unit_name_from_dbus_path(path, &unit); - if (r < 0) - return log_oom(); } r = show_one(bus, path, unit, show_mode, &new_line, &ellipsized); diff --git a/test/units/testsuite-26.sh b/test/units/testsuite-26.sh index 09470a6f06..ee84447d90 100755 --- a/test/units/testsuite-26.sh +++ b/test/units/testsuite-26.sh @@ -245,6 +245,7 @@ systemctl status "systemd-*.timer" systemctl status "systemd-journald*.socket" systemctl status "sys-devices-*-ttyS0.device" systemctl status -- -.mount +systemctl status 1 # --marked systemctl restart "$UNIT_NAME" |