cgroup_clone: use pid of newly created task for new cgroup
authorSerge E. Hallyn <serue@us.ibm.com>
Fri, 25 Jul 2008 08:47:06 +0000 (01:47 -0700)
committerLinus Torvalds <torvalds@linux-foundation.org>
Fri, 25 Jul 2008 17:53:37 +0000 (10:53 -0700)
cgroup_clone creates a new cgroup with the pid of the task.  This works
correctly for unshare, but for clone cgroup_clone is called from
copy_namespaces inside copy_process, which happens before the new pid is
created.  As a result, the new cgroup was created with current's pid.
This patch:

1. Moves the call inside copy_process to after the new pid
   is created
2. Passes the struct pid into ns_cgroup_clone (as it is not
   yet attached to the task)
3. Passes a name from ns_cgroup_clone() into cgroup_clone()
   so as to keep cgroup_clone() itself simpler
4. Uses pid_vnr() to get the process id value, so that the
   pid used to name the new cgroup is always the pid as it
   would be known to the task which did the cloning or
   unsharing.  I think that is the most intuitive thing to
   do.  This way, task t1 does clone(CLONE_NEWPID) to get
   t2, which does clone(CLONE_NEWPID) to get t3, then the
   cgroup for t3 will be named for the pid by which t2 knows
   t3.

(Thanks to Dan Smith for finding the main bug)

Changelog:
June 11: Incorporate Paul Menage's feedback:  don't pass
         NULL to ns_cgroup_clone from unshare, and reduce
 patch size by using 'nodename' in cgroup_clone.
June 10: Original version

[akpm@linux-foundation.org: build fix]
[akpm@linux-foundation.org: coding-style fixes]
Signed-off-by: Serge Hallyn <serge@us.ibm.com>
Acked-by: Paul Menage <menage@google.com>
Tested-by: Dan Smith <danms@us.ibm.com>
Cc: Balbir Singh <balbir@in.ibm.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
include/linux/cgroup.h
include/linux/nsproxy.h
kernel/cgroup.c
kernel/fork.c
kernel/ns_cgroup.c
kernel/nsproxy.c

index cc59d3a21d872e7b91ce230ddea572355e273a5d..c98dd7cb7076f44484a68643995f10fd1bbc2ff8 100644 (file)
@@ -364,7 +364,8 @@ static inline struct cgroup* task_cgroup(struct task_struct *task,
        return task_subsys_state(task, subsys_id)->cgroup;
 }
 
-int cgroup_clone(struct task_struct *tsk, struct cgroup_subsys *ss);
+int cgroup_clone(struct task_struct *tsk, struct cgroup_subsys *ss,
+                                                       char *nodename);
 
 /* A cgroup_iter should be treated as an opaque object */
 struct cgroup_iter {
index 0e66b57631fc767f202c32771f2ab5c8194c34cd..c8a768e59640665af635cedb9c84745e421fdbab 100644 (file)
@@ -82,9 +82,12 @@ static inline void get_nsproxy(struct nsproxy *ns)
 }
 
 #ifdef CONFIG_CGROUP_NS
-int ns_cgroup_clone(struct task_struct *tsk);
+int ns_cgroup_clone(struct task_struct *tsk, struct pid *pid);
 #else
-static inline int ns_cgroup_clone(struct task_struct *tsk) { return 0; }
+static inline int ns_cgroup_clone(struct task_struct *tsk, struct pid *pid)
+{
+       return 0;
+}
 #endif
 
 #endif
index 86b71e714e1336c29f4bb8c4d99c58b3eb9b8439..66ec9fd21e0c8206b1f89e16119b513fdc3a8896 100644 (file)
@@ -2848,16 +2848,17 @@ void cgroup_exit(struct task_struct *tsk, int run_callbacks)
  * cgroup_clone - clone the cgroup the given subsystem is attached to
  * @tsk: the task to be moved
  * @subsys: the given subsystem
+ * @nodename: the name for the new cgroup
  *
  * Duplicate the current cgroup in the hierarchy that the given
  * subsystem is attached to, and move this task into the new
  * child.
  */
-int cgroup_clone(struct task_struct *tsk, struct cgroup_subsys *subsys)
+int cgroup_clone(struct task_struct *tsk, struct cgroup_subsys *subsys,
+                                                       char *nodename)
 {
        struct dentry *dentry;
        int ret = 0;
-       char nodename[MAX_CGROUP_TYPE_NAMELEN];
        struct cgroup *parent, *child;
        struct inode *inode;
        struct css_set *cg;
@@ -2882,8 +2883,6 @@ int cgroup_clone(struct task_struct *tsk, struct cgroup_subsys *subsys)
        cg = tsk->cgroups;
        parent = task_cgroup(tsk, subsys->subsys_id);
 
-       snprintf(nodename, MAX_CGROUP_TYPE_NAMELEN, "%d", tsk->pid);
-
        /* Pin the hierarchy */
        atomic_inc(&parent->root->sb->s_active);
 
index 5a5d6fef341dd0d58cb6bde2d3b1602970ef6a0d..228f80c9155a40654d9f2eefddf2a7998cd16821 100644 (file)
@@ -1107,6 +1107,12 @@ static struct task_struct *copy_process(unsigned long clone_flags,
        if (clone_flags & CLONE_THREAD)
                p->tgid = current->tgid;
 
+       if (current->nsproxy != p->nsproxy) {
+               retval = ns_cgroup_clone(p, pid);
+               if (retval)
+                       goto bad_fork_free_pid;
+       }
+
        p->set_child_tid = (clone_flags & CLONE_CHILD_SETTID) ? child_tidptr : NULL;
        /*
         * Clear TID on mm_release()?
index 48d7ed6fc3a4d2dcc206d5eb304f55ae7436ac70..43c2111cd54de719917c0cdc9ace9e92445f4513 100644 (file)
@@ -7,6 +7,7 @@
 #include <linux/module.h>
 #include <linux/cgroup.h>
 #include <linux/fs.h>
+#include <linux/proc_fs.h>
 #include <linux/slab.h>
 #include <linux/nsproxy.h>
 
@@ -24,9 +25,12 @@ static inline struct ns_cgroup *cgroup_to_ns(
                            struct ns_cgroup, css);
 }
 
-int ns_cgroup_clone(struct task_struct *task)
+int ns_cgroup_clone(struct task_struct *task, struct pid *pid)
 {
-       return cgroup_clone(task, &ns_subsys);
+       char name[PROC_NUMBUF];
+
+       snprintf(name, PROC_NUMBUF, "%d", pid_vnr(pid));
+       return cgroup_clone(task, &ns_subsys, name);
 }
 
 /*
index adc785146a1cb81d6f7b3d676877921f10bb59e7..21575fc46d0597914d4b92b5749c53f59f9be4e1 100644 (file)
@@ -157,12 +157,6 @@ int copy_namespaces(unsigned long flags, struct task_struct *tsk)
                goto out;
        }
 
-       err = ns_cgroup_clone(tsk);
-       if (err) {
-               put_nsproxy(new_ns);
-               goto out;
-       }
-
        tsk->nsproxy = new_ns;
 
 out:
@@ -209,7 +203,7 @@ int unshare_nsproxy_namespaces(unsigned long unshare_flags,
                goto out;
        }
 
-       err = ns_cgroup_clone(current);
+       err = ns_cgroup_clone(current, task_pid(current));
        if (err)
                put_nsproxy(*new_nsp);