Skip to content

Commit

Permalink
cgroup-util: allow cg_read_pid() to return unmappable PIDs
Browse files Browse the repository at this point in the history
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 systemd#32534.
See also: microsoft/WSL#8879.

Co-authored-by: Timo Rothenpieler <[email protected]>
  • Loading branch information
keszybz and BtbN committed May 9, 2024
1 parent 85e23ef commit c1cff9f
Show file tree
Hide file tree
Showing 4 changed files with 26 additions and 7 deletions.
16 changes: 12 additions & 4 deletions src/basic/cgroup-util.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;

Expand All @@ -140,6 +142,12 @@ int cg_read_pidref(FILE *f, PidRef *ret) {
return 0;
}

if (pid == 0)
/* Un 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;
Expand Down
4 changes: 3 additions & 1 deletion src/cgtop/cgtop.c
Original file line number Diff line number Diff line change
Expand Up @@ -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++;
}

Expand Down
7 changes: 6 additions & 1 deletion src/shared/cgroup-setup.c
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand All @@ -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;

Expand Down
6 changes: 5 additions & 1 deletion src/shared/cgroup-show.c
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down

0 comments on commit c1cff9f

Please sign in to comment.