Subject: make the fork hook to return an error From: Daniel Lezcano The fork hooks does not do any error checking making possible to have a cgroup subsystem to fail without detecting an error. This patch makes the fork hook to return an error if a fork hook failed in one of the subsystems. When a fork hook fails for a subsystem, the function 'cgroup_fork_callbacks' rollbacks, by calling the exit callback for all the subsystem the fork callback were called at this point. Signed-off-by: Daniel Lezcano Signed-off-by: Cedric Le Goater --- include/linux/cgroup.h | 8 ++++---- kernel/cgroup.c | 37 +++++++++++++++++++++++++++---------- kernel/cgroup_freezer.c | 6 ++++-- kernel/exit.c | 2 +- kernel/fork.c | 13 +++++++------ 5 files changed, 43 insertions(+), 23 deletions(-) Index: linux-2.6/include/linux/cgroup.h =================================================================== --- linux-2.6.orig/include/linux/cgroup.h +++ linux-2.6/include/linux/cgroup.h @@ -31,9 +31,9 @@ extern void cgroup_lock(void); extern bool cgroup_lock_live_group(struct cgroup *cgrp); extern void cgroup_unlock(void); extern void cgroup_fork(struct task_struct *p); -extern void cgroup_fork_callbacks(struct task_struct *p); +extern int cgroup_fork_callbacks(struct task_struct *p); extern void cgroup_post_fork(struct task_struct *p); -extern void cgroup_exit(struct task_struct *p, int run_callbacks); +extern void cgroup_exit(struct task_struct *p); extern int cgroupstats_build(struct cgroupstats *stats, struct dentry *dentry); @@ -430,7 +430,7 @@ struct cgroup_subsys { void (*attach)(struct cgroup_subsys *ss, struct cgroup *cgrp, struct cgroup *old_cgrp, struct task_struct *tsk, bool threadgroup); - void (*fork)(struct cgroup_subsys *ss, struct task_struct *task); + int (*fork)(struct cgroup_subsys *ss, struct task_struct *task); void (*exit)(struct cgroup_subsys *ss, struct task_struct *task); int (*populate)(struct cgroup_subsys *ss, struct cgroup *cgrp); @@ -569,7 +569,7 @@ unsigned short css_depth(struct cgroup_s static inline int cgroup_init_early(void) { return 0; } static inline int cgroup_init(void) { return 0; } static inline void cgroup_fork(struct task_struct *p) {} -static inline void cgroup_fork_callbacks(struct task_struct *p) {} +static inline int cgroup_fork_callbacks(struct task_struct *p) {} static inline void cgroup_post_fork(struct task_struct *p) {} static inline void cgroup_exit(struct task_struct *p, int callbacks) {} Index: linux-2.6/kernel/cgroup.c =================================================================== --- linux-2.6.orig/kernel/cgroup.c +++ linux-2.6/kernel/cgroup.c @@ -3436,16 +3436,33 @@ void cgroup_fork(struct task_struct *chi * tasklist. No need to take any locks since no-one can * be operating on this task. */ -void cgroup_fork_callbacks(struct task_struct *child) +int cgroup_fork_callbacks(struct task_struct *child) { - if (need_forkexit_callback) { - int i; - for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) { - struct cgroup_subsys *ss = subsys[i]; - if (ss->fork) - ss->fork(ss, child); - } + int i, ret = 0; + struct cgroup_subsys *ss; + + if (!need_forkexit_callback) + goto out; + + for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) { + ss = subsys[i]; + if (!ss->fork) + continue; + ret = ss->fork(ss, child); + if (ret) + goto out_exit; } +out: + return ret; + +out_exit: + for (i--; i >= 0; i--) { + ss = subsys[i]; + if (!ss->exit) + continue; + ss->exit(ss, child); + } + goto out; } /** @@ -3503,12 +3520,12 @@ void cgroup_post_fork(struct task_struct * which wards off any cgroup_attach_task() attempts, or task is a failed * fork, never visible to cgroup_attach_task. */ -void cgroup_exit(struct task_struct *tsk, int run_callbacks) +void cgroup_exit(struct task_struct *tsk) { int i; struct css_set *cg; - if (run_callbacks && need_forkexit_callback) { + if (need_forkexit_callback) { for (i = 0; i < CGROUP_SUBSYS_COUNT; i++) { struct cgroup_subsys *ss = subsys[i]; if (ss->exit) Index: linux-2.6/kernel/cgroup_freezer.c =================================================================== --- linux-2.6.orig/kernel/cgroup_freezer.c +++ linux-2.6/kernel/cgroup_freezer.c @@ -193,7 +193,7 @@ static int freezer_can_attach(struct cgr return 0; } -static void freezer_fork(struct cgroup_subsys *ss, struct task_struct *task) +static int freezer_fork(struct cgroup_subsys *ss, struct task_struct *task) { struct freezer *freezer; @@ -210,7 +210,7 @@ static void freezer_fork(struct cgroup_s * following check. */ if (!freezer->css.cgroup->parent) - return; + goto out; spin_lock_irq(&freezer->lock); BUG_ON(freezer->state == CGROUP_FROZEN); @@ -219,6 +219,8 @@ static void freezer_fork(struct cgroup_s if (freezer->state == CGROUP_FREEZING) freeze_task(task, true); spin_unlock_irq(&freezer->lock); +out: + return 0; } /* Index: linux-2.6/kernel/exit.c =================================================================== --- linux-2.6.orig/kernel/exit.c +++ linux-2.6/kernel/exit.c @@ -968,7 +968,7 @@ NORET_TYPE void do_exit(long code) exit_fs(tsk); check_stack_usage(); exit_thread(); - cgroup_exit(tsk, 1); + cgroup_exit(tsk); if (group_dead && tsk->signal->leader) disassociate_ctty(1); Index: linux-2.6/kernel/fork.c =================================================================== --- linux-2.6.orig/kernel/fork.c +++ linux-2.6/kernel/fork.c @@ -979,7 +979,6 @@ static struct task_struct *copy_process( { int retval; struct task_struct *p; - int cgroup_callbacks_done = 0; if ((clone_flags & (CLONE_NEWNS|CLONE_FS)) == (CLONE_NEWNS|CLONE_FS)) return ERR_PTR(-EINVAL); @@ -1217,13 +1216,14 @@ static struct task_struct *copy_process( /* Now that the task is set up, run cgroup callbacks if * necessary. We need to run them before the task is visible * on the tasklist. */ - cgroup_fork_callbacks(p); - cgroup_callbacks_done = 1; + retval = cgroup_fork_callbacks(p); + if (retval) + goto bad_fork_free_pid; if (current->nsproxy != p->nsproxy) { retval = ns_cgroup_clone(p, pid); if (retval) - goto bad_fork_free_pid; + goto bad_fork_cgroup_callbacks; } /* Need tasklist lock for parent etc handling! */ @@ -1268,7 +1268,7 @@ static struct task_struct *copy_process( spin_unlock(¤t->sighand->siglock); write_unlock_irq(&tasklist_lock); retval = -ERESTARTNOINTR; - goto bad_fork_free_pid; + goto bad_fork_cgroup_callbacks; } if (clone_flags & CLONE_THREAD) { @@ -1306,6 +1306,8 @@ static struct task_struct *copy_process( perf_event_fork(p); return p; +bad_fork_cgroup_callbacks: + cgroup_exit(p); bad_fork_free_pid: if (pid != &init_struct_pid) free_pid(pid); @@ -1335,7 +1337,6 @@ bad_fork_cleanup_policy: mpol_put(p->mempolicy); bad_fork_cleanup_cgroup: #endif - cgroup_exit(p, cgroup_callbacks_done); delayacct_tsk_free(p); module_put(task_thread_info(p)->exec_domain->module); bad_fork_cleanup_count: