Commit 576dd464 authored by Tejun Heo's avatar Tejun Heo

cgroup: drop the matching uid requirement on migration for cgroup v2

Along with the write access to the cgroup.procs or tasks file, cgroup
has required the writer's euid, unless root, to match [s]uid of the
target process or task.  On cgroup v1, this is necessary because
there's nothing preventing a delegatee from pulling in tasks or
processes from all over the system.

If a user has a cgroup subdirectory delegated to it, the user would
have write access to the cgroup.procs or tasks file.  If there are no
further checks than file write access check, the user would be able to
pull processes from all over the system into its subhierarchy which is
clearly not the intended behavior.  The matching [s]uid requirement
partially prevents this problem by allowing a delegatee to pull in the
processes that belongs to it.  This isn't a sufficient protection
however, because a user would still be able to jump processes across
two disjoint sub-hierarchies that has been delegated to them.

cgroup v2 resolves the issue by requiring the writer to have access to
the common ancestor of the cgroup.procs file of the source and target
cgroups.  This confines each delegatee to their own sub-hierarchy
proper and bases all permission decisions on the cgroup filesystem
rather than having to pull in explicit uid matching.

cgroup v2 has still been applying the matching [s]uid requirement just
for historical reasons.  On cgroup2, the requirement doesn't serve any
purpose while unnecessarily complicating the permission model.  Let's
drop it.
Signed-off-by: default avatarTejun Heo <tj@kernel.org>
parent 968ebff1
...@@ -332,14 +332,12 @@ a process with a non-root euid to migrate a target process into a ...@@ -332,14 +332,12 @@ a process with a non-root euid to migrate a target process into a
cgroup by writing its PID to the "cgroup.procs" file, the following cgroup by writing its PID to the "cgroup.procs" file, the following
conditions must be met. conditions must be met.
- The writer's euid must match either uid or suid of the target process.
- The writer must have write access to the "cgroup.procs" file. - The writer must have write access to the "cgroup.procs" file.
- The writer must have write access to the "cgroup.procs" file of the - The writer must have write access to the "cgroup.procs" file of the
common ancestor of the source and destination cgroups. common ancestor of the source and destination cgroups.
The above three constraints ensure that while a delegatee may migrate The above two constraints ensure that while a delegatee may migrate
processes around freely in the delegated sub-hierarchy it can't pull processes around freely in the delegated sub-hierarchy it can't pull
in from or push out to outside the sub-hierarchy. in from or push out to outside the sub-hierarchy.
...@@ -354,10 +352,10 @@ all processes under C0 and C1 belong to U0. ...@@ -354,10 +352,10 @@ all processes under C0 and C1 belong to U0.
Let's also say U0 wants to write the PID of a process which is Let's also say U0 wants to write the PID of a process which is
currently in C10 into "C00/cgroup.procs". U0 has write access to the currently in C10 into "C00/cgroup.procs". U0 has write access to the
file and uid match on the process; however, the common ancestor of the file; however, the common ancestor of the source cgroup C10 and the
source cgroup C10 and the destination cgroup C00 is above the points destination cgroup C00 is above the points of delegation and U0 would
of delegation and U0 would not have write access to its "cgroup.procs" not have write access to its "cgroup.procs" files and thus the write
files and thus the write will be denied with -EACCES. will be denied with -EACCES.
2-6. Guidelines 2-6. Guidelines
......
...@@ -2349,20 +2349,9 @@ static int cgroup_procs_write_permission(struct task_struct *task, ...@@ -2349,20 +2349,9 @@ static int cgroup_procs_write_permission(struct task_struct *task,
struct cgroup *dst_cgrp, struct cgroup *dst_cgrp,
struct kernfs_open_file *of) struct kernfs_open_file *of)
{ {
const struct cred *cred = current_cred();
const struct cred *tcred = get_task_cred(task);
int ret = 0; int ret = 0;
/* if (cgroup_on_dfl(dst_cgrp)) {
* even if we're attaching all tasks in the thread group, we only
* need to check permissions on one of them.
*/
if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) &&
!uid_eq(cred->euid, tcred->uid) &&
!uid_eq(cred->euid, tcred->suid))
ret = -EACCES;
if (!ret && cgroup_on_dfl(dst_cgrp)) {
struct super_block *sb = of->file->f_path.dentry->d_sb; struct super_block *sb = of->file->f_path.dentry->d_sb;
struct cgroup *cgrp; struct cgroup *cgrp;
struct inode *inode; struct inode *inode;
...@@ -2380,9 +2369,21 @@ static int cgroup_procs_write_permission(struct task_struct *task, ...@@ -2380,9 +2369,21 @@ static int cgroup_procs_write_permission(struct task_struct *task,
ret = inode_permission(inode, MAY_WRITE); ret = inode_permission(inode, MAY_WRITE);
iput(inode); iput(inode);
} }
} else {
const struct cred *cred = current_cred();
const struct cred *tcred = get_task_cred(task);
/*
* even if we're attaching all tasks in the thread group,
* we only need to check permissions on one of them.
*/
if (!uid_eq(cred->euid, GLOBAL_ROOT_UID) &&
!uid_eq(cred->euid, tcred->uid) &&
!uid_eq(cred->euid, tcred->suid))
ret = -EACCES;
put_cred(tcred);
} }
put_cred(tcred);
return ret; return ret;
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment