You can not select more than 25 topics
Topics must start with a letter or number, can include dashes ('-') and can be up to 35 characters long.
224 lines
7.8 KiB
224 lines
7.8 KiB
From 9b30c003c8f80bf44f18168d07ea11c48e6d8864 Mon Sep 17 00:00:00 2001
|
|
From: Lennart Poettering <lennart@poettering.net>
|
|
Date: Wed, 7 Jul 2021 15:57:51 +0200
|
|
Subject: [PATCH] process-util: explicitly handle processes lacking parents in
|
|
get_process_ppid()
|
|
|
|
Let's make sure we signal out-of-band via an error message if a process
|
|
doesn't have a parent process whose PID we could return. Otherwise we'll
|
|
too likely hide errors, as we return an invalid PID 0, which in other
|
|
contexts has special meaning (i.e. usually "myself").
|
|
|
|
Replaces: #20153
|
|
|
|
This is based on work by @dtardon, but goes a different route, by
|
|
ensuring we propagate a proper error in this case.
|
|
|
|
This modernizes the function in question a bit in other ways, i.e.
|
|
renames stuff and makes the return parameter optional.
|
|
|
|
(cherry picked from commit 0c4d1e6d96a549054bfe0597d197f829838917f1)
|
|
|
|
Resolves: #1977569
|
|
---
|
|
src/basic/process-util.c | 27 +++++++++++++-------
|
|
src/coredump/coredump.c | 23 +++++++++--------
|
|
src/test/test-process-util.c | 48 +++++++++++++++++++++++++++++++++---
|
|
3 files changed, 74 insertions(+), 24 deletions(-)
|
|
|
|
diff --git a/src/basic/process-util.c b/src/basic/process-util.c
|
|
index 0a4a747ba4..6016d83d41 100644
|
|
--- a/src/basic/process-util.c
|
|
+++ b/src/basic/process-util.c
|
|
@@ -603,20 +603,23 @@ int get_process_environ(pid_t pid, char **env) {
|
|
return 0;
|
|
}
|
|
|
|
-int get_process_ppid(pid_t pid, pid_t *_ppid) {
|
|
- int r;
|
|
+int get_process_ppid(pid_t pid, pid_t *ret) {
|
|
_cleanup_free_ char *line = NULL;
|
|
long unsigned ppid;
|
|
const char *p;
|
|
+ int r;
|
|
|
|
assert(pid >= 0);
|
|
- assert(_ppid);
|
|
|
|
if (pid == 0 || pid == getpid_cached()) {
|
|
- *_ppid = getppid();
|
|
+ if (ret)
|
|
+ *ret = getppid();
|
|
return 0;
|
|
}
|
|
|
|
+ if (pid == 1) /* PID 1 has no parent, shortcut this case */
|
|
+ return -EADDRNOTAVAIL;
|
|
+
|
|
p = procfs_file_alloca(pid, "stat");
|
|
r = read_one_line_file(p, &line);
|
|
if (r == -ENOENT)
|
|
@@ -624,9 +627,8 @@ int get_process_ppid(pid_t pid, pid_t *_ppid) {
|
|
if (r < 0)
|
|
return r;
|
|
|
|
- /* Let's skip the pid and comm fields. The latter is enclosed
|
|
- * in () but does not escape any () in its value, so let's
|
|
- * skip over it manually */
|
|
+ /* Let's skip the pid and comm fields. The latter is enclosed in () but does not escape any () in its
|
|
+ * value, so let's skip over it manually */
|
|
|
|
p = strrchr(line, ')');
|
|
if (!p)
|
|
@@ -640,10 +642,17 @@ int get_process_ppid(pid_t pid, pid_t *_ppid) {
|
|
&ppid) != 1)
|
|
return -EIO;
|
|
|
|
- if ((long unsigned) (pid_t) ppid != ppid)
|
|
+ /* If ppid is zero the process has no parent. Which might be the case for PID 1 but also for
|
|
+ * processes originating in other namespaces that are inserted into a pidns. Return a recognizable
|
|
+ * error in this case. */
|
|
+ if (ppid == 0)
|
|
+ return -EADDRNOTAVAIL;
|
|
+
|
|
+ if ((pid_t) ppid < 0 || (long unsigned) (pid_t) ppid != ppid)
|
|
return -ERANGE;
|
|
|
|
- *_ppid = (pid_t) ppid;
|
|
+ if (ret)
|
|
+ *ret = (pid_t) ppid;
|
|
|
|
return 0;
|
|
}
|
|
diff --git a/src/coredump/coredump.c b/src/coredump/coredump.c
|
|
index 2a130e8838..fb3a6ecfe9 100644
|
|
--- a/src/coredump/coredump.c
|
|
+++ b/src/coredump/coredump.c
|
|
@@ -591,8 +591,7 @@ static int get_process_ns(pid_t pid, const char *namespace, ino_t *ns) {
|
|
return 0;
|
|
}
|
|
|
|
-static int get_mount_namespace_leader(pid_t pid, pid_t *container_pid) {
|
|
- pid_t cpid = pid, ppid = 0;
|
|
+static int get_mount_namespace_leader(pid_t pid, pid_t *ret) {
|
|
ino_t proc_mntns;
|
|
int r = 0;
|
|
|
|
@@ -602,8 +601,12 @@ static int get_mount_namespace_leader(pid_t pid, pid_t *container_pid) {
|
|
|
|
for (;;) {
|
|
ino_t parent_mntns;
|
|
+ pid_t ppid;
|
|
|
|
- r = get_process_ppid(cpid, &ppid);
|
|
+ r = get_process_ppid(pid, &ppid);
|
|
+ if (r == -EADDRNOTAVAIL) /* Reached the top (i.e. typically PID 1, but could also be a process
|
|
+ * whose parent is not in our pidns) */
|
|
+ return -ENOENT;
|
|
if (r < 0)
|
|
return r;
|
|
|
|
@@ -611,17 +614,13 @@ static int get_mount_namespace_leader(pid_t pid, pid_t *container_pid) {
|
|
if (r < 0)
|
|
return r;
|
|
|
|
- if (proc_mntns != parent_mntns)
|
|
- break;
|
|
-
|
|
- if (ppid == 1)
|
|
- return -ENOENT;
|
|
+ if (proc_mntns != parent_mntns) {
|
|
+ *ret = ppid;
|
|
+ return 0;
|
|
+ }
|
|
|
|
- cpid = ppid;
|
|
+ pid = ppid;
|
|
}
|
|
-
|
|
- *container_pid = ppid;
|
|
- return 0;
|
|
}
|
|
|
|
/* Returns 1 if the parent was found.
|
|
diff --git a/src/test/test-process-util.c b/src/test/test-process-util.c
|
|
index 26e3247993..6b14ff592b 100644
|
|
--- a/src/test/test-process-util.c
|
|
+++ b/src/test/test-process-util.c
|
|
@@ -19,6 +19,7 @@
|
|
#include "macro.h"
|
|
#include "parse-util.h"
|
|
#include "process-util.h"
|
|
+#include "procfs-util.h"
|
|
#include "signal-util.h"
|
|
#include "stdio-util.h"
|
|
#include "string-util.h"
|
|
@@ -56,9 +57,12 @@ static void test_get_process_comm(pid_t pid) {
|
|
assert_se(get_process_cmdline(pid, 1, false, &d) >= 0);
|
|
log_info("PID"PID_FMT" cmdline truncated to 1: '%s'", pid, d);
|
|
|
|
- assert_se(get_process_ppid(pid, &e) >= 0);
|
|
- log_info("PID"PID_FMT" PPID: "PID_FMT, pid, e);
|
|
- assert_se(pid == 1 ? e == 0 : e > 0);
|
|
+ r = get_process_ppid(pid, &e);
|
|
+ assert_se(pid == 1 ? r == -EADDRNOTAVAIL : r >= 0);
|
|
+ if (r >= 0) {
|
|
+ log_info("PID"PID_FMT" PPID: "PID_FMT, pid, e);
|
|
+ assert_se(e > 0);
|
|
+ }
|
|
|
|
assert_se(is_kernel_thread(pid) == 0 || pid != 1);
|
|
|
|
@@ -585,6 +589,43 @@ static void test_ioprio_class_from_to_string(void) {
|
|
test_ioprio_class_from_to_string_one("-1", -1);
|
|
}
|
|
|
|
+static void test_get_process_ppid(void) {
|
|
+ uint64_t limit;
|
|
+ int r;
|
|
+
|
|
+ log_info("/* %s */", __func__);
|
|
+
|
|
+ assert_se(get_process_ppid(1, NULL) == -EADDRNOTAVAIL);
|
|
+
|
|
+ /* the process with the PID above the global limit definitely doesn't exist. Verify that */
|
|
+ assert_se(procfs_tasks_get_limit(&limit) >= 0);
|
|
+ assert_se(limit >= INT_MAX || get_process_ppid(limit+1, NULL) == -ESRCH);
|
|
+
|
|
+ for (pid_t pid = 0;;) {
|
|
+ _cleanup_free_ char *c1 = NULL, *c2 = NULL;
|
|
+ pid_t ppid;
|
|
+
|
|
+ r = get_process_ppid(pid, &ppid);
|
|
+ if (r == -EADDRNOTAVAIL) {
|
|
+ log_info("No further parent PID");
|
|
+ break;
|
|
+ }
|
|
+
|
|
+ assert_se(r >= 0);
|
|
+
|
|
+ /* NOTE: The size is SIZE_MAX in the original commit, but it would require backporting a
|
|
+ * lot more stuff to support that (the current version of get_process_cmdline() just fails with
|
|
+ * ENOMEM). UINT16_MAX should be enough for practical purposes.
|
|
+ */
|
|
+ assert_se(get_process_cmdline(pid, UINT16_MAX, true, &c1) >= 0);
|
|
+ assert_se(get_process_cmdline(ppid, UINT16_MAX, true, &c2) >= 0);
|
|
+
|
|
+ log_info("Parent of " PID_FMT " (%s) is " PID_FMT " (%s).", pid, c1, ppid, c2);
|
|
+
|
|
+ pid = ppid;
|
|
+ }
|
|
+}
|
|
+
|
|
int main(int argc, char *argv[]) {
|
|
log_set_max_level(LOG_DEBUG);
|
|
log_parse_environment();
|
|
@@ -614,6 +655,7 @@ int main(int argc, char *argv[]) {
|
|
test_safe_fork();
|
|
test_pid_to_ptr();
|
|
test_ioprio_class_from_to_string();
|
|
+ test_get_process_ppid();
|
|
|
|
return 0;
|
|
}
|