From: Lennart Poettering <lennart@poettering.net>
Date: Tue, 17 Oct 2023 10:12:05 +0000 (+0200)
Subject: process-util: change pid_is_alive() to not eat up errors, and add pidref_is_alive()
X-Git-Tag: v255-rc1~209^2~7
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=becdfcb9f1cb555c50dcfe51894cb0b155f7f01e;p=thirdparty%2Fsystemd.git

process-util: change pid_is_alive() to not eat up errors, and add pidref_is_alive()

Let's no eat up errors, but propagate unexpected ones.
---

diff --git a/src/basic/process-util.c b/src/basic/process-util.c
index f381b6369cd..2414f263dd2 100644
--- a/src/basic/process-util.c
+++ b/src/basic/process-util.c
@@ -1043,13 +1043,13 @@ bool pid_is_unwaited(pid_t pid) {
         return errno != ESRCH;
 }
 
-bool pid_is_alive(pid_t pid) {
+int pid_is_alive(pid_t pid) {
         int r;
 
         /* Checks whether a PID is still valid and not a zombie */
 
         if (pid < 0)
-                return false;
+                return -ESRCH;
 
         if (pid <= 1) /* If we or PID 1 would be a zombie, this code would not be running */
                 return true;
@@ -1058,10 +1058,31 @@ bool pid_is_alive(pid_t pid) {
                 return true;
 
         r = get_process_state(pid);
-        if (IN_SET(r, -ESRCH, 'Z'))
+        if (r == -ESRCH)
                 return false;
+        if (r < 0)
+                return r;
+
+        return r != 'Z';
+}
+
+int pidref_is_alive(const PidRef *pidref) {
+        int r, result;
+
+        if (!pidref_is_set(pidref))
+                return -ESRCH;
 
-        return true;
+        result = pid_is_alive(pidref->pid);
+        if (result < 0)
+                return result;
+
+        r = pidref_verify(pidref);
+        if (r == -ESRCH)
+                return false;
+        if (r < 0)
+                return r;
+
+        return result;
 }
 
 int pid_from_same_root_fs(pid_t pid) {
diff --git a/src/basic/process-util.h b/src/basic/process-util.h
index 53dce1ee71f..7824127a6de 100644
--- a/src/basic/process-util.h
+++ b/src/basic/process-util.h
@@ -86,7 +86,8 @@ int pidref_is_kernel_thread(const PidRef *pid);
 
 int getenv_for_pid(pid_t pid, const char *field, char **_value);
 
-bool pid_is_alive(pid_t pid);
+int pid_is_alive(pid_t pid);
+int pidref_is_alive(const PidRef *pidref);
 bool pid_is_unwaited(pid_t pid);
 int pid_is_my_child(pid_t pid);
 int pid_from_same_root_fs(pid_t pid);
diff --git a/src/core/service.c b/src/core/service.c
index bb268eb9397..a2f8bc7c870 100644
--- a/src/core/service.c
+++ b/src/core/service.c
@@ -1053,6 +1053,7 @@ static void service_dump(Unit *u, FILE *f, const char *prefix) {
 
 static int service_is_suitable_main_pid(Service *s, PidRef *pid, int prio) {
         Unit *owner;
+        int r;
 
         assert(s);
         assert(pidref_is_set(pid));
@@ -1067,8 +1068,11 @@ static int service_is_suitable_main_pid(Service *s, PidRef *pid, int prio) {
         if (pidref_equal(pid, &s->control_pid))
                 return log_unit_full_errno(UNIT(s), prio, SYNTHETIC_ERRNO(EPERM), "New main PID "PID_FMT" is the control process, refusing.", pid->pid);
 
-        if (!pid_is_alive(pid->pid))
+        r = pidref_is_alive(pid);
+        if (r == 0)
                 return log_unit_full_errno(UNIT(s), prio, SYNTHETIC_ERRNO(ESRCH), "New main PID "PID_FMT" does not exist or is a zombie.", pid->pid);
+        if (r < 0)
+                return log_unit_full_errno(UNIT(s), prio, r, "Failed to check if main PID "PID_FMT" exists or is a zombie.", pid->pid);
 
         owner = manager_get_unit_by_pidref(UNIT(s)->manager, pid);
         if (owner == UNIT(s)) {
@@ -1827,7 +1831,7 @@ static int main_pid_good(Service *s) {
 
                 /* If it's an alien child let's check if it is still alive ... */
                 if (s->main_pid_alien && pidref_is_set(&s->main_pid))
-                        return pid_is_alive(s->main_pid.pid);
+                        return pidref_is_alive(&s->main_pid);
 
                 /* .. otherwise assume we'll get a SIGCHLD for it, which we really should wait for to collect
                  * exit status and code */
diff --git a/src/libsystemd/sd-bus/bus-creds.c b/src/libsystemd/sd-bus/bus-creds.c
index 48990a27f09..64af18de9e5 100644
--- a/src/libsystemd/sd-bus/bus-creds.c
+++ b/src/libsystemd/sd-bus/bus-creds.c
@@ -139,7 +139,7 @@ sd_bus_creds* bus_creds_new(void) {
 }
 
 _public_ int sd_bus_creds_new_from_pid(sd_bus_creds **ret, pid_t pid, uint64_t mask) {
-        sd_bus_creds *c;
+        _cleanup_(sd_bus_creds_unrefp) sd_bus_creds *c = NULL;
         int r;
 
         assert_return(pid >= 0, -EINVAL);
@@ -154,19 +154,18 @@ _public_ int sd_bus_creds_new_from_pid(sd_bus_creds **ret, pid_t pid, uint64_t m
                 return -ENOMEM;
 
         r = bus_creds_add_more(c, mask | SD_BUS_CREDS_AUGMENT, pid, 0);
-        if (r < 0) {
-                sd_bus_creds_unref(c);
+        if (r < 0)
                 return r;
-        }
 
         /* Check if the process existed at all, in case we haven't
          * figured that out already */
-        if (!pid_is_alive(pid)) {
-                sd_bus_creds_unref(c);
+        r = pid_is_alive(pid);
+        if (r < 0)
+                return r;
+        if (r == 0)
                 return -ESRCH;
-        }
 
-        *ret = c;
+        *ret = TAKE_PTR(c);
         return 0;
 }
 
@@ -1089,12 +1088,13 @@ int bus_creds_add_more(sd_bus_creds *c, uint64_t mask, pid_t pid, pid_t tid) {
                         c->mask |= SD_BUS_CREDS_TTY;
         }
 
-        /* In case only the exe path was to be read we cannot
-         * distinguish the case where the exe path was unreadable
-         * because the process was a kernel thread, or when the
-         * process didn't exist at all. Hence, let's do a final check,
-         * to be sure. */
-        if (!pid_is_alive(pid))
+        /* In case only the exe path was to be read we cannot distinguish the case where the exe path was
+         * unreadable because the process was a kernel thread, or when the process didn't exist at
+         * all. Hence, let's do a final check, to be sure. */
+        r = pid_is_alive(pid);
+        if (r < 0)
+                return r;
+        if (r == 0)
                 return -ESRCH;
 
         if (tid > 0 && tid != pid && !pid_is_unwaited(tid))
diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
index 6f548ecae9c..e6dea2f05f0 100644
--- a/src/test/test-process-util.c
+++ b/src/test/test-process-util.c
@@ -224,11 +224,11 @@ TEST(pid_is_alive) {
         } else {
                 int status;
 
-                waitpid(pid, &status, 0);
-                assert_se(!pid_is_alive(pid));
+                assert_se(waitpid(pid, &status, 0) == pid);
+                assert_se(pid_is_alive(pid) == 0);
         }
-        assert_se(pid_is_alive(getpid_cached()));
-        assert_se(!pid_is_alive(-1));
+        assert_se(pid_is_alive(getpid_cached()) > 0);
+        assert_se(pid_is_alive(-1) < 0);
 }
 
 TEST(personality) {
diff --git a/src/tty-ask-password-agent/tty-ask-password-agent.c b/src/tty-ask-password-agent/tty-ask-password-agent.c
index 87c610bc73b..3a30bfe042e 100644
--- a/src/tty-ask-password-agent/tty-ask-password-agent.c
+++ b/src/tty-ask-password-agent/tty-ask-password-agent.c
@@ -207,7 +207,7 @@ static int process_one_password_file(const char *filename) {
         if (not_after > 0 && now(CLOCK_MONOTONIC) > not_after)
                 return 0;
 
-        if (pid > 0 && !pid_is_alive(pid))
+        if (pid > 0 && pid_is_alive(pid) <= 0)
                 return 0;
 
         switch (arg_action) {