Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

cgroup-util: make sure cg_read_pid() can deal sanely with unmapped PIDs (as in foreign pidns) #32534

Merged
merged 1 commit into from
May 14, 2024
Merged

cgroup-util: make sure cg_read_pid() can deal sanely with unmapped PIDs (as in foreign pidns) #32534

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
45 changes: 27 additions & 18 deletions src/basic/cgroup-util.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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");
YHNdnzj marked this conversation as resolved.
Show resolved Hide resolved
}

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);
Expand All @@ -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;
Expand Down Expand Up @@ -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);
keszybz marked this conversation as resolved.
Show resolved Hide resolved
if (r < 0)
return RET_GATHER(ret, r);
if (r == 0)
Expand Down Expand Up @@ -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;

Expand Down
17 changes: 9 additions & 8 deletions src/basic/cgroup-util.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion src/cgtop/cgtop.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
6 changes: 4 additions & 2 deletions src/core/cgroup.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
YHNdnzj marked this conversation as resolved.
Show resolved Hide resolved
if (r < 0)
return r;
if (r == 0)
Expand Down Expand Up @@ -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) {
Expand Down
2 changes: 1 addition & 1 deletion src/core/dbus-unit.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
6 changes: 5 additions & 1 deletion src/shared/cgroup-setup.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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())
Expand Down
2 changes: 1 addition & 1 deletion src/shared/cgroup-show.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down