On Fri, Nov 22, 2013 at 02:39:37PM -0600, Serge Hallyn wrote:
> This is necessary to have the rights to remove files owned by our subuids.
> 
> Also update lxc_rmdir_onedev to return 0 on success, -1 on failure.
> Callers were not consistent in using it correctly, and this is more
> in keeping with the rest of our code.
> 
> Signed-off-by: Serge Hallyn <serge.hal...@ubuntu.com>

Acked-by: Stéphane Graber <stgra...@ubuntu.com>

> ---
>  src/lxc/bdev.c         |   2 +-
>  src/lxc/conf.c         | 155 
> ++++++++++++++++++++++++++++++++++++++++++++++++-
>  src/lxc/conf.h         |   3 +
>  src/lxc/lxc_destroy.c  |   7 ---
>  src/lxc/lxccontainer.c |  28 ++++++---
>  src/lxc/utils.c        |  10 ++--
>  6 files changed, 182 insertions(+), 23 deletions(-)
> 
> diff --git a/src/lxc/bdev.c b/src/lxc/bdev.c
> index 6acd29a..03fecfb 100644
> --- a/src/lxc/bdev.c
> +++ b/src/lxc/bdev.c
> @@ -450,7 +450,7 @@ static int dir_clonepaths(struct bdev *orig, struct bdev 
> *new, const char *oldna
>  
>  static int dir_destroy(struct bdev *orig)
>  {
> -     if (!lxc_rmdir_onedev(orig->src))
> +     if (lxc_rmdir_onedev(orig->src) < 0)
>               return -1;
>       return 0;
>  }
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index c8809d2..4b786b1 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -75,6 +75,7 @@
>  #include "bdev.h"
>  #include "cgroup.h"
>  #include "lxclock.h"
> +#include "namespace.h"
>  #include "lsm/lsm.h"
>  
>  #if HAVE_SYS_CAPABILITY_H
> @@ -3810,11 +3811,10 @@ int lxc_clear_config_caps(struct lxc_conf *c)
>       return 0;
>  }
>  
> -int lxc_clear_idmaps(struct lxc_conf *c)
> -{
> +int lxc_free_idmap(struct lxc_list *id_map) {
>       struct lxc_list *it, *next;
>  
> -     lxc_list_for_each_safe(it, &c->id_map, next) {
> +     lxc_list_for_each_safe(it, id_map, next) {
>               lxc_list_del(it);
>               free(it->elem);
>               free(it);
> @@ -3822,6 +3822,11 @@ int lxc_clear_idmaps(struct lxc_conf *c)
>       return 0;
>  }
>  
> +int lxc_clear_idmaps(struct lxc_conf *c)
> +{
> +     return lxc_free_idmap(&c->id_map);
> +}
> +
>  int lxc_clear_config_keepcaps(struct lxc_conf *c)
>  {
>       struct lxc_list *it,*next;
> @@ -3941,3 +3946,147 @@ void lxc_conf_free(struct lxc_conf *conf)
>       lxc_clear_idmaps(conf);
>       free(conf);
>  }
> +
> +struct userns_fn_data {
> +     int (*fn)(void *);
> +     void *arg;
> +     int p[2];
> +};
> +
> +static int run_userns_fn(void *data)
> +{
> +     struct userns_fn_data *d = data;
> +     char c;
> +     // we're not sharing with the parent any more, if it was a thread
> +
> +     close(d->p[1]);
> +     if (read(d->p[0], &c, 1) != 1)
> +             return -1;
> +     close(d->p[0]);
> +     return d->fn(d->arg);
> +}
> +
> +/*
> + * 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.
> + */
> +static struct lxc_list *idmap_add_id(struct lxc_conf *conf, uid_t uid)
> +{
> +     int hostid_mapped = mapped_hostid(uid, conf);
> +     struct lxc_list *new = NULL, *tmp, *it, *next;
> +     struct id_map *entry;
> +
> +     if (hostid_mapped < 0) {
> +             hostid_mapped = find_unmapped_nsuid(conf);
> +             if (hostid_mapped < 0) {
> +                     ERROR("Could not find free uid to map");
> +                     return NULL;
> +             }
> +             new = malloc(sizeof(*new));
> +             if (!new) {
> +                     ERROR("Out of memory building id map");
> +                     return NULL;
> +             }
> +             entry = malloc(sizeof(*entry));
> +             if (!entry) {
> +                     free(new);
> +                     ERROR("Out of memory building idmap entry");
> +                     return NULL;
> +             }
> +             new->elem = entry;
> +             entry->idtype = ID_TYPE_UID;
> +             entry->nsid = hostid_mapped;
> +             entry->hostid = (unsigned long)uid;
> +             entry->range = 1;
> +             lxc_list_init(new);
> +     }
> +     lxc_list_for_each_safe(it, &conf->id_map, next) {
> +             tmp = malloc(sizeof(*tmp));
> +             if (!tmp)
> +                     goto err;
> +             entry = malloc(sizeof(*entry));
> +             if (!entry) {
> +                     free(tmp);
> +                     goto err;
> +             }
> +             memset(entry, 0, sizeof(*entry));
> +             memcpy(entry, it->elem, sizeof(*entry));
> +             tmp->elem = entry;
> +             if (!new) {
> +                     new = tmp;
> +                     lxc_list_init(new);
> +             } else
> +                     lxc_list_add_tail(new, tmp);
> +     }
> +
> +     return new;
> +
> +err:
> +     ERROR("Out of memory building a new uid map");
> +     lxc_free_idmap(new);
> +     return NULL;
> +}
> +
> +/*
> + * Run a function in a new user namespace.
> + * The caller's euid will be mapped in if it is not already.
> + */
> +int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void *data)
> +{
> +     int ret, pid;
> +     struct userns_fn_data d;
> +     char c = '1';
> +     int p[2];
> +     struct lxc_list *idmap;
> +
> +     process_lock();
> +     ret = pipe(p);
> +     process_unlock();
> +     if (ret < 0) {
> +             SYSERROR("opening pipe");
> +             return -1;
> +     }
> +     d.fn = fn;
> +     d.arg = data;
> +     d.p[0] = p[0];
> +     d.p[1] = p[1];
> +     pid = lxc_clone(run_userns_fn, &d, CLONE_NEWUSER);
> +     if (pid < 0)
> +             goto err;
> +     process_lock();
> +     close(p[0]);
> +     process_unlock();
> +     p[0] = -1;
> +
> +     if ((idmap = idmap_add_id(conf, geteuid())) == NULL) {
> +             ERROR("Error adding self to container uid map");
> +             goto err;
> +     }
> +
> +     ret = lxc_map_ids(idmap, pid);
> +     lxc_free_idmap(idmap);
> +     if (ret < 0) {
> +             ERROR("Error setting up child mappings");
> +             goto err;
> +     }
> +
> +     // kick the child
> +     if (write(p[1], &c, 1) != 1) {
> +             SYSERROR("writing to pipe to child");
> +             goto err;
> +     }
> +
> +     if ((ret = wait_for_pid(pid)) < 0) {
> +             ERROR("Child returned an error: %d\n", ret);
> +             goto err;
> +     }
> +err:
> +     process_lock();
> +     if (p[0] != -1)
> +             close(p[0]);
> +     close(p[1]);
> +     process_unlock();
> +     return -1;
> +}
> diff --git a/src/lxc/conf.h b/src/lxc/conf.h
> index 090c5b3..2ce4843 100644
> --- a/src/lxc/conf.h
> +++ b/src/lxc/conf.h
> @@ -164,6 +164,8 @@ struct id_map {
>       unsigned long hostid, nsid, range;
>  };
>  
> +extern int lxc_free_idmap(struct lxc_list *idmap);
> +
>  /*
>   * Defines a structure containing a pty information for
>   * virtualizing a tty
> @@ -367,4 +369,5 @@ extern int find_unmapped_nsuid(struct lxc_conf *conf);
>  extern int mapped_hostid(int id, struct lxc_conf *conf);
>  extern int chown_mapped_root(char *path, struct lxc_conf *conf);
>  extern int ttys_shift_ids(struct lxc_conf *c);
> +extern int userns_exec_1(struct lxc_conf *conf, int (*fn)(void *), void 
> *data);
>  #endif
> diff --git a/src/lxc/lxc_destroy.c b/src/lxc/lxc_destroy.c
> index 1d1e687..06d2d0d 100644
> --- a/src/lxc/lxc_destroy.c
> +++ b/src/lxc/lxc_destroy.c
> @@ -64,13 +64,6 @@ int main(int argc, char *argv[])
>  {
>       struct lxc_container *c;
>  
> -     /* this is a short term test.  We'll probably want to check for
> -      * write access to lxcpath instead */
> -     if (geteuid()) {
> -             fprintf(stderr, "%s must be run as root\n", argv[0]);
> -             exit(1);
> -     }
> -
>       if (lxc_arguments_parse(&my_args, argc, argv))
>               exit(1);
>  
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index c1f99d5..283fbb5 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -1863,11 +1863,18 @@ out:
>       return bret;
>  }
>  
> +static int lxc_rmdir_onedev_wrapper(void *data)
> +{
> +     char *arg = (char *) data;
> +     return lxc_rmdir_onedev(arg);
> +}
> +
>  // do we want the api to support --force, or leave that to the caller?
>  static bool lxcapi_destroy(struct lxc_container *c)
>  {
>       struct bdev *r = NULL;
>       bool ret = false;
> +     bool am_unpriv;
>  
>       if (!c || !lxcapi_is_defined(c))
>               return false;
> @@ -1881,20 +1888,23 @@ static bool lxcapi_destroy(struct lxc_container *c)
>               goto out;
>       }
>  
> +     am_unpriv = c->lxc_conf && geteuid() != 0 && 
> !lxc_list_empty(&c->lxc_conf->id_map);
> +
>       if (c->lxc_conf && has_snapshots(c)) {
>               ERROR("container %s has dependent snapshots", c->name);
>               goto out;
>       }
>  
> -     if (c->lxc_conf && c->lxc_conf->rootfs.path && 
> c->lxc_conf->rootfs.mount)
> +     if (!am_unpriv && c->lxc_conf->rootfs.path && 
> c->lxc_conf->rootfs.mount) {
>               r = bdev_init(c->lxc_conf->rootfs.path, 
> c->lxc_conf->rootfs.mount, NULL);
> -     if (r) {
> -             if (r->ops->destroy(r) < 0) {
> +             if (r) {
> +                     if (r->ops->destroy(r) < 0) {
> +                             bdev_put(r);
> +                             ERROR("Error destroying rootfs for %s", 
> c->name);
> +                             goto out;
> +                     }
>                       bdev_put(r);
> -                     ERROR("Error destroying rootfs for %s", c->name);
> -                     goto out;
>               }
> -             bdev_put(r);
>       }
>  
>       mod_all_rdeps(c, false);
> @@ -1902,7 +1912,11 @@ static bool lxcapi_destroy(struct lxc_container *c)
>       const char *p1 = lxcapi_get_config_path(c);
>       char *path = alloca(strlen(p1) + strlen(c->name) + 2);
>       sprintf(path, "%s/%s", p1, c->name);
> -     if (lxc_rmdir_onedev(path) < 0) {
> +     if (am_unpriv)
> +             ret = userns_exec_1(c->lxc_conf, lxc_rmdir_onedev_wrapper, 
> path);
> +     else
> +             ret = lxc_rmdir_onedev(path);
> +     if (ret < 0) {
>               ERROR("Error destroying container directory for %s", c->name);
>               goto out;
>       }
> diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> index e2d2639..e80a782 100644
> --- a/src/lxc/utils.c
> +++ b/src/lxc/utils.c
> @@ -66,7 +66,7 @@ static int _recursive_rmdir_onedev(char *dirname, dev_t 
> pdev)
>       process_unlock();
>       if (!dir) {
>               ERROR("%s: failed to open %s", __func__, dirname);
> -             return 0;
> +             return -1;
>       }
>  
>       while (!readdir_r(dir, &dirent, &direntp)) {
> @@ -95,7 +95,7 @@ static int _recursive_rmdir_onedev(char *dirname, dev_t 
> pdev)
>               if (mystat.st_dev != pdev)
>                       continue;
>               if (S_ISDIR(mystat.st_mode)) {
> -                     if (!_recursive_rmdir_onedev(pathname, pdev))
> +                     if (_recursive_rmdir_onedev(pathname, pdev) < 0)
>                               failed=1;
>               } else {
>                       if (unlink(pathname) < 0) {
> @@ -118,17 +118,17 @@ static int _recursive_rmdir_onedev(char *dirname, dev_t 
> pdev)
>               failed=1;
>       }
>  
> -     return !failed;
> +     return failed ? -1 : 0;
>  }
>  
> -/* returns 1 on success, 0 if there were any failures */
> +/* returns 0 on success, -1 if there were any failures */
>  extern int lxc_rmdir_onedev(char *path)
>  {
>       struct stat mystat;
>  
>       if (lstat(path, &mystat) < 0) {
>               ERROR("%s: failed to stat %s", __func__, path);
> -             return 0;
> +             return -1;
>       }
>  
>       return _recursive_rmdir_onedev(path, mystat.st_dev);
> -- 
> 1.8.3.2
> 
> 
> ------------------------------------------------------------------------------
> Shape the Mobile Experience: Free Subscription
> Software experts and developers: Be at the forefront of tech innovation.
> Intel(R) Software Adrenaline delivers strategic insight and game-changing 
> conversations that shape the rapidly evolving mobile landscape. Sign up now. 
> http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
> _______________________________________________
> Lxc-devel mailing list
> Lxc-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/lxc-devel

-- 
Stéphane Graber
Ubuntu developer
http://www.ubuntu.com

Attachment: signature.asc
Description: Digital signature

------------------------------------------------------------------------------
Shape the Mobile Experience: Free Subscription
Software experts and developers: Be at the forefront of tech innovation.
Intel(R) Software Adrenaline delivers strategic insight and game-changing 
conversations that shape the rapidly evolving mobile landscape. Sign up now. 
http://pubads.g.doubleclick.net/gampad/clk?id=63431311&iu=/4140/ostg.clktrk
_______________________________________________
Lxc-devel mailing list
Lxc-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/lxc-devel

Reply via email to