[lxc-devel] [PATCH] Fix to work lxc-destroy with unprivileged containers on recent kernel

S.Çağlar Onur caglar at 10ur.org
Tue Jul 1 02:09:49 UTC 2014


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.

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>


More information about the lxc-devel mailing list