[Devel] [RFC rh7] ve: cgroups -- Allow to attach non-self into ve cgroups

[Konstantin Khorenko]

On 05/14/2015 07:52 PM, Cyrill Gorcunov wrote:
> In vzctl/libvzctl bundle we restore container like
> 
>  - create ve/$ctid cgroup
>  - move self into this cgroup
>  - run criu from inside
> 
> So that kernel code passes ve_can_attach test. In turn for
> our P.Haul project (which is managing live migration) the
> situation is different -- it opens ve/$ctid but moves
> criu service pid instead (so that the service will
> start restore procedure). Which leads to situation
> where ve_can_attach fails with -EINVAL.
> 
> Reported-by: Nikita Spiridonov <nspiridonov at odin.com>
> Signed-off-by: Cyrill Gorcunov <gorcunov at odin.com>
> CC: Vladimir Davydov <vdavydov at odin.com>
> CC: Konstantin Khorenko <khorenko at odin.com>
> CC: Pavel Emelyanov <xemul at odin.com>
> CC: Andrey Vagin <avagin at odin.com>
> ---
> 
> Guys, could you please take a look, especially from
> security POV, is it safe to remove all these checks?
> 
>  kernel/ve/ve.c |   31 +++++++++++++------------------
>  1 file changed, 13 insertions(+), 18 deletions(-)
> 
> Index: linux-pcs7.git/kernel/ve/ve.c
> ===================================================================
> --- linux-pcs7.git.orig/kernel/ve/ve.c
> +++ linux-pcs7.git/kernel/ve/ve.c
> @@ -750,13 +750,6 @@ static void ve_destroy(struct cgroup *cg
>  static int ve_can_attach(struct cgroup *cg, struct cgroup_taskset *tset)
>  {
>  	struct ve_struct *ve = cgroup_ve(cg);
> -	struct task_struct *task = current;
> -
> -	if (cgroup_taskset_size(tset) != 1 ||
> -	    cgroup_taskset_first(tset) != task ||
> -	    !thread_group_leader(task) ||
> -	    !thread_group_empty(task))
> -		return -EINVAL;

Is this true that without these checks a single thread of a multithread process can enter CT?
If no - where is the check for this case?
If yes - let's prohibit this.

>  	if (ve->is_locked)
>  		return -EBUSY;
> @@ -775,20 +768,22 @@ static int ve_can_attach(struct cgroup *
>  static void ve_attach(struct cgroup *cg, struct cgroup_taskset *tset)
>  {
>  	struct ve_struct *ve = cgroup_ve(cg);
> -	struct task_struct *tsk = current;
> -
> -	/* this probihibts ptracing of task entered to VE from host system */
> -	if (ve->is_running && tsk->mm)
> -		tsk->mm->vps_dumpable = VD_VE_ENTER_TASK;
> +	struct task_struct *tsk;
>  
> -	/* Drop OOM protection. */
> -	tsk->signal->oom_score_adj = 0;
> -	tsk->signal->oom_score_adj_min = 0;
> +	cgroup_taskset_for_each(tsk, cg, tset) {
> +		/* this probihibts ptracing of task entered to VE from host system */
> +		if (ve->is_running && tsk->mm)
> +			tsk->mm->vps_dumpable = VD_VE_ENTER_TASK;
> +
> +		/* Drop OOM protection. */
> +		tsk->signal->oom_score_adj = 0;
> +		tsk->signal->oom_score_adj_min = 0;
>  
> -	/* Leave parent exec domain */
> -	tsk->parent_exec_id--;
> +		/* Leave parent exec domain */
> +		tsk->parent_exec_id--;
>  
> -	tsk->task_ve = ve;
> +		tsk->task_ve = ve;
> +	}
>  }
>  
>  static int ve_state_read(struct cgroup *cg, struct cftype *cft,
> _______________________________________________
> Devel mailing list
> Devel at openvz.org
> https://lists.openvz.org/mailman/listinfo/devel
>