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

[Cyrill Gorcunov]

On Mon, May 18, 2015 at 11:21:40AM +0300, Konstantin Khorenko wrote:
> 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.

"yes", i think it's incomplete, will update, thanks!