[Devel] [PATCH rh7] ub: sync exec_ub on fork

[Vladimir Davydov]

Since we want to be able to get/set current's exec_ub, when attaching a
task to a ub cgroup we change its exec_ub in a task_work (i.e. from its
own context, when it returns to userspace). This design works perfectly
well, but we have to be careful with forking tasks while currently we
are not.

The problem is that if a forking task is moved between cgroups, the
child will have exec_ub set to the source cgroup while being attached to
the destination cgroup. To avoid this discrepancy, we have to
synchronize the child's exec_ub with its cgroup on cgroup_post_fork.
This patch does the trick. It is safe to change exec_ub there, because
the task is not allowed to run yet and therefore cannot get/set its
exec_ub.

Reported-by: Kirill Tkhai <ktkhai at odin.com>
Signed-off-by: Vladimir Davydov <vdavydov at parallels.com>
---
 kernel/bc/beancounter.c | 26 ++++++++++++++++++++++++--
 1 file changed, 24 insertions(+), 2 deletions(-)

diff --git a/kernel/bc/beancounter.c b/kernel/bc/beancounter.c
index ecc5a96c1a5e..6bb3d3951262 100644
--- a/kernel/bc/beancounter.c
+++ b/kernel/bc/beancounter.c
@@ -570,20 +570,27 @@ static void ub_cgroup_css_free(struct cgroup *cg)
 	free_ub(ub);
 }
 
-static void ub_cgroup_attach_work_fn(struct callback_head *ch)
+static void __ub_cgroup_attach(struct task_struct *tsk)
 {
-	struct task_struct *tsk = current;
 	struct user_beancounter *ub;
 
 	rcu_read_lock();
 	do {
 		ub = cgroup_ub(task_cgroup(current, ub_subsys_id));
+		if (tsk->task_bc.exec_ub == ub)
+			goto out;
 	} while (!get_beancounter_rcu(ub));
 	put_beancounter(tsk->task_bc.exec_ub);
 	tsk->task_bc.exec_ub = ub;
+out:
 	rcu_read_unlock();
 }
 
+static void ub_cgroup_attach_work_fn(struct callback_head *ch)
+{
+	__ub_cgroup_attach(current);
+}
+
 static void ub_cgroup_attach(struct cgroup *cg, struct cgroup_taskset *tset)
 {
 	struct task_struct *p;
@@ -607,6 +614,20 @@ static void ub_cgroup_attach(struct cgroup *cg, struct cgroup_taskset *tset)
 	}
 }
 
+static void ub_cgroup_fork(struct task_struct *tsk)
+{
+	/*
+	 * If a forking task is moved between cgroups, the child will have
+	 * exec_ub set to the source cgroup while being attached to the
+	 * destination cgroup, because the parent's exec_ub will only change
+	 * when it returns to userspace (see ub_cgroup_attach). To avoid this
+	 * discrepancy, here we synchronize the child's exec_ub with its
+	 * cgroup. It is safe, because the task is not allowed to run yet and
+	 * therefore cannot get/set its exec_ub.
+	 */
+	__ub_cgroup_attach(tsk);
+}
+
 static ssize_t ub_cgroup_read(struct cgroup *cg, struct cftype *cft,
 			      struct file *file, char __user *buf,
 			      size_t nbytes, loff_t *ppos)
@@ -831,6 +852,7 @@ struct cgroup_subsys ub_subsys = {
 	.css_offline = ub_cgroup_css_offline,
 	.css_free = ub_cgroup_css_free,
 	.attach = ub_cgroup_attach,
+	.fork = ub_cgroup_fork,
 	.base_cftypes = ub_cgroup_files,
 	.use_id = true,
 };
-- 
2.1.4