Skip to content

Commit

Permalink
cgroup: unify attach permission checking
Browse files Browse the repository at this point in the history
The core codepaths to check whether a process can be attached to a
cgroup are the same for threads and thread-group leaders. Only a small
piece of code verifying that source and destination cgroup are in the
same domain differentiates the thread permission checking from
thread-group leader permission checking.
Since cgroup_migrate_vet_dst() only matters cgroup2 - it is a noop on
cgroup1 - we can move it out of cgroup_attach_task().
All checks can now be consolidated into a new helper
cgroup_attach_permissions() callable from both cgroup_procs_write() and
cgroup_threads_write().

Cc: Tejun Heo <[email protected]>
Cc: Li Zefan <[email protected]>
Cc: Johannes Weiner <[email protected]>
Cc: [email protected]
Acked-by: Michal Koutný <[email protected]>
Signed-off-by: Christian Brauner <[email protected]>
Signed-off-by: Tejun Heo <[email protected]>
  • Loading branch information
Christian Brauner authored and htejun committed Feb 12, 2020
1 parent a49e462 commit 6df970e
Showing 1 changed file with 25 additions and 14 deletions.
39 changes: 25 additions & 14 deletions kernel/cgroup/cgroup.c
Original file line number Diff line number Diff line change
Expand Up @@ -2714,11 +2714,7 @@ int cgroup_attach_task(struct cgroup *dst_cgrp, struct task_struct *leader,
{
DEFINE_CGROUP_MGCTX(mgctx);
struct task_struct *task;
int ret;

ret = cgroup_migrate_vet_dst(dst_cgrp);
if (ret)
return ret;
int ret = 0;

/* look up all src csets */
spin_lock_irq(&css_set_lock);
Expand Down Expand Up @@ -4695,6 +4691,26 @@ static int cgroup_procs_write_permission(struct cgroup *src_cgrp,
return 0;
}

static int cgroup_attach_permissions(struct cgroup *src_cgrp,
struct cgroup *dst_cgrp,
struct super_block *sb, bool threadgroup)
{
int ret = 0;

ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp, sb);
if (ret)
return ret;

ret = cgroup_migrate_vet_dst(dst_cgrp);
if (ret)
return ret;

if (!threadgroup && (src_cgrp->dom_cgrp != dst_cgrp->dom_cgrp))
ret = -EOPNOTSUPP;

return ret;
}

static ssize_t cgroup_procs_write(struct kernfs_open_file *of,
char *buf, size_t nbytes, loff_t off)
{
Expand All @@ -4717,8 +4733,8 @@ static ssize_t cgroup_procs_write(struct kernfs_open_file *of,
src_cgrp = task_cgroup_from_root(task, &cgrp_dfl_root);
spin_unlock_irq(&css_set_lock);

ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp,
of->file->f_path.dentry->d_sb);
ret = cgroup_attach_permissions(src_cgrp, dst_cgrp,
of->file->f_path.dentry->d_sb, true);
if (ret)
goto out_finish;

Expand Down Expand Up @@ -4762,16 +4778,11 @@ static ssize_t cgroup_threads_write(struct kernfs_open_file *of,
spin_unlock_irq(&css_set_lock);

/* thread migrations follow the cgroup.procs delegation rule */
ret = cgroup_procs_write_permission(src_cgrp, dst_cgrp,
of->file->f_path.dentry->d_sb);
ret = cgroup_attach_permissions(src_cgrp, dst_cgrp,
of->file->f_path.dentry->d_sb, false);
if (ret)
goto out_finish;

/* and must be contained in the same domain */
ret = -EOPNOTSUPP;
if (src_cgrp->dom_cgrp != dst_cgrp->dom_cgrp)
goto out_finish;

ret = cgroup_attach_task(dst_cgrp, task, false);

out_finish:
Expand Down

0 comments on commit 6df970e

Please sign in to comment.