Skip to content
New issue

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

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

Already on GitHub? # to your account

BUG: pam_cgroup simply doesn't work with cgroup2 #84

Closed
0n-s opened this issue Dec 26, 2021 · 5 comments
Closed

BUG: pam_cgroup simply doesn't work with cgroup2 #84

0n-s opened this issue Dec 26, 2021 · 5 comments
Assignees
Labels
Milestone

Comments

@0n-s
Copy link

0n-s commented Dec 26, 2021

pam_cgroup, at least under a cgroup2-only environment (maybe cgroup1 as well, I haven't tested), simply doesn't seem to work at all. It still acts as if it succeeded.

Tracing a random sshd instance for every file it opens, there are only 3:

25666 sshd                3   0 /proc/cgroups
25666 sshd                5   0 /proc/mounts
25666 sshd                7   0 /sys/fs/cgroup/cgroup.controllers

(nothing else cgroup or libcgroup-related)
For whatever reason cgrules.conf is never loaded. Despite this, it claims it changed the cgroup successfully (with debug flag):

Dec 26 13:49:12 h pam_cgroup(sshd:session): Changed cgroup for process 25666  with username [redacted].

This is not true, as we can see from the just logged in session:

[redacted]@h $ cat /proc/self/cgroup
0::/

This failure also doesn't result in the login failing, even though in my PAM configuration pam_cgroup is not optional but required.

(tested with libcgroup v2.0)

@drakenclimber
Copy link
Member

Thanks for the bug report, @0n-s.

The pam module hasn't been a priority for the current users of libcgroup, so we have not emphasized development or testing in this area. I'm uncertain how much the libcgroup/pam module is being used, as this is the first issue reported against it even though v2.0 has been out 7+ month. Hmmmm....

@drakenclimber drakenclimber self-assigned this Jan 3, 2022
@drakenclimber
Copy link
Member

I'm going to assign this to v2.0.1. I'm not certain if it will be fixed in that release, but I would like us to make a decision on how to proceed with pam by that release.

Thanks again for the report!

@drakenclimber drakenclimber added this to the v2.0.1 milestone Jan 3, 2022
@ghost
Copy link

ghost commented Jan 3, 2022

The pam module is a great thing to have. Cgrulesengd is a hack anyway.

I think the Slackware community has seen similar problems with cgroupv1 (and had to rely on cgrulesengd), but sometimes people are just too lazy or shy to report bugs :(.

@drakenclimber
Copy link
Member

The pam module is a great thing to have. Cgrulesengd is a hack anyway.

Won't argue with the cgrules comment :)

I think the Slackware community has seen similar problems with cgroupv1 (and had to rely on cgrulesengd), but sometimes people are just too lazy or shy to report bugs :(.

That's good to know. Thank you.

@drakenclimber drakenclimber changed the title pam_cgroup simply doesn't work with cgroup2 BUG: pam_cgroup simply doesn't work with cgroup2 Jan 12, 2022
kamalesh-babulal added a commit to kamalesh-babulal/libcgroup that referenced this issue Jan 14, 2022
Services using pam_cgroup.so plugin, asks to match a rule, in the
cached list of rules already constructed from /etc/cgrules.conf.
This works well with active cgrulesengd but in the instances where
its disabled, the rules are not read and cached by default. Fix,
this is by reloading and caching the rules, when called with
CGFLAG_USECACHE flag.

Fixes: libcgroup#84

Reported-by: Kent Overstreet <kent.overstreet@gmail.com>
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
drakenclimber pushed a commit that referenced this issue Jan 18, 2022
pam_cgroup.so plugin uses /etc/cgrules.conf to assign processes
to the requested cgroup.  This works well with active
cgrulesengd but in the instances where cgrulesengd is disabled,
the rules are not read and cached by default. Fix this is by
reloading and caching the rules when called with CGFLAG_USECACHE
flag.

Fixes: #84

Reported-by: Kent Overstreet <kent.overstreet@gmail.com>
Signed-off-by: Kamalesh Babulal <kamalesh.babulal@oracle.com>
Signed-off-by: Tom Hromatka <tom.hromatka@oracle.com>
TJH: Minor commit comment changes
(cherry picked from commit 80a0bd4)
@drakenclimber
Copy link
Member

drakenclimber commented Jan 18, 2022

Thanks for the comments, @0n-s and @lockywolf. I have merged a fix into main and release-2.0. Let me know if you have any issues.

I'm hoping to get out another release of the 2.0 branch in the next month or so. This fix will obviously be in it.

Thanks again!

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

Successfully merging a pull request may close this issue.

2 participants