From f98195b6bcf3bdcd526b322bb752fe20bcb522af Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Zbigniew=20J=C4=99drzejewski-Szmek?= Date: Wed, 8 May 2024 18:29:06 +0200 Subject: [PATCH] cgroup-util: allow cg_read_pid() to return unmappable PIDs In some environments, namely WSL2, the cgroup.procs PID list for some reason contain a ton of zeros everywhere, most likely those are from other instances under the same WSL Kernel, which at least always hosts the system instance with the X/Wayland/PA/Pipe server. Without this patch, whenever cg_read_pid encounters such a zero, it throws an error. This makes systemd near unusable inside of WSL. Change cg_read_pid() to return 0, and adjust all callers to handle that appropriately. In general, we cannot do anything with such processes, so most operations have to be refused. The only thing we can do with them is count them, in particular, we can answer the question whether the cgroup is empty in the negative. On normal systems, where the list does not contain any zeros to begin with, this has no averse effects. This replaces https://github.com/systemd/systemd/pull/32534. See also: https://github.com/microsoft/WSL/issues/8879. Co-authored-by: Timo Rothenpieler --- src/basic/cgroup-util.c | 16 ++++++++++++---- src/cgtop/cgtop.c | 4 +++- src/shared/cgroup-setup.c | 7 ++++++- src/shared/cgroup-show.c | 6 +++++- 4 files changed, 26 insertions(+), 7 deletions(-) diff --git a/src/basic/cgroup-util.c b/src/basic/cgroup-util.c index 8303b376843e7..68de4f770e9a2 100644 --- a/src/basic/cgroup-util.c +++ b/src/basic/cgroup-util.c @@ -98,14 +98,18 @@ int cg_enumerate_processes(const char *controller, const char *path, FILE **ret) int cg_read_pid(FILE *f, pid_t *ret) { unsigned long ul; - /* Note that the cgroup.procs might contain duplicates! See cgroups.txt for details. */ + /* Note that the cgroup.procs might contain duplicates! See cgroups.txt for details. + * + * In some circumstances (e.g. WSL2), cgroups might contain unmappable PIDs from other + * contexts. This function may return 0 in *ret, which is not a valid PID. Depending on + * the caller, it should either be skipped over or treated as an error. + */ assert(f); assert(ret); errno = 0; if (fscanf(f, "%lu", &ul) != 1) { - if (feof(f)) { *ret = 0; return 0; @@ -114,8 +118,6 @@ int cg_read_pid(FILE *f, pid_t *ret) { return errno_or_else(EIO); } - if (ul <= 0) - return -EIO; if (ul > PID_T_MAX) return -EIO; @@ -140,6 +142,12 @@ int cg_read_pidref(FILE *f, PidRef *ret) { return 0; } + if (pid == 0) + /* An unmappable PID. We certainly cannot create a pidref for it, so ignore + * it like other PIDs that we cannot find. (Also 0 would be interpreted as + * us by pidref_set_pid(), which we cannot allow to happen.) */ + continue; + r = pidref_set_pid(ret, pid); if (r >= 0) return 1; diff --git a/src/cgtop/cgtop.c b/src/cgtop/cgtop.c index ca514554408e8..2f01963d68e62 100644 --- a/src/cgtop/cgtop.c +++ b/src/cgtop/cgtop.c @@ -208,10 +208,12 @@ static int process( g->n_tasks = 0; while (cg_read_pid(f, &pid) > 0) { - if (arg_count == COUNT_USERSPACE_PROCESSES && pid_is_kernel_thread(pid) > 0) continue; + /* Any unmappable PIDs will be counted here. There is no great solution, + * and this should only occur in broken environments anyway. */ + g->n_tasks++; } diff --git a/src/shared/cgroup-setup.c b/src/shared/cgroup-setup.c index 6896a03da54e1..0c2ab30a74553 100644 --- a/src/shared/cgroup-setup.c +++ b/src/shared/cgroup-setup.c @@ -615,6 +615,10 @@ int cg_migrate( return RET_GATHER(ret, r); while ((r = cg_read_pid(f, &pid)) > 0) { + /* Ignore unmappable PIDs. We can't migrate those processes anyway. */ + if (pid == 0) + continue; + /* 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()) @@ -625,7 +629,8 @@ int cg_migrate( /* Ignore kernel threads. Since they can only exist in the root cgroup, we only * check for them there. */ - if (cfrom && empty_or_root(pfrom) && + if (cfrom && + empty_or_root(pfrom) && pid_is_kernel_thread(pid) > 0) continue; diff --git a/src/shared/cgroup-show.c b/src/shared/cgroup-show.c index c7af10687df13..5d31725cd3dcc 100644 --- a/src/shared/cgroup-show.c +++ b/src/shared/cgroup-show.c @@ -114,7 +114,11 @@ static int show_cgroup_one_by_path( if (r < 0) return r; - if (!(flags & OUTPUT_KERNEL_THREADS) && pid_is_kernel_thread(pid) > 0) + if (pid == 0) /* Ignore unmappable PIDs for foreign processes. */ + continue; + + if (!FLAGS_SET(flags, OUTPUT_KERNEL_THREADS) && + pid_is_kernel_thread(pid) > 0) continue; if (!GREEDY_REALLOC(pids, n + 1))