cgroup-util: allow cg_read_pid() to skip unmapped (zero) pids

Author: BtbN

src/basic/cgroup-util.c

@@ -95,35 +95,41 @@ 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. */
 
         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,14 +138,17 @@ 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) {
                         *ret = PIDREF_NULL;
                         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;
 

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);

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;

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) {

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)

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())

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)