[lxc-devel] [PATCH] c/r: remember to chown the cgroup path (correctly)

[Tycho Andersen]

On Thu, Jan 14, 2016 at 07:43:20PM +0000, Serge Hallyn wrote:
> Quoting Tycho Andersen (tycho.andersen at canonical.com):
> > On Thu, Jan 14, 2016 at 09:28:07AM +0000, Serge Hallyn wrote:
> > > Quoting Tycho Andersen (tycho.andersen at canonical.com):
> > > > On Wed, Jan 13, 2016 at 09:47:50PM +0000, Serge Hallyn wrote:
> > > > > Quoting Tycho Andersen (tycho.andersen at canonical.com):
> > > > > > 1. remember to chown the cgroup path when migrating a container
> > > > > > 2. when restoring the cgroup path, try to compute the euid for root vs.
> > > > > >    using geteuid(); geteuid works for start, but it doesn't work for
> > > > > >    migration since we're still real root at that point.
> > > > > > 
> > > > > > Signed-off-by: Tycho Andersen <tycho.andersen at canonical.com>
> > > > > > ---
> > > > > >  src/lxc/cgmanager.c | 6 +++++-
> > > > > >  src/lxc/criu.c      | 5 +++++
> > > > > >  2 files changed, 10 insertions(+), 1 deletion(-)
> > > > > > 
> > > > > > diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
> > > > > > index 357182a..54e6912 100644
> > > > > > --- a/src/lxc/cgmanager.c
> > > > > > +++ b/src/lxc/cgmanager.c
> > > > > > @@ -488,7 +488,11 @@ static bool chown_cgroup(const char *cgroup_path, struct lxc_conf *conf)
> > > > > >  		return true;
> > > > > >  
> > > > > >  	data.cgroup_path = cgroup_path;
> > > > > > -	data.origuid = geteuid();
> > > > > > +	data.origuid = mapped_hostid(0, conf, ID_TYPE_UID);
> > > 
> > > now, when starting a container, this happens in the
> > > parent task in original uid.  So the geteuid() returns 1000,
> > > mapped_hostid(0, conf, ID_TYPE_UID) something like 100000.
> > 
> > Are you sure? Wouldn't it chmod everything to 1000 instead of 100000
> > in that case and be all screwed up?
> 
> No I'm not sure :)  I was lazily asking you to add an fprintf and verify :)

You're right! I think this may actually be the wrong fix,

> 
> Cgmanager does that automatically.  When you chown cgroup freezer:/lxc/c1,
> cgmanager chowns the directory itself (so you can create new cgroups), and
> the procs and tasks files, so you can move tasks in.  It does not change
> the other files so you can't escape your limits.
> 
> And right, that is why the parent dir must be -1:100000, so you can't chown
> the files.  This gets sabotaged when, as uid 1000 on the host, you map hostuid
> 1000 into the container.  But parent cgroups (like systemd user scopes) will at
> least still confine you.

Ok. Let's drop this patch for now and I'll see about figuring out
another way to do what I need here.

Tycho