cgroup: unify attach permission checking
authorChristian Brauner <christian.brauner@ubuntu.com>
Wed, 5 Feb 2020 13:26:18 +0000 (14:26 +0100)
committerTejun Heo <tj@kernel.org>
Wed, 12 Feb 2020 22:57:51 +0000 (17:57 -0500)
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 <tj@kernel.org>
Cc: Li Zefan <lizefan@huawei.com>
Cc: Johannes Weiner <hannes@cmpxchg.org>
Cc: cgroups@vger.kernel.org
Acked-by: Michal Koutný <mkoutny@suse.com>
Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>
Signed-off-by: Tejun Heo <tj@kernel.org>
kernel/cgroup/cgroup.c

index 7a310db..9ca51bf 100644 (file)
@@ -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);
@@ -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)
 {
@@ -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;
 
@@ -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: