From 00f1714311c38ff1de6bbb96a33cc6448920bce2 Mon Sep 17 00:00:00 2001 From: Timo Rothenpieler Date: Sun, 28 Apr 2024 16:27:06 +0200 Subject: [PATCH] cgroup-util: allow cg_read_pid() to skip unmapped (zero) pids --- src/basic/cgroup-util.c | 45 +++++++++++++++++++++++---------------- src/basic/cgroup-util.h | 17 ++++++++------- src/cgtop/cgtop.c | 2 +- src/core/cgroup.c | 6 ++++-- src/core/dbus-unit.c | 2 +- src/shared/cgroup-setup.c | 6 +++++- src/shared/cgroup-show.c | 2 +- 7 files changed, 48 insertions(+), 32 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 8303b376843..c0d0fe6f148 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -95,7 +95,7 @@ int cg_enumerate_processes(const char *controller, const char *path, FILE **ret) return cg_enumerate_items(controller, path, ret, "cgroup.procs"); } -int cg_read_pid(FILE *f, pid_t *ret) { +int cg_read_pid(FILE *f, pid_t *ret, CGroupFlags flags) { unsigned long ul; /* Note that the cgroup.procs might contain duplicates! See cgroups.txt for details. */ @@ -103,27 +103,33 @@ int cg_read_pid(FILE *f, pid_t *ret) { assert(f); assert(ret); - errno = 0; - if (fscanf(f, "%lu", &ul) != 1) { + for (;;) { + errno = 0; + if (fscanf(f, "%lu", &ul) != 1) { - if (feof(f)) { - *ret = 0; - return 0; + if (feof(f)) { + *ret = 0; + return 0; + } + + return errno_or_else(EIO); } - return errno_or_else(EIO); - } + if (ul > PID_T_MAX) + return -EIO; - if (ul <= 0) - return -EIO; - if (ul > PID_T_MAX) - return -EIO; + /* In some circumstances (e.g. WSL), cgroups might contain unmappable PIDs from other + * contexts. These show up as zeros, and depending on the caller, can either be plain + * skipped over, or returned as-is. */ + if (ul == 0 && !FLAGS_SET(flags, CGROUP_DONT_SKIP_UNMAPPED)) + continue; - *ret = (pid_t) ul; - return 1; + *ret = (pid_t) ul; + return 1; + } } -int cg_read_pidref(FILE *f, PidRef *ret) { +int cg_read_pidref(FILE *f, PidRef *ret, CGroupFlags flags) { int r; assert(f); @@ -132,7 +138,7 @@ int cg_read_pidref(FILE *f, PidRef *ret) { for (;;) { pid_t pid; - r = cg_read_pid(f, &pid); + r = cg_read_pid(f, &pid, flags); if (r < 0) return r; if (r == 0) { @@ -140,6 +146,9 @@ int cg_read_pidref(FILE *f, PidRef *ret) { return 0; } + if (pid == 0) + return -EREMOTE; + r = pidref_set_pid(ret, pid); if (r >= 0) return 1; @@ -343,7 +352,7 @@ static int cg_kill_items( for (;;) { _cleanup_(pidref_done) PidRef pidref = PIDREF_NULL; - r = cg_read_pidref(f, &pidref); + r = cg_read_pidref(f, &pidref, /* flags = */ 0); if (r < 0) return RET_GATHER(ret, r); if (r == 0) @@ -938,7 +947,7 @@ int cg_is_empty(const char *controller, const char *path) { if (r < 0) return r; - r = cg_read_pid(f, &pid); + r = cg_read_pid(f, &pid, CGROUP_DONT_SKIP_UNMAPPED); if (r < 0) return r; diff --git a/src/basic/cgroup-util.h b/src/basic/cgroup-util.h index 9ad6dd8eb6c..29417b39ad5 100644 --- a/src/basic/cgroup-util.h +++ b/src/basic/cgroup-util.h @@ -183,20 +183,21 @@ typedef enum CGroupUnified { int cg_path_open(const char *controller, const char *path); int cg_cgroupid_open(int fsfd, uint64_t id); +typedef enum CGroupFlags { + CGROUP_SIGCONT = 1 << 0, + CGROUP_IGNORE_SELF = 1 << 1, + CGROUP_REMOVE = 1 << 2, + CGROUP_DONT_SKIP_UNMAPPED = 1 << 3, +} CGroupFlags; + int cg_enumerate_processes(const char *controller, const char *path, FILE **ret); -int cg_read_pid(FILE *f, pid_t *ret); -int cg_read_pidref(FILE *f, PidRef *ret); +int cg_read_pid(FILE *f, pid_t *ret, CGroupFlags flags); +int cg_read_pidref(FILE *f, PidRef *ret, CGroupFlags flags); int cg_read_event(const char *controller, const char *path, const char *event, char **ret); int cg_enumerate_subgroups(const char *controller, const char *path, DIR **ret); int cg_read_subgroup(DIR *d, char **ret); -typedef enum CGroupFlags { - CGROUP_SIGCONT = 1 << 0, - CGROUP_IGNORE_SELF = 1 << 1, - CGROUP_REMOVE = 1 << 2, -} CGroupFlags; - typedef int (*cg_kill_log_func_t)(const PidRef *pid, int sig, void *userdata); int cg_kill(const char *path, int sig, CGroupFlags flags, Set *s, cg_kill_log_func_t kill_log, void *userdata); diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c index ca514554408..08eae5988b0 100644 --- a/src/cgtop/cgtop.c +++ b/src/cgtop/cgtop.c @@ -207,7 +207,7 @@ static int process( return r; g->n_tasks = 0; - while (cg_read_pid(f, &pid) > 0) { + while (cg_read_pid(f, &pid, CGROUP_DONT_SKIP_UNMAPPED) > 0) { if (arg_count == COUNT_USERSPACE_PROCESSES && pid_is_kernel_thread(pid) > 0) continue; diff --git a/src/core/cgroup.c b/src/core/cgroup.c index 324da95a432..34fd2a250c4 100644 --- a/src/core/cgroup.c +++ b/src/core/cgroup.c @@ -3627,7 +3627,9 @@ int unit_search_main_pid(Unit *u, PidRef *ret) { for (;;) { _cleanup_(pidref_done) PidRef npidref = PIDREF_NULL; - r = cg_read_pidref(f, &npidref); + /* cg_read_pidref() will return an error on unmapped PIDs. + * We can't reasonably deal with units that contain those. */ + r = cg_read_pidref(f, &npidref, CGROUP_DONT_SKIP_UNMAPPED); if (r < 0) return r; if (r == 0) @@ -3669,7 +3671,7 @@ static int unit_watch_pids_in_path(Unit *u, const char *path) { for (;;) { _cleanup_(pidref_done) PidRef pid = PIDREF_NULL; - r = cg_read_pidref(f, &pid); + r = cg_read_pidref(f, &pid, /* flags = */ 0); if (r == 0) break; if (r < 0) { diff --git a/src/core/dbus-unit.c b/src/core/dbus-unit.c index 21039b7d4a7..20fa43af9e3 100644 --- a/src/core/dbus-unit.c +++ b/src/core/dbus-unit.c @@ -1300,7 +1300,7 @@ static int append_cgroup(sd_bus_message *reply, const char *p, Set *pids) { * threaded domain cgroup contains the PIDs of all processes in the subtree and is not * readable in the subtree proper. */ - r = cg_read_pidref(f, &pidref); + r = cg_read_pidref(f, &pidref, /* flags = */ 0); if (IN_SET(r, 0, -EOPNOTSUPP)) break; if (r < 0) diff --git a/src/shared/cgroup-setup.c b/src/shared/cgroup-setup.c index 6896a03da54..093b6d0d22e 100644 --- a/src/shared/cgroup-setup.c +++ b/src/shared/cgroup-setup.c @@ -614,7 +614,11 @@ int cg_migrate( if (r < 0) return RET_GATHER(ret, r); - while ((r = cg_read_pid(f, &pid)) > 0) { + while ((r = cg_read_pid(f, &pid, flags)) > 0) { + /* Throw an error if unmappable PIDs are in output, we can't migrate those. */ + if (pid == 0) + return -EREMOTE; + /* This might do weird stuff if we aren't a single-threaded program. However, we * luckily know we are. */ if (FLAGS_SET(flags, CGROUP_IGNORE_SELF) && pid == getpid_cached()) diff --git a/src/shared/cgroup-show.c b/src/shared/cgroup-show.c index ef4f3b3b52e..87177316da8 100644 --- a/src/shared/cgroup-show.c +++ b/src/shared/cgroup-show.c @@ -108,7 +108,7 @@ static int show_cgroup_one_by_path( * From https://docs.kernel.org/admin-guide/cgroup-v2.html#threads, * “cgroup.procs” in a threaded domain cgroup contains the PIDs of all processes in * the subtree and is not readable in the subtree proper. */ - r = cg_read_pid(f, &pid); + r = cg_read_pid(f, &pid, /* flags = */ 0); if (IN_SET(r, 0, -EOPNOTSUPP)) break; if (r < 0) -- 2.47.3