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

[Kirill Tkhai]

В Вт, 16/06/2015 в 19:02 +0300, 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>

Reviewed-by: Kirill Tkhai <ktkhai at odin.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,
>  };