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.

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

Handle process cgroup paths containng ':' #152

Merged
merged 1 commit into from Oct 14, 2021
Merged

Conversation

rosds
Copy link
Contributor

@rosds rosds commented Oct 14, 2021

Cgroup paths containing ':' will cause Process::cgroups to return an
incomplete pathname. The reason is that parsing the fields in
/proc/self/cgroup is perform with .split(':'). If something as
follows is set:

mkdir /sys/fs/cgroup/memory/foo:bar
echo $$ > /sys/fs/cgroup/memory/foo:bar/tasks
cat /proc/self/cgroup
...
8:memory:/foo:bar
...

the resulting ProcessCgroup will have a pathname equal to just /foo.

This patch addresses this by taking the remainer of the line after the
second ':' as the pathname. This is safe because cgroups(7) describes
/proc/self/cgroup as only having 3 fields.

Cgroup paths containing ':' will cause `Process::cgroups` to return an
incomplete pathname. The reason is that parsing the fields in
`/proc/self/cgroup` is perform with `.split(':')`. If something as
follows is set:

```bash
mkdir /sys/fs/cgroup/memory/foo:bar
echo $$ > /sys/fs/cgroup/memory/foo:bar/tasks
cat /proc/self/cgroup
...
8:memory:/foo:bar
...
```

the resulting `ProcessCgroup` will have a `pathname` equal to just `/foo`.

This patch addresses this by taking the remainer of the line after the
second ':' as the pathname. This is safe because `cgroups(7)` describes
`/proc/self/cgroup` as only having 3 fields.
@rosds rosds marked this pull request as ready for review October 14, 2021 09:24
@eminence
Copy link
Owner

Nice find and nice fix, thanks!

@eminence
Copy link
Owner

Hi @alfonsoros88, the latest v0.11.0 release contains your fix for this issue. Thanks again

@rosds
Copy link
Contributor Author

rosds commented Oct 14, 2021

@eminence thanks a lot! that was fast :)

@eminence
Copy link
Owner

There were several other pending fixes that needed to be released, so your fix was just some good extra motivation to cut the release :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants