Messages in this thread | | | From | Suren Baghdasaryan <> | Date | Wed, 1 Mar 2023 10:05:36 -0800 | Subject | Re: [PATCH 1/1] cgroup: limit cgroup psi file writes to processes with CAP_SYS_RESOURCE |
| |
On Wed, Mar 1, 2023 at 1:47 AM Michal Hocko <mhocko@suse.com> wrote: > > On Tue 28-02-23 17:46:51, Suren Baghdasaryan wrote: > > Currently /proc/pressure/* files can be written only by processes with > > CAP_SYS_RESOURCE capability to prevent any unauthorized user from > > creating psi triggers. However no such limitation is required for > > per-cgroup pressure files. Fix this inconsistency by requiring the same > > capability for writing per-cgroup psi files. > > > > Fixes: 6db12ee0456d ("psi: allow unprivileged users with CAP_SYS_RESOURCE to write psi files") > > Is this really a regression from this commit? 6db12ee0456d is changing > permissions of those files to be world writeable with the > CAP_SYS_RESOURCE requirement. Permissions of cgroup files is not changed > and the default mode is 644 (with root as an owner) so only privileged > processes are allowed without any delegation.
Agree, the Fixes line here is not valid. I will remove it.
> > I think you should instead construct this slightly differently. The > ultimate goal is to allow a reasonable delegation after all, no?
Yes.
> > So keep your current patch and extend it by removing the min timeout > constrain and justify the change by the necessity of the granularity > tuning as reported by Sudarshan Rajagopala. If this causes any > regression then a revert would also return the min timeout constrain > back and we will have to think about a different approach.
I think adding CAP_SYS_RESOURCE check is needed even if we keep the min timeout capped like today. Without it one could create multiple cgroups and add a trigger into each one, therefore creating an unlimited number of "psimon" kernel threads. At some point I expect them to affect system performance because even at high polling intervals they still consume some cpu resources. So, this change I think is needed regardless of the change to min polling period and I would suggest keeping them separate.
> > The consistency with the global case is a valid point only partially > because different cgroups might have different owners which is not > usually the case for the global psi interface, right?
Correct.
> > Makes sense?
Yes but hopefully my argument about keeping this and min period patches separate is reasonable? Thanks, Suren.
> -- > Michal Hocko > SUSE Labs
| |