[lxc-devel] [PATCH] c/r: remember to chown the cgroup path (correctly)
Serge Hallyn
serge.hallyn at ubuntu.com
Thu Jan 14 09:28:07 UTC 2016
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.
This is probably ok - but did you run all the lxc tests against
this to make sure? You can still run 'lxc-cgroup' etc as an
unpriv user?
> > > + if (data.origuid < 0) {
> >
> > Can you confirm that this does not break
> >
> > sudo lxc-create -t download -n x1 -- -d ubuntu -r trusty -a amd64
> > sudo lxc-start -n x1
> >
> > Because in that case I think we have no mappings, and mapped_hostid() will
> > return -1.
>
> You can't see it in the patch, but just above this is a
D'oh! I even looked for such a thing this morning but missed it.
> lxc_list_empty() test, and this whole path isn't executed if
> lxc_list_empty() is true, so I think it should be ok.
>
> Tycho
>
> > > + ERROR("failed to get mapped root id");
> > > + return false;
> > > + }
> > >
> > > /* Unpriv users can't chown it themselves, so chown from
> > > * a child namespace mapping both our own and the target uid
> > > diff --git a/src/lxc/criu.c b/src/lxc/criu.c
> > > index 6ef4905..f442612 100644
> > > --- a/src/lxc/criu.c
> > > +++ b/src/lxc/criu.c
> > > @@ -466,6 +466,11 @@ void do_restore(struct lxc_container *c, int pipe, char *directory, bool verbose
> > > goto out_fini_handler;
> > > }
> > >
> > > + if (!cgroup_chown(handler)) {
> > > + ERROR("failed creating groups");
> > > + goto out_fini_handler;
> > > + }
> > > +
> > > if (!restore_net_info(c)) {
> > > ERROR("failed restoring network info");
> > > goto out_fini_handler;
> > > --
> > > 2.6.4
> > >
> > > _______________________________________________
> > > lxc-devel mailing list
> > > lxc-devel at lists.linuxcontainers.org
> > > http://lists.linuxcontainers.org/listinfo/lxc-devel
> > _______________________________________________
> > lxc-devel mailing list
> > lxc-devel at lists.linuxcontainers.org
> > http://lists.linuxcontainers.org/listinfo/lxc-devel
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
More information about the lxc-devel
mailing list