date:20151030

Re: [lxc-devel] [RFC lxc] hooks: put binary hooks into $libdir/lxc/hooks

2015-10-30 Thread Wolfgang Bumiller

> On October 29, 2015 at 10:55 PM Stéphane Graber  wrote:
> Hi,
> 
> As this is an executable binary, shouldn't it be under libexecdir instead?

Ah sorry, must have been some packaging trickery here that confused me (I blame
debian as it ended up in the same location here somehow)
(There hasn't been a 'libexec' dir on my systems for quite a while :-P)

Will resend in a bit.

> On Wed, Oct 28, 2015 at 03:47:17PM +0100, Wolfgang Bumiller wrote:
> > Signed-off-by: Wolfgang Bumiller 
> > ---
> >  configure.ac  | 1 +
> >  hooks/Makefile.am | 3 ++-
> >  2 files changed, 3 insertions(+), 1 deletion(-)
> > 
> > diff --git a/configure.ac b/configure.ac
> > index 874b446..29706cc 100644
> > --- a/configure.ac
> > +++ b/configure.ac
> > @@ -539,6 +539,7 @@ AS_AC_EXPAND(LXCROOTFSMOUNT, "$with_rootfs_path")
> >  AS_AC_EXPAND(LXCTEMPLATEDIR, "$datadir/lxc/templates")
> >  AS_AC_EXPAND(LXCTEMPLATECONFIG, "$datadir/lxc/config")
> >  AS_AC_EXPAND(LXCHOOKDIR, "$datadir/lxc/hooks")
> > +AS_AC_EXPAND(LXCLIBHOOKDIR, "$libdir/lxc/hooks")
> >  AS_AC_EXPAND(LXCINITDIR, "$libexecdir")
> >  AS_AC_EXPAND(LOGPATH, "$with_log_path")
> >  AS_AC_EXPAND(RUNTIME_PATH, "$with_runtime_path")
> > diff --git a/hooks/Makefile.am b/hooks/Makefile.am
> > index 499a2c4..263f62c 100644
> > --- a/hooks/Makefile.am
> > +++ b/hooks/Makefile.am
> > @@ -1,4 +1,5 @@
> >  hooksdir=@LXCHOOKDIR@
> > +libhooksdir=@LXCLIBHOOKDIR@
> >  
> >  hooks_SCRIPTS = \
> > clonehostname \
> > @@ -6,7 +7,7 @@ hooks_SCRIPTS = \
> > ubuntu-cloud-prep \
> > squid-deb-proxy-client
> >  
> > -hooks_PROGRAMS = \
> > +libhooks_PROGRAMS = \
> > unmount-namespace
> >  
> >  unmount_namespace_SOURCES = \
> > -- 
> > 2.1.4
> > 
> > 
> > ___
> > lxc-devel mailing list
> > lxc-devel@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@lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel

___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

[lxc-devel] [PATCH 1/2 v2] Add clone_update_unexp_ovl_dir() function

2015-10-30 Thread Christian Brauner

This functions updates absolute paths for overlay upper- and workdirs so users
can simply clone and start new containers without worrying about absolute paths
in lxc.mount.entry overlay entries.

Signed-off-by: Christian Brauner 
---
 src/lxc/confile.c | 103 ++
 src/lxc/confile.h |   3 ++
 2 files changed, 106 insertions(+)

diff --git a/src/lxc/confile.c b/src/lxc/confile.c
index f7d6814..bc5b7ef 100644
--- a/src/lxc/confile.c
+++ b/src/lxc/confile.c
@@ -2598,6 +2598,109 @@ void clear_unexp_config_line(struct lxc_conf *conf, 
const char *key, bool rm_sub
}
 }
 
+bool clone_update_unexp_ovl_dir(struct lxc_conf *conf, const char *oldpath,
+   const char *newpath, const char *oldname,
+   const char *newname, const char *ovldir)
+{
+   const char *key = "lxc.mount.entry";
+   int ret;
+   char *lstart = conf->unexpanded_config;
+   char *lend;
+   char *p;
+   char *q;
+   size_t newdirlen = strlen(ovldir) + strlen(newpath) + strlen(newname) + 
2;
+   size_t olddirlen = strlen(ovldir) + strlen(oldpath) + strlen(oldname) + 
2;
+   char *olddir = alloca(olddirlen + 1);
+   char *newdir = alloca(newdirlen + 1);
+
+   ret = snprintf(olddir, olddirlen + 1, "%s=%s/%s", ovldir, oldpath, 
oldname);
+   if (ret < 0 || ret >= olddirlen + 1) {
+   ERROR("Bug in %s", __func__);
+   return false;
+   }
+   ret = snprintf(newdir, newdirlen + 1, "%s=%s/%s", ovldir, newpath, 
newname);
+   if (ret < 0 || ret >= newdirlen + 1) {
+   ERROR("Bug in %s", __func__);
+   return false;
+   }
+   if (!conf->unexpanded_config)
+   return true;
+   while (*lstart) {
+   lend = strchr(lstart, '\n');
+   if (!lend)
+   lend = lstart + strlen(lstart);
+   else
+   lend++;
+   if (strncmp(lstart, key, strlen(key)) != 0) {
+   lstart = lend;
+   continue;
+   }
+   p = strchr(lstart + strlen(key), '=');
+   if (!p) {
+   lstart = lend;
+   continue;
+   }
+   p++;
+   while (isblank(*p))
+   p++;
+   if (!*p)
+   return true;
+   /* We check that when an lxc.mount.entry is found the substrings
+* " overlay " or " aufs " are present before we try to update
+* the line. This seems like a bit more safety. We can check for
+* " overlay " and " aufs " since both substrings need to have
+* at least one space before and after them. When the space
+* before or after is missing it is very likely that these
+* substrings are part of a path or something else. So we
+* shouldn't bother to do any further work...
+*/
+   if (!strstr(p, " overlay ") && !strstr(p, " aufs ")) {
+   lstart = lend;
+   continue;
+   }
+   if (!(q = strstr(p, olddir))) {
+   lstart = lend;
+   continue;
+   }
+
+   /* replace the olddir with newdir */
+   if (olddirlen >= newdirlen) {
+   size_t diff = olddirlen - newdirlen;
+   memcpy(q, newdir, newdirlen);
+   if (olddirlen != newdirlen) {
+   memmove(q + newdirlen, q + newdirlen + diff,
+   strlen(q) - newdirlen - diff + 1);
+   lend -= diff;
+   conf->unexpanded_len -= diff;
+   }
+   lstart = lend;
+   } else {
+   char *new;
+   size_t diff = newdirlen - olddirlen;
+   size_t oldlen = conf->unexpanded_len;
+   size_t newlen = oldlen + diff;
+   size_t poffset = q - conf->unexpanded_config;
+   new = realloc(conf->unexpanded_config, newlen + 1);
+   if (!new) {
+   ERROR("Out of memory");
+   return false;
+   }
+   conf->unexpanded_len = newlen;
+   conf->unexpanded_alloced = newlen + 1;
+   new[newlen-1] = '\0';
+   lend = new + (lend - conf->unexpanded_config);
+   /* move over the remainder to make room for the newdir 
*/
+   memmove(new + poffset + newdirlen,
+   new + poffset +

[lxc-devel] [PATCH 2/2 v2] Update absolute paths for overlay and aufs mounts

2015-10-30 Thread Christian Brauner

When using overlay and aufs mounts with lxc.mount.entry users have to specify
absolute paths for upperdir and workdir which will then get created
automatically by mount_entry_create_overlay_dirs() and
mount_entry_create_aufs_dirs() in conf.c. When we clone a container with
overlay or aufs lxc.mount.entry entries we need to update these absolute paths.
In order to do this we add the function update_union_mount_entry_paths() in
lxccontainer.c. The function updates the mounts in two locations:

1) lxc_conf->mount_list

and

2) lxc_conf->unexpanded_config (by calling clone_update_unexp_ovl_dir())

If we were to only update 2) we would end up with wrong upperdir and workdir
mounts as the absolute paths would still point to the container that serves as
the base for the clone. If we were to only update 1) we would end up with wrong
upperdir and workdir lxc.mount.entry entries in the clone's config as the
absolute paths in upperdir and workdir would still point to the container that
serves as the base for the clone. Updating both will get the job done.

NOTE: This function does not sanitize paths apart from removing trailing
slashes. (So when a user specifies //home//someone/// it will be cleaned to
//home//someone. This is the minimal path cleansing which is also done by
lxc_container_new().) But the mount_entry_create_overlay_dirs() and
mount_entry_create_aufs_dirs() functions both try to be extremely strict about
when to create upperdirs and workdirs. They will only accept sanitized paths,
i.e. they require /home/someone. I think this is a (safety) virtue and we
should consider sanitizing paths in general. In short:
update_union_mount_entry_paths() does update all absolute paths to the new
container but mount_entry_create_overlay_dirs() and
mount_entry_create_aufs_dirs() will still refuse to create upperdir and workdir
when the updated path is unclean. This happens easily when e.g. a user calls
lxc-clone -o OLD -n NEW -P //home//chb///.

Signed-off-by: Christian Brauner 
---
 src/lxc/lxccontainer.c | 97 ++
 1 file changed, 97 insertions(+)

diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
index 42e23e7..8edf6a2 100644
--- a/src/lxc/lxccontainer.c
+++ b/src/lxc/lxccontainer.c
@@ -2894,6 +2894,99 @@ static int create_file_dirname(char *path, struct 
lxc_conf *conf)
return ret;
 }
 
+/* When we clone a container with overlay or aufs lxc.mount.entry entries we
+*  need to update these absolute paths. In order to do this we add the function
+*  update_union_mount_entry_paths() in lxccontainer.c. The function operates on
+*  c->lxc_conf->unexpanded_config instead of the intuitively plausible
+*  c->lxc_conf->mount_list because the latter also contains mounts from other
+*  files as well as generic mounts. */
+static int update_union_mount_entry_paths(struct lxc_conf *lxc_conf,
+ const char *lxc_path,
+ const char *lxc_name,
+ const char *newpath,
+ const char *newname)
+{
+   char new_upper[MAXPATHLEN];
+   char new_work[MAXPATHLEN];
+   char old_upper[MAXPATHLEN];
+   char old_work[MAXPATHLEN];
+   char *cleanpath = NULL;
+   int i;
+   int fret = -1;
+   int ret = 0;
+   struct lxc_list *iterator;
+   const char *ovl_dirs[] = {"br", "upperdir", "workdir"};
+
+   cleanpath = strdup(newpath);
+   if (!cleanpath)
+   goto err;
+
+   remove_trailing_slashes(cleanpath);
+
+   for (i = 0; i < sizeof(ovl_dirs) / sizeof(ovl_dirs[0]); i++) {
+   if (!clone_update_unexp_ovl_dir(lxc_conf, lxc_path, newpath,
+   lxc_name, newname, ovl_dirs[i]))
+   goto err;
+   }
+
+   ret = snprintf(old_work, MAXPATHLEN, "workdir=%s/%s", lxc_path, 
lxc_name);
+   if (ret < 0 || ret >= MAXPATHLEN)
+   goto err;
+
+   ret = snprintf(new_work, MAXPATHLEN, "workdir=%s/%s", cleanpath, 
newname);
+   if (ret < 0 || ret >= MAXPATHLEN)
+   goto err;
+
+   lxc_list_for_each(iterator, _conf->mount_list) {
+   char *mnt_entry = NULL;
+   char *new_mnt_entry = NULL;
+   char *tmp = NULL;
+   char *tmp_mnt_entry = NULL;
+   mnt_entry = iterator->elem;
+
+   if (strstr(mnt_entry, "overlay"))
+   tmp = "upperdir";
+   else if (strstr(mnt_entry, "aufs"))
+   tmp = "br";
+
+   if (!tmp)
+   continue;
+
+   ret = snprintf(old_upper, MAXPATHLEN, "%s=%s/%s", tmp, 
lxc_path, lxc_name);
+   if (ret < 0 || ret >= MAXPATHLEN)
+   goto err;
+
+   ret =

Re: [lxc-devel] LXC security issue - affects all supported releases

2015-10-30 Thread Serge Hallyn

Quoting Thomas Moschny (thomas.mosc...@gmail.com):
> 2015-10-24 4:37 GMT+02:00 Serge Hallyn :
> > Quoting Thomas Moschny (thomas.mosc...@gmail.com):
> >> 2015-10-02 15:50 GMT+02:00 Serge Hallyn :
> >> > Can you tell me what happens when you do an openat with
> >> > O_PATH?  Does it simply return < 0?  If so then I think this is all ok.
> >>
> >> As far as I can see, it behaves as if O_PATH wasn't given at all - so
> >> it doesn't really make a difference whether one "copies" the value of
> >> O_PATH over from elsewhere, or defines it to 0. Both ways feel hackish
> >> though. The second openat() call in open_if_safe() should fail anyway,
> >> so...
> >>
> >> > (since an openat without O_PATH already failed, you shouldn't be allowed
> >> > to mount on it in this case)
> >>
> >> ... a really clean solution would be to #ifdef that code in
> >> open_if_safe(), so it compiles cleanly.
> >
> > Heh, a really clean solution would be a mountfd system call :)
> >
> > If you can send a patch along the lines of what you'r thinking that
> > would be great.
> 
> From a very pragmatic point of view, this commit:
> 
> https://github.com/lxc/lxc/commit/27ec06f9
> 
> already sort of "fixes" the issue also on RHEL/CentOS6...
> 
> What I had in mind would look more like this:
> 
> diff --git a/src/lxc/utils.c b/src/lxc/utils.c
> index 214c5a8..264b554 100644
> --- a/src/lxc/utils.c
> +++ b/src/lxc/utils.c
> @@ -1226,6 +1226,7 @@ static int open_if_safe(int dirfd, const char *nextpath)
> if (errno == ELOOP)
> return newfd;
> 
> +#ifndef O_PATH

Did you mean ifdef?

> if (errno == EPERM || errno == EACCES) {
> /* we're not root (cause we got EPERM) so
>try opening with O_PATH */
> @@ -1242,6 +1243,7 @@ static int open_if_safe(int dirfd, const char *nextpath)
> }
> }
> }
> +#endif
> 
> return newfd;
>  }

I'm fine either way.  If you send a patch I'll let stgraber decide.

Your way has the advantage of being clearer what's going on.  (And should
then add a comment by the ifdef saying "if o_path isn't defined, there are
no user namespaces so we expect to be real root")
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

[lxc-devel] [lxc/lxc] 427d42: arch template: Fix systemd-sysctl service

2015-10-30 Thread GitHub

  Branch: refs/heads/master
  Home:   https://github.com/lxc/lxc
  Commit: 427d42930d99f93bf78c61ec9f555dd883c5039e
  https://github.com/lxc/lxc/commit/427d42930d99f93bf78c61ec9f555dd883c5039e
  Author: Jakub Sztandera 
  Date:   2015-10-30 (Fri, 30 Oct 2015)

  Changed paths:
M templates/lxc-archlinux.in

  Log Message:
  ---
  arch template: Fix systemd-sysctl service

The systemd-sysctl service includes condition that /proc/sys/ has to be 
read-write.
In lxc only /proc/sys/net/ is read-write which causes the condition to fail and 
service not to run.
This patch changes the check to /proc/sys/net/ and makes the service apply only 
rules that are in net tree.

Signed-off-by: Jakub Sztandera 


  Commit: 208a29f10c08c2c6c5533347c2fdd9473e7fc4fc
  https://github.com/lxc/lxc/commit/208a29f10c08c2c6c5533347c2fdd9473e7fc4fc
  Author: Stéphane Graber 
  Date:   2015-10-30 (Fri, 30 Oct 2015)

  Changed paths:
M templates/lxc-archlinux.in

  Log Message:
  ---
  Merge pull request #683 from Kubuxu/patch-1

arch template: Fix systemd-sysctl service


Compare: https://github.com/lxc/lxc/compare/407fef433c1c...208a29f10c08___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Re: [lxc-devel] [PATCH 1/2 v2] Add clone_update_unexp_ovl_dir() function

2015-10-30 Thread Christian Brauner

On Oct 30, 2015 4:08 PM, "Serge Hallyn"  wrote:
>
> Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > This functions updates absolute paths for overlay upper- and workdirs
so users
> > can simply clone and start new containers without worrying about
absolute paths
> > in lxc.mount.entry overlay entries.
> >
> > Signed-off-by: Christian Brauner 
> > ---
> >  src/lxc/confile.c | 103
++
> >  src/lxc/confile.h |   3 ++
> >  2 files changed, 106 insertions(+)
> >
> > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > index f7d6814..bc5b7ef 100644
> > --- a/src/lxc/confile.c
> > +++ b/src/lxc/confile.c
> > @@ -2598,6 +2598,109 @@ void clear_unexp_config_line(struct lxc_conf
*conf, const char *key, bool rm_sub
> >   }
> >  }
> >
> > +bool clone_update_unexp_ovl_dir(struct lxc_conf *conf, const char
*oldpath,
> > + const char *newpath, const char *oldname,
> > + const char *newname, const char *ovldir)
> > +{
> > + const char *key = "lxc.mount.entry";
> > + int ret;
> > + char *lstart = conf->unexpanded_config;
> > + char *lend;
> > + char *p;
> > + char *q;
> > + size_t newdirlen = strlen(ovldir) + strlen(newpath) +
strlen(newname) + 2;
> > + size_t olddirlen = strlen(ovldir) + strlen(oldpath) +
strlen(oldname) + 2;
> > + char *olddir = alloca(olddirlen + 1);
> > + char *newdir = alloca(newdirlen + 1);
> > +
> > + ret = snprintf(olddir, olddirlen + 1, "%s=%s/%s", ovldir,
oldpath, oldname);
> > + if (ret < 0 || ret >= olddirlen + 1) {
> > + ERROR("Bug in %s", __func__);
> > + return false;
> > + }
> > + ret = snprintf(newdir, newdirlen + 1, "%s=%s/%s", ovldir,
newpath, newname);
> > + if (ret < 0 || ret >= newdirlen + 1) {
> > + ERROR("Bug in %s", __func__);
> > + return false;
> > + }
> > + if (!conf->unexpanded_config)
> > + return true;
> > + while (*lstart) {
> > + lend = strchr(lstart, '\n');
> > + if (!lend)
> > + lend = lstart + strlen(lstart);
> > + else
> > + lend++;
> > + if (strncmp(lstart, key, strlen(key)) != 0) {
> > + lstart = lend;
> > + continue;
> > + }
> > + p = strchr(lstart + strlen(key), '=');
> > + if (!p) {
> > + lstart = lend;
> > + continue;
> > + }
> > + p++;
> > + while (isblank(*p))
> > + p++;
> > + if (!*p)
> > + return true;
>
> I think you should
>
> if (p >= lend)
> continue;
>
> (and then you can skip the !*p check above as this should catch that)
>
> What you have will only get confused on invalid configs, but this would
> be stricter and make me feel better.

Agreed. Than we should change that in the other function
(clone_update_unexp_hooks()) as well.

>
> > + /* We check that when an lxc.mount.entry is found the
substrings
> > +  * " overlay " or " aufs " are present before we try to
update
> > +  * the line. This seems like a bit more safety. We can
check for
> > +  * " overlay " and " aufs " since both substrings need to
have
> > +  * at least one space before and after them. When the
space
> > +  * before or after is missing it is very likely that these
> > +  * substrings are part of a path or something else. So we
> > +  * shouldn't bother to do any further work...
> > +  */
> > + if (!strstr(p, " overlay ") && !strstr(p, " aufs ")) {
> > + lstart = lend;
> > + continue;
> > + }
>
> But this will find " overlay " in any subsequent lines too right?  Don't
you
> need to limit your search to the line?  Maybe by saving *lend and setting
> *lend = '\0'?

But we do want to check in all lines, don't we? If we do not find the
substring in the current line we skip it and check the next one.

>
> > + if (!(q = strstr(p, olddir))) {
> > + lstart = lend;
> > + continue;
> > + }
> > +
> > + /* replace the olddir with newdir */
> > + if (olddirlen >= newdirlen) {
> > + size_t diff = olddirlen - newdirlen;
> > + memcpy(q, newdir, newdirlen);
> > + if (olddirlen != newdirlen) {
> > + memmove(q + newdirlen, q + newdirlen +
diff,
> > + strlen(q) - newdirlen - diff + 1);
> > + lend -= diff;
> > + conf->unexpanded_len -=

Re: [lxc-devel] [PATCH 2/2 v2] Update absolute paths for overlay and aufs mounts

2015-10-30 Thread Serge Hallyn

Quoting Christian Brauner (christianvanbrau...@gmail.com):
> When using overlay and aufs mounts with lxc.mount.entry users have to specify
> absolute paths for upperdir and workdir which will then get created
> automatically by mount_entry_create_overlay_dirs() and
> mount_entry_create_aufs_dirs() in conf.c. When we clone a container with
> overlay or aufs lxc.mount.entry entries we need to update these absolute 
> paths.
> In order to do this we add the function update_union_mount_entry_paths() in
> lxccontainer.c. The function updates the mounts in two locations:
> 
> 1) lxc_conf->mount_list
> 
> and
> 
> 2) lxc_conf->unexpanded_config (by calling 
> clone_update_unexp_ovl_dir())
> 
> If we were to only update 2) we would end up with wrong upperdir and workdir
> mounts as the absolute paths would still point to the container that serves as
> the base for the clone. If we were to only update 1) we would end up with 
> wrong
> upperdir and workdir lxc.mount.entry entries in the clone's config as the
> absolute paths in upperdir and workdir would still point to the container that
> serves as the base for the clone. Updating both will get the job done.
> 
> NOTE: This function does not sanitize paths apart from removing trailing
> slashes. (So when a user specifies //home//someone/// it will be cleaned to
> //home//someone. This is the minimal path cleansing which is also done by
> lxc_container_new().) But the mount_entry_create_overlay_dirs() and
> mount_entry_create_aufs_dirs() functions both try to be extremely strict about
> when to create upperdirs and workdirs. They will only accept sanitized paths,
> i.e. they require /home/someone. I think this is a (safety) virtue and we
> should consider sanitizing paths in general. In short:
> update_union_mount_entry_paths() does update all absolute paths to the new
> container but mount_entry_create_overlay_dirs() and
> mount_entry_create_aufs_dirs() will still refuse to create upperdir and 
> workdir
> when the updated path is unclean. This happens easily when e.g. a user calls
> lxc-clone -o OLD -n NEW -P //home//chb///.
> 
> Signed-off-by: Christian Brauner 

Acked-by: Serge E. Hallyn 

> ---
>  src/lxc/lxccontainer.c | 97 
> ++
>  1 file changed, 97 insertions(+)
> 
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 42e23e7..8edf6a2 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -2894,6 +2894,99 @@ static int create_file_dirname(char *path, struct 
> lxc_conf *conf)
>   return ret;
>  }
>  
> +/* When we clone a container with overlay or aufs lxc.mount.entry entries we
> +*  need to update these absolute paths. In order to do this we add the 
> function
> +*  update_union_mount_entry_paths() in lxccontainer.c. The function operates 
> on
> +*  c->lxc_conf->unexpanded_config instead of the intuitively plausible
> +*  c->lxc_conf->mount_list because the latter also contains mounts from other
> +*  files as well as generic mounts. */
> +static int update_union_mount_entry_paths(struct lxc_conf *lxc_conf,
> +   const char *lxc_path,
> +   const char *lxc_name,
> +   const char *newpath,
> +   const char *newname)
> +{
> + char new_upper[MAXPATHLEN];
> + char new_work[MAXPATHLEN];
> + char old_upper[MAXPATHLEN];
> + char old_work[MAXPATHLEN];
> + char *cleanpath = NULL;
> + int i;
> + int fret = -1;
> + int ret = 0;
> + struct lxc_list *iterator;
> + const char *ovl_dirs[] = {"br", "upperdir", "workdir"};
> +
> + cleanpath = strdup(newpath);
> + if (!cleanpath)
> + goto err;
> +
> + remove_trailing_slashes(cleanpath);
> +

Would be wortha comment here saying "we have to update the unexpanded
configuration separately from the expanded one."

> + for (i = 0; i < sizeof(ovl_dirs) / sizeof(ovl_dirs[0]); i++) {
> + if (!clone_update_unexp_ovl_dir(lxc_conf, lxc_path, newpath,
> + lxc_name, newname, ovl_dirs[i]))
> + goto err;
> + }
> +
> + ret = snprintf(old_work, MAXPATHLEN, "workdir=%s/%s", lxc_path, 
> lxc_name);
> + if (ret < 0 || ret >= MAXPATHLEN)
> + goto err;
> +
> + ret = snprintf(new_work, MAXPATHLEN, "workdir=%s/%s", cleanpath, 
> newname);
> + if (ret < 0 || ret >= MAXPATHLEN)
> + goto err;
> +
> + lxc_list_for_each(iterator, _conf->mount_list) {
> + char *mnt_entry = NULL;
> + char *new_mnt_entry = NULL;
> + char *tmp = NULL;
> + char *tmp_mnt_entry = NULL;
> + mnt_entry = iterator->elem;
> +
> + if (strstr(mnt_entry, "overlay"))
> + tmp = "upperdir";
> +

[lxc-devel] Fwd: Re: [PATCH 2/2 v2] Update absolute paths for overlay and aufs mounts

2015-10-30 Thread Christian Brauner

-- Forwarded message --
From: "Christian Brauner" 
Date: Oct 30, 2015 4:27 PM
Subject: Re: [PATCH 2/2 v2] Update absolute paths for overlay and aufs
mounts
To: "Serge E. Hallyn" 
Cc: "lxc-dev" 

>
> On Oct 30, 2015 4:22 PM, "Serge Hallyn"  wrote:
> >
> > Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > > When using overlay and aufs mounts with lxc.mount.entry users have to
specify
> > > absolute paths for upperdir and workdir which will then get created
> > > automatically by mount_entry_create_overlay_dirs() and
> > > mount_entry_create_aufs_dirs() in conf.c. When we clone a container
with
> > > overlay or aufs lxc.mount.entry entries we need to update these
absolute paths.
> > > In order to do this we add the function
update_union_mount_entry_paths() in
> > > lxccontainer.c. The function updates the mounts in two locations:
> > >
> > > 1) lxc_conf->mount_list
> > >
> > > and
> > >
> > > 2) lxc_conf->unexpanded_config (by calling
clone_update_unexp_ovl_dir())
> > >
> > > If we were to only update 2) we would end up with wrong upperdir and
workdir
> > > mounts as the absolute paths would still point to the container that
serves as
> > > the base for the clone. If we were to only update 1) we would end up
with wrong
> > > upperdir and workdir lxc.mount.entry entries in the clone's config as
the
> > > absolute paths in upperdir and workdir would still point to the
container that
> > > serves as the base for the clone. Updating both will get the job done.
> > >
> > > NOTE: This function does not sanitize paths apart from removing
trailing
> > > slashes. (So when a user specifies //home//someone/// it will be
cleaned to
> > > //home//someone. This is the minimal path cleansing which is also
done by
> > > lxc_container_new().) But the mount_entry_create_overlay_dirs() and
> > > mount_entry_create_aufs_dirs() functions both try to be extremely
strict about
> > > when to create upperdirs and workdirs. They will only accept
sanitized paths,
> > > i.e. they require /home/someone. I think this is a (safety) virtue
and we
> > > should consider sanitizing paths in general. In short:
> > > update_union_mount_entry_paths() does update all absolute paths to
the new
> > > container but mount_entry_create_overlay_dirs() and
> > > mount_entry_create_aufs_dirs() will still refuse to create upperdir
and workdir
> > > when the updated path is unclean. This happens easily when e.g. a
user calls
> > > lxc-clone -o OLD -n NEW -P //home//chb///.
> > >
> > > Signed-off-by: Christian Brauner 
> >
> > Acked-by: Serge E. Hallyn 
> >
> > > ---
> > >  src/lxc/lxccontainer.c | 97
++
> > >  1 file changed, 97 insertions(+)
> > >
> > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > > index 42e23e7..8edf6a2 100644
> > > --- a/src/lxc/lxccontainer.c
> > > +++ b/src/lxc/lxccontainer.c
> > > @@ -2894,6 +2894,99 @@ static int create_file_dirname(char *path,
struct lxc_conf *conf)
> > >   return ret;
> > >  }
> > >
> > > +/* When we clone a container with overlay or aufs lxc.mount.entry
entries we
> > > +*  need to update these absolute paths. In order to do this we add
the function
> > > +*  update_union_mount_entry_paths() in lxccontainer.c. The function
operates on
> > > +*  c->lxc_conf->unexpanded_config instead of the intuitively
plausible
> > > +*  c->lxc_conf->mount_list because the latter also contains mounts
from other
> > > +*  files as well as generic mounts. */
> > > +static int update_union_mount_entry_paths(struct lxc_conf *lxc_conf,
> > > +   const char *lxc_path,
> > > +   const char *lxc_name,
> > > +   const char *newpath,
> > > +   const char *newname)
> > > +{
> > > + char new_upper[MAXPATHLEN];
> > > + char new_work[MAXPATHLEN];
> > > + char old_upper[MAXPATHLEN];
> > > + char old_work[MAXPATHLEN];
> > > + char *cleanpath = NULL;
> > > + int i;
> > > + int fret = -1;
> > > + int ret = 0;
> > > + struct lxc_list *iterator;
> > > + const char *ovl_dirs[] = {"br", "upperdir", "workdir"};
> > > +
> > > + cleanpath = strdup(newpath);
> > > + if (!cleanpath)
> > > + goto err;
> > > +
> > > + remove_trailing_slashes(cleanpath);
> > > +
> >
> > Would be wortha comment here saying "we have to update the unexpanded
> > configuration separately from the expanded one."

I can insert it, add your Acked-by line to the cover letter and leave a
short reminder in the cover letter that I only inserted a comment per your
request so you know what's going on. Ok?

> >
> > > + for (i = 0; i < sizeof(ovl_dirs) / sizeof(ovl_dirs[0]); i++) {
> > > +

Re: [lxc-devel] [PATCH 2/2 v2] Update absolute paths for overlay and aufs mounts

2015-10-30 Thread Serge Hallyn

Quoting Christian Brauner (christianvanbrau...@gmail.com):
> On Oct 30, 2015 4:22 PM, "Serge Hallyn"  wrote:
> >
> > Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > > When using overlay and aufs mounts with lxc.mount.entry users have to
> specify
> > > absolute paths for upperdir and workdir which will then get created
> > > automatically by mount_entry_create_overlay_dirs() and
> > > mount_entry_create_aufs_dirs() in conf.c. When we clone a container with
> > > overlay or aufs lxc.mount.entry entries we need to update these
> absolute paths.
> > > In order to do this we add the function
> update_union_mount_entry_paths() in
> > > lxccontainer.c. The function updates the mounts in two locations:
> > >
> > > 1) lxc_conf->mount_list
> > >
> > > and
> > >
> > > 2) lxc_conf->unexpanded_config (by calling
> clone_update_unexp_ovl_dir())
> > >
> > > If we were to only update 2) we would end up with wrong upperdir and
> workdir
> > > mounts as the absolute paths would still point to the container that
> serves as
> > > the base for the clone. If we were to only update 1) we would end up
> with wrong
> > > upperdir and workdir lxc.mount.entry entries in the clone's config as
> the
> > > absolute paths in upperdir and workdir would still point to the
> container that
> > > serves as the base for the clone. Updating both will get the job done.
> > >
> > > NOTE: This function does not sanitize paths apart from removing trailing
> > > slashes. (So when a user specifies //home//someone/// it will be
> cleaned to
> > > //home//someone. This is the minimal path cleansing which is also done
> by
> > > lxc_container_new().) But the mount_entry_create_overlay_dirs() and
> > > mount_entry_create_aufs_dirs() functions both try to be extremely
> strict about
> > > when to create upperdirs and workdirs. They will only accept sanitized
> paths,
> > > i.e. they require /home/someone. I think this is a (safety) virtue and
> we
> > > should consider sanitizing paths in general. In short:
> > > update_union_mount_entry_paths() does update all absolute paths to the
> new
> > > container but mount_entry_create_overlay_dirs() and
> > > mount_entry_create_aufs_dirs() will still refuse to create upperdir and
> workdir
> > > when the updated path is unclean. This happens easily when e.g. a user
> calls
> > > lxc-clone -o OLD -n NEW -P //home//chb///.
> > >
> > > Signed-off-by: Christian Brauner 
> >
> > Acked-by: Serge E. Hallyn 
> >
> > > ---
> > >  src/lxc/lxccontainer.c | 97
> ++
> > >  1 file changed, 97 insertions(+)
> > >
> > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > > index 42e23e7..8edf6a2 100644
> > > --- a/src/lxc/lxccontainer.c
> > > +++ b/src/lxc/lxccontainer.c
> > > @@ -2894,6 +2894,99 @@ static int create_file_dirname(char *path,
> struct lxc_conf *conf)
> > >   return ret;
> > >  }
> > >
> > > +/* When we clone a container with overlay or aufs lxc.mount.entry
> entries we
> > > +*  need to update these absolute paths. In order to do this we add the
> function
> > > +*  update_union_mount_entry_paths() in lxccontainer.c. The function
> operates on
> > > +*  c->lxc_conf->unexpanded_config instead of the intuitively plausible
> > > +*  c->lxc_conf->mount_list because the latter also contains mounts
> from other
> > > +*  files as well as generic mounts. */
> > > +static int update_union_mount_entry_paths(struct lxc_conf *lxc_conf,
> > > +   const char *lxc_path,
> > > +   const char *lxc_name,
> > > +   const char *newpath,
> > > +   const char *newname)
> > > +{
> > > + char new_upper[MAXPATHLEN];
> > > + char new_work[MAXPATHLEN];
> > > + char old_upper[MAXPATHLEN];
> > > + char old_work[MAXPATHLEN];
> > > + char *cleanpath = NULL;
> > > + int i;
> > > + int fret = -1;
> > > + int ret = 0;
> > > + struct lxc_list *iterator;
> > > + const char *ovl_dirs[] = {"br", "upperdir", "workdir"};
> > > +
> > > + cleanpath = strdup(newpath);
> > > + if (!cleanpath)
> > > + goto err;
> > > +
> > > + remove_trailing_slashes(cleanpath);
> > > +
> >
> > Would be wortha comment here saying "we have to update the unexpanded
> > configuration separately from the expanded one."
> 
> I can insert it, add your Acked-by line to the cover letter and leave a
> short reminder in the cover letter that I only inserted a comment per your
> request so you know what's going on. Ok?

Yup, thanks!

___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Re: [lxc-devel] [PATCH 1/2 v2] Add clone_update_unexp_ovl_dir() function

2015-10-30 Thread Serge Hallyn

Quoting Christian Brauner (christianvanbrau...@gmail.com):
> On Oct 30, 2015 4:08 PM, "Serge Hallyn"  wrote:
> >
> > Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > > This functions updates absolute paths for overlay upper- and workdirs
> so users
> > > can simply clone and start new containers without worrying about
> absolute paths
> > > in lxc.mount.entry overlay entries.
> > >
> > > Signed-off-by: Christian Brauner 
> > > ---
> > >  src/lxc/confile.c | 103
> ++
> > >  src/lxc/confile.h |   3 ++
> > >  2 files changed, 106 insertions(+)
> > >
> > > diff --git a/src/lxc/confile.c b/src/lxc/confile.c
> > > index f7d6814..bc5b7ef 100644
> > > --- a/src/lxc/confile.c
> > > +++ b/src/lxc/confile.c
> > > @@ -2598,6 +2598,109 @@ void clear_unexp_config_line(struct lxc_conf
> *conf, const char *key, bool rm_sub
> > >   }
> > >  }
> > >
> > > +bool clone_update_unexp_ovl_dir(struct lxc_conf *conf, const char
> *oldpath,
> > > + const char *newpath, const char *oldname,
> > > + const char *newname, const char *ovldir)
> > > +{
> > > + const char *key = "lxc.mount.entry";
> > > + int ret;
> > > + char *lstart = conf->unexpanded_config;
> > > + char *lend;
> > > + char *p;
> > > + char *q;
> > > + size_t newdirlen = strlen(ovldir) + strlen(newpath) +
> strlen(newname) + 2;
> > > + size_t olddirlen = strlen(ovldir) + strlen(oldpath) +
> strlen(oldname) + 2;
> > > + char *olddir = alloca(olddirlen + 1);
> > > + char *newdir = alloca(newdirlen + 1);
> > > +
> > > + ret = snprintf(olddir, olddirlen + 1, "%s=%s/%s", ovldir,
> oldpath, oldname);
> > > + if (ret < 0 || ret >= olddirlen + 1) {
> > > + ERROR("Bug in %s", __func__);
> > > + return false;
> > > + }
> > > + ret = snprintf(newdir, newdirlen + 1, "%s=%s/%s", ovldir,
> newpath, newname);
> > > + if (ret < 0 || ret >= newdirlen + 1) {
> > > + ERROR("Bug in %s", __func__);
> > > + return false;
> > > + }
> > > + if (!conf->unexpanded_config)
> > > + return true;
> > > + while (*lstart) {
> > > + lend = strchr(lstart, '\n');
> > > + if (!lend)
> > > + lend = lstart + strlen(lstart);
> > > + else
> > > + lend++;
> > > + if (strncmp(lstart, key, strlen(key)) != 0) {
> > > + lstart = lend;
> > > + continue;
> > > + }
> > > + p = strchr(lstart + strlen(key), '=');
> > > + if (!p) {
> > > + lstart = lend;
> > > + continue;
> > > + }
> > > + p++;
> > > + while (isblank(*p))
> > > + p++;
> > > + if (!*p)
> > > + return true;
> >
> > I think you should
> >
> > if (p >= lend)
> > continue;
> >
> > (and then you can skip the !*p check above as this should catch that)
> >
> > What you have will only get confused on invalid configs, but this would
> > be stricter and make me feel better.
> 
> Agreed. Than we should change that in the other function
> (clone_update_unexp_hooks()) as well.
> 
> >
> > > + /* We check that when an lxc.mount.entry is found the
> substrings
> > > +  * " overlay " or " aufs " are present before we try to
> update
> > > +  * the line. This seems like a bit more safety. We can
> check for
> > > +  * " overlay " and " aufs " since both substrings need to
> have
> > > +  * at least one space before and after them. When the
> space
> > > +  * before or after is missing it is very likely that these
> > > +  * substrings are part of a path or something else. So we
> > > +  * shouldn't bother to do any further work...
> > > +  */
> > > + if (!strstr(p, " overlay ") && !strstr(p, " aufs ")) {
> > > + lstart = lend;
> > > + continue;
> > > + }
> >
> > But this will find " overlay " in any subsequent lines too right?  Don't
> you
> > need to limit your search to the line?  Maybe by saving *lend and setting
> > *lend = '\0'?
> 
> But we do want to check in all lines, don't we? If we do not find the
> substring in the current line we skip it and check the next one.

Right, so the you set *lend back to the saved *lend before going to the
top of the loop - there's someplace else i lxc that does this.
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel

Re: [lxc-devel] [RFC lxc] hooks: put binary hooks into $libdir/lxc/hooks

[lxc-devel] [PATCH 1/2 v2] Add clone_update_unexp_ovl_dir() function

[lxc-devel] [PATCH 2/2 v2] Update absolute paths for overlay and aufs mounts

Re: [lxc-devel] LXC security issue - affects all supported releases

[lxc-devel] [lxc/lxc] 427d42: arch template: Fix systemd-sysctl service

Re: [lxc-devel] [PATCH 1/2 v2] Add clone_update_unexp_ovl_dir() function

Re: [lxc-devel] [PATCH 2/2 v2] Update absolute paths for overlay and aufs mounts

[lxc-devel] Fwd: Re: [PATCH 2/2 v2] Update absolute paths for overlay and aufs mounts

Re: [lxc-devel] [PATCH 2/2 v2] Update absolute paths for overlay and aufs mounts

Re: [lxc-devel] [PATCH 1/2 v2] Add clone_update_unexp_ovl_dir() function

10 matches

Site Navigation

Mail list logo

Footer information