From d9219935aba4436363e0ed8acef5a4437463e602 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 24 Feb 2024 08:55:33 +0800 Subject: [PATCH 1/3] systemctl-util: use strv_free_and_replace at one more place --- src/systemctl/systemctl-util.c | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/systemctl/systemctl-util.c b/src/systemctl/systemctl-util.c index de6f53aafe..2dc92ce283 100644 --- a/src/systemctl/systemctl-util.c +++ b/src/systemctl/systemctl-util.c @@ -762,9 +762,7 @@ int maybe_extend_with_unit_dependencies(sd_bus *bus, char ***list) { if (r < 0) return log_error_errno(r, "Failed to append unit dependencies: %m"); - strv_free(*list); - *list = TAKE_PTR(list_with_deps); - return 0; + return strv_free_and_replace(*list, list_with_deps); } int unit_get_dependencies(sd_bus *bus, const char *name, char ***ret) { From 716a4cdb0e098ebfed366b59c093d5d29e2f54e0 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 24 Feb 2024 09:01:22 +0800 Subject: [PATCH 2/3] systemctl: generalize GetUnitByPIDFD handling --- src/systemctl/systemctl-util.c | 148 ++++++++++++++++++++++++++++ src/systemctl/systemctl-util.h | 3 + src/systemctl/systemctl-whoami.c | 162 ++++--------------------------- 3 files changed, 170 insertions(+), 143 deletions(-) diff --git a/src/systemctl/systemctl-util.c b/src/systemctl/systemctl-util.c index 2dc92ce283..c3da750d64 100644 --- a/src/systemctl/systemctl-util.c +++ b/src/systemctl/systemctl-util.c @@ -18,6 +18,8 @@ #include "glob-util.h" #include "macro.h" #include "path-util.h" +#include "pidref.h" +#include "process-util.h" #include "reboot-util.h" #include "set.h" #include "spawn-ask-password-agent.h" @@ -985,3 +987,149 @@ int halt_now(enum action a) { assert_not_reached(); } } + +int get_unit_by_pid(sd_bus *bus, pid_t pid, char **ret_unit, char **ret_path) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + int r; + + assert(bus); + assert(pid >= 0); /* 0 is accepted by GetUnitByPID for querying our own process. */ + + r = bus_call_method(bus, bus_systemd_mgr, "GetUnitByPID", &error, &reply, "u", (uint32_t) pid); + if (r < 0) { + if (sd_bus_error_has_name(&error, BUS_ERROR_NO_UNIT_FOR_PID)) + return log_error_errno(r, "%s", bus_error_message(&error, r)); + + return log_error_errno(r, + "Failed to get unit that PID " PID_FMT " belongs to: %s", + pid > 0 ? pid : getpid_cached(), + bus_error_message(&error, r)); + } + + _cleanup_free_ char *u = NULL, *p = NULL; + const char *path; + + r = sd_bus_message_read_basic(reply, 'o', &path); + if (r < 0) + return bus_log_parse_error(r); + + if (ret_unit) { + r = unit_name_from_dbus_path(path, &u); + if (r < 0) + return log_error_errno(r, + "Failed to extract unit name from D-Bus object path '%s': %m", + path); + } + + if (ret_path) { + p = strdup(path); + if (!p) + return log_oom(); + } + + if (ret_unit) + *ret_unit = TAKE_PTR(u); + if (ret_path) + *ret_path = TAKE_PTR(p); + + return 0; +} + +static int get_unit_by_pidfd(sd_bus *bus, const PidRef *pid, char **ret_unit, char **ret_path) { + _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; + _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; + int r; + + assert(bus); + assert(pidref_is_set(pid)); + + if (pid->fd < 0) + return -EOPNOTSUPP; + + r = bus_call_method(bus, bus_systemd_mgr, "GetUnitByPIDFD", &error, &reply, "h", pid->fd); + if (r < 0) { + if (sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_METHOD)) + return -EOPNOTSUPP; + + if (sd_bus_error_has_names(&error, BUS_ERROR_NO_UNIT_FOR_PID, BUS_ERROR_NO_SUCH_PROCESS)) + return log_error_errno(r, "%s", bus_error_message(&error, r)); + + return log_error_errno(r, + "Failed to get unit that PID " PID_FMT " belongs to: %s", + pid->pid, bus_error_message(&error, r)); + } + + _cleanup_free_ char *u = NULL, *p = NULL; + const char *path, *unit; + + r = sd_bus_message_read(reply, "os", &path, &unit); + if (r < 0) + return bus_log_parse_error(r); + + if (ret_unit) { + u = strdup(unit); + if (!u) + return log_oom(); + } + + if (ret_path) { + p = strdup(path); + if (!p) + return log_oom(); + } + + if (ret_unit) + *ret_unit = TAKE_PTR(u); + if (ret_path) + *ret_path = TAKE_PTR(p); + + return 0; +} + +int lookup_unit_by_pidref(sd_bus *bus, pid_t pid, char **ret_unit, char **ret_path) { + int r; + + assert(bus); + assert(pid >= 0); /* 0 means our own process */ + + if (arg_transport != BUS_TRANSPORT_LOCAL) + return get_unit_by_pid(bus, pid, ret_unit, ret_path); + + static bool use_pidfd = true; + _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; + + r = pidref_set_pid(&pidref, pid); + if (r < 0) + return log_error_errno(r, + r == -ESRCH ? + "PID " PID_FMT " doesn't exist or is already gone." : + "Failed to create reference to PID " PID_FMT ": %m", + pid); + + if (use_pidfd) { + r = get_unit_by_pidfd(bus, &pidref, ret_unit, ret_path); + if (r != -EOPNOTSUPP) + return r; + + use_pidfd = false; + log_debug_errno(r, "Unable to look up process using pidfd, falling back to pid."); + } + + _cleanup_free_ char *u = NULL, *p = NULL; + + r = get_unit_by_pid(bus, pidref.pid, ret_unit ? &u : NULL, ret_path ? &p : NULL); + if (r < 0) + return r; + + r = pidref_verify(&pidref); + if (r < 0) + return log_error_errno(r, "Failed to verify our reference to PID " PID_FMT ": %m", pidref.pid); + + if (ret_unit) + *ret_unit = TAKE_PTR(u); + if (ret_path) + *ret_path = TAKE_PTR(p); + + return 0; +} diff --git a/src/systemctl/systemctl-util.h b/src/systemctl/systemctl-util.h index 17975e110d..a950dde515 100644 --- a/src/systemctl/systemctl-util.h +++ b/src/systemctl/systemctl-util.h @@ -58,3 +58,6 @@ int mangle_names(const char *operation, char * const *original_names, char ***re UnitFileFlags unit_file_flags_from_args(void); int halt_now(enum action a); + +int get_unit_by_pid(sd_bus *bus, pid_t pid, char **ret_unit, char **ret_path); +int lookup_unit_by_pidref(sd_bus *bus, pid_t pid, char **ret_unit, char **ret_path); diff --git a/src/systemctl/systemctl-whoami.c b/src/systemctl/systemctl-whoami.c index bac72c8973..607f2db047 100644 --- a/src/systemctl/systemctl-whoami.c +++ b/src/systemctl/systemctl-whoami.c @@ -1,149 +1,14 @@ /* SPDX-License-Identifier: LGPL-2.1-or-later */ -#include "bus-common-errors.h" -#include "bus-error.h" -#include "bus-locator.h" #include "format-util.h" #include "parse-util.h" -#include "pidref.h" -#include "process-util.h" #include "systemctl.h" #include "systemctl-util.h" #include "systemctl-whoami.h" -static int get_unit_by_pid(sd_bus *bus, pid_t pid, char **ret) { - _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 *unit = NULL; - const char *path; - int r; - - assert(bus); - assert(pid >= 0); /* 0 is accepted by GetUnitByPID for querying our own process. */ - assert(ret); - - r = bus_call_method(bus, bus_systemd_mgr, "GetUnitByPID", &error, &reply, "u", (uint32_t) pid); - if (r < 0) { - if (sd_bus_error_has_name(&error, BUS_ERROR_NO_UNIT_FOR_PID)) - return log_error_errno(r, "%s", bus_error_message(&error, r)); - - return log_error_errno(r, - "Failed to get unit that PID " PID_FMT " belongs to: %s", - pid > 0 ? pid : getpid_cached(), - bus_error_message(&error, r)); - } - - r = sd_bus_message_read_basic(reply, 'o', &path); - if (r < 0) - return bus_log_parse_error(r); - - r = unit_name_from_dbus_path(path, &unit); - if (r < 0) - return log_error_errno(r, "Failed to extract unit name from D-Bus object path '%s': %m", path); - - *ret = TAKE_PTR(unit); - return 0; -} - -static int lookup_pidfd(sd_bus *bus, const PidRef *pid, char **ret) { - _cleanup_(sd_bus_error_free) sd_bus_error error = SD_BUS_ERROR_NULL; - _cleanup_(sd_bus_message_unrefp) sd_bus_message *reply = NULL; - const char *unit; - int r; - - assert(bus); - assert(pidref_is_set(pid)); - assert(ret); - - if (pid->fd < 0) - return -EOPNOTSUPP; - - r = bus_call_method(bus, bus_systemd_mgr, "GetUnitByPIDFD", &error, &reply, "h", pid->fd); - if (r < 0) { - if (sd_bus_error_has_name(&error, SD_BUS_ERROR_UNKNOWN_METHOD)) - return -EOPNOTSUPP; - - if (sd_bus_error_has_names(&error, BUS_ERROR_NO_UNIT_FOR_PID, BUS_ERROR_NO_SUCH_PROCESS)) - return log_error_errno(r, "%s", bus_error_message(&error, r)); - - return log_error_errno(r, - "Failed to get unit that PID " PID_FMT " belongs to: %s", - pid->pid, bus_error_message(&error, r)); - } - - r = sd_bus_message_read(reply, "os", NULL, &unit); - if (r < 0) - return bus_log_parse_error(r); - - char *u = strdup(unit); - if (!u) - return log_oom(); - - *ret = TAKE_PTR(u); - - return 0; -} - -static int lookup_pid(sd_bus *bus, const char *pidstr) { - _cleanup_free_ char *unit = NULL; - int r; - - assert(bus); - assert(pidstr); - - if (arg_transport == BUS_TRANSPORT_LOCAL) { - static bool use_pidfd = true; - _cleanup_(pidref_done) PidRef pid = PIDREF_NULL; - - r = pidref_set_pidstr(&pid, pidstr); - if (r < 0) - return log_error_errno(r, - r == -ESRCH ? - "PID %s doesn't exist or is already gone." : - "Failed to create reference to PID %s: %m", - pidstr); - - if (use_pidfd) { - r = lookup_pidfd(bus, &pid, &unit); - if (r == -EOPNOTSUPP) { - use_pidfd = false; - log_debug_errno(r, "Unable to look up process using pidfd, ignoring."); - } else if (r < 0) - return r; - } - - if (!use_pidfd) { - assert(!unit); - - r = get_unit_by_pid(bus, pid.pid, &unit); - if (r < 0) - return r; - - r = pidref_verify(&pid); - if (r < 0) - return log_error_errno(r, - "Failed to verify our reference to PID " PID_FMT ": %m", - pid.pid); - } - } else { - pid_t pid; - - r = parse_pid(pidstr, &pid); - if (r < 0) - return log_error_errno(r, "Failed to parse PID %s: %m", pidstr); - - r = get_unit_by_pid(bus, pid, &unit); - if (r < 0) - return r; - } - - puts(unit); - return 0; -} - int verb_whoami(int argc, char *argv[], void *userdata) { sd_bus *bus; - int r; + int r, ret = 0; r = acquire_bus(BUS_FULL, &bus); if (r < 0) @@ -153,11 +18,12 @@ int verb_whoami(int argc, char *argv[], void *userdata) { _cleanup_free_ char *unit = NULL; if (arg_transport != BUS_TRANSPORT_LOCAL) - return log_error_errno(SYNTHETIC_ERRNO(EREMOTE), "Refusing to look up our local PID on remote host."); + return log_error_errno(SYNTHETIC_ERRNO(EREMOTE), + "Refusing to look up our local PID on remote host."); - /* Our own process can never go away while querying, hence no need to open pidfd. */ + /* Our own process can never go away while querying, hence no need to go through pidfd. */ - r = get_unit_by_pid(bus, 0, &unit); + r = get_unit_by_pid(bus, 0, &unit, /* ret_path = */ NULL); if (r < 0) return r; @@ -165,10 +31,20 @@ int verb_whoami(int argc, char *argv[], void *userdata) { return 0; } - r = 0; + STRV_FOREACH(pidstr, strv_skip(argv, 1)) { + _cleanup_free_ char *unit = NULL; + pid_t pid; - STRV_FOREACH(pid, strv_skip(argv, 1)) - RET_GATHER(r, lookup_pid(bus, *pid)); + r = parse_pid(*pidstr, &pid); + if (r < 0) + return log_error_errno(r, "Invalid PID specified: %s", *pidstr); - return r; + r = lookup_unit_by_pidref(bus, pid, &unit, /* ret_path = */ NULL); + if (r < 0) + RET_GATHER(ret, r); + else + puts(unit); + } + + return ret; } From d95705cc883bf18eccc9f7ed31c25a9bb9e33850 Mon Sep 17 00:00:00 2001 From: Mike Yuan Date: Sat, 24 Feb 2024 09:39:32 +0800 Subject: [PATCH 3/3] systemctl-show: use lookup_unit_by_pidref too Follow-up for e0e7bc8223c3f28fcb48db9f0f003d9f03ca46d7 This allows us to pin the process locally when GetUnitByPIDFD is not available, just like what we have been doing for 'systemctl whoami'. Also, fix looking up remote pid. We can't use pidfd for those. --- src/systemctl/systemctl-show.c | 94 +--------------------------------- 1 file changed, 2 insertions(+), 92 deletions(-) diff --git a/src/systemctl/systemctl-show.c b/src/systemctl/systemctl-show.c index 999e08b16e..7926bfbeca 100644 --- a/src/systemctl/systemctl-show.c +++ b/src/systemctl/systemctl-show.c @@ -2200,96 +2200,6 @@ static int show_one( return 0; } -static int get_unit_dbus_path_by_pid_fallback( - 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; - 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", &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 = -EBADF; - 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) && !ERRNO_IS_PRIVILEGE(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); - - path = strdup(p); - if (!path) - return log_oom(); - - unit = strdup(u); - if (!unit) - return log_oom(); - - *ret_unit = TAKE_PTR(unit); - *ret_path = TAKE_PTR(path); - - return 0; -} - static int show_all( sd_bus *bus, SystemctlShowMode show_mode, @@ -2461,9 +2371,9 @@ int verb_show(int argc, char *argv[], void *userdata) { } else { /* Interpret as PID */ - r = get_unit_dbus_path_by_pid(bus, id, &path, &unit); + r = lookup_unit_by_pidref(bus, (pid_t) id, &unit, &path); if (r < 0) { - ret = r; + RET_GATHER(ret, r); continue; } }