[lxc-devel] [PATCH] Fix to work lxc-destroy with unprivileged containers on recent kernel
S.Çağlar Onur
caglar at 10ur.org
Wed Jul 2 03:13:18 UTC 2014
OK I think I'm seeing something but haven't found the real problem,
yet. Looks like some caller (or the caller of the caller etc.) is not
checking the return code of the chown_mapped_root (from
src/lxc/conf.c) function and trying to progress. That seems to be
causing this problem. The following patch makes my tests happy again
(chown is failing with EACCES on my machine)
diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index bdd623a..12f1690 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -3566,10 +3566,10 @@ int chown_mapped_root(char *path, struct lxc_conf *conf)
}
// a trick for chgrp the file that is not owned by oneself
- if (chown(path, -1, hostgid) < 0) {
- ERROR("Error chgrp %s", path);
- return -1;
- }
+ //if (chown(path, -1, hostgid) < 0) {
+ // ERROR("Error chgrp %s", path);
+ // return -1;
+ //}
// "u:0:rootuid:1"
ret = snprintf(map1, 100, "u:0:%d:1", rootuid);
Try to create an unprivileged container and then clone it, you will
see "lxc_container: Error chgrp" error but interestingly lxc-clone is
going to succeed and will write "Created container X as copy of Y"
message multiple times.
# lxc-create -q -t download -n rubik -- -d ubuntu -r trusty -a amd64
lxc-clone -n kibur rubik
lxc_container: Error chgrp /home/caglar/.local/share/lxc/kibur/rootfs
to 1000 (Operation not permitted)
Created container kibur as copy of rubik
Created container kibur as copy of rubik
On Tue, Jul 1, 2014 at 12:07 PM, S.Çağlar Onur <caglar at 10ur.org> wrote:
> Hey Stéphane,
>
> On Mon, Jun 30, 2014 at 10:20 PM, Stéphane Graber <stgraber at ubuntu.com> wrote:
>> On Mon, Jun 30, 2014 at 10:09:49PM -0400, S.Çağlar Onur wrote:
>>> Hey there,
>>>
>>> This change set caused my go test suite to fail unexpectedly. Some of
>>> my tests produced errors like
>>>
>>> === RUN TestClone
>>> lxc_container: Error chgrp /home/caglar/.local/share/lxc/consectetur/rootfs
>>> --- PASS: TestClone (6.31 seconds)
>>>
>>> or
>>>
>>> lxc_container: failed mounting
>>> /home/caglar/.local/share/lxcsnaps/lorem/snap0/rootfs onto
>>> /home/caglar/.local/share/lxcsnaps/lorem/snap0/rootfs
>>> lxc_container: Error copying storage
>>> --- FAIL: TestRestoreSnapshot (76.30 seconds)
>>> lxc_test.go:241: restoring the container failed
>>>
>>> Then, some other test cases started to run multiple times for no
>>> reason and they eventually got stuck (see
>>> http://paste.ubuntu.com/7729266/ for an example)
>>>
>>> I'm on ubuntu trusty with latest kernel (3.13.0-30-generic) and using
>>> stable-1.0 branch. Reverting following three commits made my tests
>>> happy again
>>>
>>> * "Cast to gid_t to fix android build failure", commit
>>> 8c760bbd101b033ab7dbbd518f4500572b204d9a.
>>>
>>> * "Fix to work lxc-start with unprivileged containers on recent
>>> kernel", commit bd3214ff7f39956924baa83a6cfbdd542268b44.
>>>
>>> * "Fix to work lxc-destroy with unprivileged containers on recent
>>> kernel", commit 308fddd2e93bd6ae83f089d5a7e914e7134901cf.
>>>
>>> I'm not sure what triggers this problem. Unfortunately I don't have
>>> enough time to debug it further now but will try find some time. In
>>> the meantime, I'm wondering whether you are experiencing some test
>>> failures on your ci system as well.
>>
>> No, I had all CI tests run 3-4 times today with those commits and I
>> didn't get a single failure. The CI environment is Ubuntu 14.04 with the
>> 3.13 kernel, so should be identical to what you have, except that we're
>> running the C and python3 tests only.
>
> OK, good to know.
>
>> I wonder what could be causing this issue for the Go binding, some weird
>> threading bug again?
>
> Yeah it looks like it. Will try to dig in more over the weekend and
> will let you know if I can find something or somehow able to reproduce
> the issue outside of the go world.
>
>>>
>>> On Mon, Jun 30, 2014 at 10:46 AM, Serge Hallyn <serge.hallyn at ubuntu.com> wrote:
>>> > Quoting KATOH Yasufumi (karma at jazz.email.ne.jp):
>>> >> Hi,
>>> >>
>>> >> I applied this patch and test. lxc-destroy work fine. :-)
>>> >>
>>> >> >>> On Sat, 28 Jun 2014 18:39:54 +0900
>>> >> in message "[lxc-devel] [PATCH] Fix to work lxc-destroy with unprivileged containers on recent kernel"
>>> >> TAMUKI Shoichi-san wrote:
>>> >>
>>> >> > Change idmap_add_id() to add both ID_TYPE_UID and ID_TYPE_GID entries
>>> >> > to an existing lxc_conf, not just an ID_TYPE_UID entry, so as to work
>>> >> > lxc-destroy with unprivileged containers on recent kernel.
>>> >>
>>> >> > Signed-off-by: TAMUKI Shoichi <tamuki at linet.gr.jp>
>>> >>
>>> >> Acked-by: KATOH Yasufumi <karma at jazz.email.ne.jp>
>>> >
>>> > Thanks again!
>>> >
>>> > Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
>>> >
>>> >> > ---
>>> >> > src/lxc/conf.c | 49 ++++++++++++++++++++++++++++++++++---------------
>>> >> > 1 file changed, 34 insertions(+), 15 deletions(-)
>>> >>
>>> >> > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
>>> >> > index df2f7cc..70f57af 100644
>>> >> > --- a/src/lxc/conf.c
>>> >> > +++ b/src/lxc/conf.c
>>> >> > @@ -4508,14 +4508,14 @@ static int run_userns_fn(void *data)
>>> >> > }
>>> >>
>>> >> > /*
>>> >> > - * Add a ID_TYPE_UID entry to an existing lxc_conf, if it is not
>>> >> > - * alread there.
>>> >> > - * We may want to generalize this to do gids as well as uids, but right now
>>> >> > - * it's not necessary.
>>> >> > + * Add ID_TYPE_UID/ID_TYPE_GID entries to an existing lxc_conf,
>>> >> > + * if they are not already there.
>>> >> > */
>>> >> > -static struct lxc_list *idmap_add_id(struct lxc_conf *conf, uid_t uid)
>>> >> > +static struct lxc_list *idmap_add_id(struct lxc_conf *conf,
>>> >> > + uid_t uid, gid_t gid)
>>> >> > {
>>> >> > - int hostid_mapped = mapped_hostid(uid, conf, ID_TYPE_UID);
>>> >> > + int hostuid_mapped = mapped_hostid(uid, conf, ID_TYPE_UID);
>>> >> > + int hostgid_mapped = mapped_hostid(gid, conf, ID_TYPE_GID);
>>> >> > struct lxc_list *new = NULL, *tmp, *it, *next;
>>> >> > struct id_map *entry;
>>> >>
>>> >> > @@ -4526,9 +4526,9 @@ static struct lxc_list *idmap_add_id(struct lxc_conf *conf, uid_t uid)
>>> >> > }
>>> >> > lxc_list_init(new);
>>> >>
>>> >> > - if (hostid_mapped < 0) {
>>> >> > - hostid_mapped = find_unmapped_nsuid(conf, ID_TYPE_UID);
>>> >> > - if (hostid_mapped < 0)
>>> >> > + if (hostuid_mapped < 0) {
>>> >> > + hostuid_mapped = find_unmapped_nsuid(conf, ID_TYPE_UID);
>>> >> > + if (hostuid_mapped < 0)
>>> >> > goto err;
>>> >> > tmp = malloc(sizeof(*tmp));
>>> >> > if (!tmp)
>>> >> > @@ -4540,8 +4540,27 @@ static struct lxc_list *idmap_add_id(struct lxc_conf *conf, uid_t uid)
>>> >> > }
>>> >> > tmp->elem = entry;
>>> >> > entry->idtype = ID_TYPE_UID;
>>> >> > - entry->nsid = hostid_mapped;
>>> >> > - entry->hostid = (unsigned long)uid;
>>> >> > + entry->nsid = hostuid_mapped;
>>> >> > + entry->hostid = (unsigned long) uid;
>>> >> > + entry->range = 1;
>>> >> > + lxc_list_add_tail(new, tmp);
>>> >> > + }
>>> >> > + if (hostgid_mapped < 0) {
>>> >> > + hostgid_mapped = find_unmapped_nsuid(conf, ID_TYPE_GID);
>>> >> > + if (hostgid_mapped < 0)
>>> >> > + goto err;
>>> >> > + tmp = malloc(sizeof(*tmp));
>>> >> > + if (!tmp)
>>> >> > + goto err;
>>> >> > + entry = malloc(sizeof(*entry));
>>> >> > + if (!entry) {
>>> >> > + free(tmp);
>>> >> > + goto err;
>>> >> > + }
>>> >> > + tmp->elem = entry;
>>> >> > + entry->idtype = ID_TYPE_GID;
>>> >> > + entry->nsid = hostgid_mapped;
>>> >> > + entry->hostid = (unsigned long) gid;
>>> >> > entry->range = 1;
>>> >> > lxc_list_add_tail(new, tmp);
>>> >> > }
>>> >> > @@ -4563,7 +4582,7 @@ static struct lxc_list *idmap_add_id(struct lxc_conf *conf, uid_t uid)
>>> >> > return new;
>>> >>
>>> >> > err:
>>> >> > - ERROR("Out of memory building a new uid map");
>>> >> > + ERROR("Out of memory building a new uid/gid map");
>>> >> > if (new)
>>> >> > lxc_free_idmap(new);
>>> >> > free(new);
>>> >> > @@ -4572,7 +4591,7 @@ err:
>>> >>
>>> >> > /*
>>> >> > * Run a function in a new user namespace.
>>> >> > - * The caller's euid will be mapped in if it is not already.
>>> >> > + * The caller's euid/egid will be mapped in if it is not already.
>>> >> > */
>>> >> > int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data)
>>> >> > {
>>> >> > @@ -4597,8 +4616,8 @@ int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data)
>>> >> > close(p[0]);
>>> >> > p[0] = -1;
>>> >>
>>> >> > - if ((idmap = idmap_add_id(conf, geteuid())) == NULL) {
>>> >> > - ERROR("Error adding self to container uid map");
>>> >> > + if ((idmap = idmap_add_id(conf, geteuid(), getegid())) == NULL) {
>>> >> > + ERROR("Error adding self to container uid/gid map");
>>> >> > goto err;
>>> >> > }
>>> >>
>>> >> > --
>>> >> > 1.9.0
>>> >> > _______________________________________________
>>> >> > 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
>>>
>>>
>>>
>>> --
>>> S.Çağlar Onur <caglar at 10ur.org>
>>> _______________________________________________
>>> lxc-devel mailing list
>>> lxc-devel at lists.linuxcontainers.org
>>> http://lists.linuxcontainers.org/listinfo/lxc-devel
>>
>> --
>> Stéphane Graber
>> Ubuntu developer
>> http://www.ubuntu.com
>>
>> _______________________________________________
>> lxc-devel mailing list
>> lxc-devel at lists.linuxcontainers.org
>> http://lists.linuxcontainers.org/listinfo/lxc-devel
>>
>
>
>
> --
> S.Çağlar Onur <caglar at 10ur.org>
--
S.Çağlar Onur <caglar at 10ur.org>
More information about the lxc-devel
mailing list