subject:"\\\\\\\[lxc\\\\\\\-devel\\\\\\\] \\\\\\\[PATCH v4\\\\\\\] Parse rootfs\\\\\\\->path"

Re: [lxc-devel] [PATCH v4] Parse rootfs->path

2015-10-22 Thread Christian Brauner

On Thu, Oct 22, 2015 at 03:33:30AM +, Serge Hallyn wrote:
> Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > The mount_entry_overlay_dirs() and mount_entry_aufs_dirs() functions create
> > workdirs and upperdirs for overlay and aufs lxc.mount.entry entries. They 
> > try
> > to make sure that the workdirs and upperdirs can only be created under the
> > containerdir (e.g. /path/to/the/container/CONTAINERNAME). In order to do 
> > this
> > the right hand side of
> > 
> > if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> > (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> > 
> > was thought to check if the rootfs->path is not present in the workdir and
> > upperdir mount options. But the current check is bogus since it will be
> > trivially true whenever the container is a block-dev or overlay or aufs 
> > backed
> > since the rootfs->path will then have a form like e.g.
> > 
> > overlayfs:/some/path:/some/other/path
> > 
> > This patch adds the function ovl_get_rootfs_dir() which parses rootfs->path 
> > by
> > searching backwards for the first occurrence of the delimiter pair ":/". We 
> > do
> > not simply search for ":" since it might be used in path names. If ":/" is 
> > not
> > found we assume the container is directory backed and simply return
> > strdup(rootfs->path).
> > 
> > Signed-off-by: Christian Brauner 
> > ---
> >  src/lxc/conf.c | 101 
> > +++--
> >  1 file changed, 76 insertions(+), 25 deletions(-)
> > 
> > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > index 16a62f8..fc072f7 100644
> > --- a/src/lxc/conf.c
> > +++ b/src/lxc/conf.c
> > @@ -1815,12 +1815,51 @@ static void cull_mntent_opt(struct mntent *mntent)
> > }
> >  }
> >  
> > +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t *rootfslen)
> > +{
> > +   char *rootfsdir = NULL;
> > +   char *s1 = NULL;
> > +   char *s2 = NULL;
> > +   char *s3 = NULL;
> > +   size_t s3len = 0;
> > +
> > +   if (!rootfs_path || !rootfslen)
> > +   return NULL;
> > +
> > +   s1 = strdup(rootfs_path);
> > +   if (!s1)
> > +   return NULL;
> > +
> > +   if ((s2 = strstr(s1, ":/"))) {
> > +   s2 = s2 + 1;
> > +   if ((s3 = strstr(s2, ":/"))) {
> > +   s3len = strlen(s3);
> > +   s2[strlen(s2) - s3len] = '\0';
> 
> patch looks good, just one q - is there any reaso nnot to
> just do
> 
>   *s3 = '\0';
> 
> here?

Nope. (That's what staring at the screen for x hours gets you. :) Updated
version to follow.

> 
> > +   }
> > +   rootfsdir = strdup(s2);
> > +   if (!rootfsdir) {
> > +   free(s1);
> > +   return NULL;
> > +   }
> > +   }
> > +
> > +   if (!rootfsdir)
> > +   rootfsdir = s1;
> > +   else
> > +   free(s1);
> > +
> > +   *rootfslen = strlen(rootfsdir);
> > +
> > +   return rootfsdir;
> > +}
> > +
> >  static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
> >const struct lxc_rootfs *rootfs,
> >const char *lxc_name,
> >const char *lxc_path)
> >  {
> > char lxcpath[MAXPATHLEN];
> > +   char *rootfsdir = NULL;
> > char *upperdir = NULL;
> > char *workdir = NULL;
> > char **opts = NULL;
> > @@ -1832,13 +1871,13 @@ static int mount_entry_create_overlay_dirs(const 
> > struct mntent *mntent,
> > size_t rootfslen = 0;
> >  
> > if (!rootfs->path || !lxc_name || !lxc_path)
> > -   return -1;
> > +   goto err;
> >  
> > opts = lxc_string_split(mntent->mnt_opts, ',');
> > if (opts)
> > arrlen = lxc_array_len((void **)opts);
> > else
> > -   return -1;
> > +   goto err;
> >  
> > for (i = 0; i < arrlen; i++) {
> > if (strstr(opts[i], "upperdir=") && (strlen(opts[i]) > (len = 
> > strlen("upperdir="
> > @@ -1848,31 +1887,37 @@ static int mount_entry_create_overlay_dirs(const 
> > struct mntent *mntent,
> > }
> >  
> > ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
> > -   if (ret < 0 || ret >= MAXPATHLEN) {
> > -   lxc_free_array((void **)opts, free);
> > -   return -1;
> > -   }
> > +   if (ret < 0 || ret >= MAXPATHLEN)
> > +   goto err;
> > +
> > +   rootfsdir = ovl_get_rootfs_dir(rootfs->path, );
> > +   if (!rootfsdir)
> > +   goto err;
> >  
> > dirlen = strlen(lxcpath);
> > -   rootfslen = strlen(rootfs->path);
> >  
> > /* We neither allow users to create upperdirs and workdirs outside the
> >  * containerdir nor inside the rootfs. The latter might be debatable. */
> > if (upperdir)
> > -   if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> > (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> > +   if ((strncmp(upperdir,

Re: [lxc-devel] [PATCH v4] Parse rootfs->path

2015-10-22 Thread Christian Brauner

On Thu, Oct 22, 2015 at 02:59:28AM +, Serge Hallyn wrote:
> Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > On Tue, Oct 20, 2015 at 09:21:48PM +0200, Christian Brauner wrote:
> > > The mount_entry_overlay_dirs() and mount_entry_aufs_dirs() functions 
> > > create
> > > workdirs and upperdirs for overlay and aufs lxc.mount.entry entries. They 
> > > try
> > > to make sure that the workdirs and upperdirs can only be created under the
> > > containerdir (e.g. /path/to/the/container/CONTAINERNAME). In order to do 
> > > this
> > > the right hand side of
> > > 
> > > if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> > > (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> > > 
> > > was thought to check if the rootfs->path is not present in the workdir and
> > > upperdir mount options. But the current check is bogus since it will be
> > > trivially true whenever the container is a block-dev or overlay or aufs 
> > > backed
> > > since the rootfs->path will then have a form like e.g.
> > > 
> > > overlayfs:/some/path:/some/other/path
> > > 
> > > This patch adds the function ovl_get_rootfs_dir() which parses 
> > > rootfs->path by
> > > searching backwards for the first occurrence of the delimiter pair ":/". 
> > > We do
> > > not simply search for ":" since it might be used in path names. If ":/" 
> > > is not
> > > found we assume the container is directory backed and simply return
> > > strdup(rootfs->path).
> > > 
> > > Signed-off-by: Christian Brauner 
> > > ---
> > >  src/lxc/conf.c | 101 
> > > +++--
> > >  1 file changed, 76 insertions(+), 25 deletions(-)
> > > 
> > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > > index 16a62f8..fc072f7 100644
> > > --- a/src/lxc/conf.c
> > > +++ b/src/lxc/conf.c
> > > @@ -1815,12 +1815,51 @@ static void cull_mntent_opt(struct mntent *mntent)
> > >   }
> > >  }
> > >  
> > > +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t 
> > > *rootfslen)
> > > +{
> > > + char *rootfsdir = NULL;
> > > + char *s1 = NULL;
> > > + char *s2 = NULL;
> > > + char *s3 = NULL;
> > > + size_t s3len = 0;
> > > +
> > > + if (!rootfs_path || !rootfslen)
> > > + return NULL;
> > > +
> > > + s1 = strdup(rootfs_path);
> > > + if (!s1)
> > > + return NULL;
> > > +
> > > + if ((s2 = strstr(s1, ":/"))) {
> > > + s2 = s2 + 1;
> > > + if ((s3 = strstr(s2, ":/"))) {
> > > + s3len = strlen(s3);
> > > + s2[strlen(s2) - s3len] = '\0';
> > > + }
> > > + rootfsdir = strdup(s2);
> > > + if (!rootfsdir) {
> > > + free(s1);
> > > + return NULL;
> > > + }
> > > + }
> > > +
> > > + if (!rootfsdir)
> > > + rootfsdir = s1;
> > > + else
> > > + free(s1);
> > > +
> > > + *rootfslen = strlen(rootfsdir);
> > > +
> > > + return rootfsdir;
> > > +}
> > > +
> > >  static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
> > >  const struct lxc_rootfs *rootfs,
> > >  const char *lxc_name,
> > >  const char *lxc_path)
> > >  {
> > >   char lxcpath[MAXPATHLEN];
> > > + char *rootfsdir = NULL;
> > >   char *upperdir = NULL;
> > >   char *workdir = NULL;
> > >   char **opts = NULL;
> > > @@ -1832,13 +1871,13 @@ static int mount_entry_create_overlay_dirs(const 
> > > struct mntent *mntent,
> > >   size_t rootfslen = 0;
> > >  
> > >   if (!rootfs->path || !lxc_name || !lxc_path)
> > > - return -1;
> > > + goto err;
> > >  
> > >   opts = lxc_string_split(mntent->mnt_opts, ',');
> > >   if (opts)
> > >   arrlen = lxc_array_len((void **)opts);
> > >   else
> > > - return -1;
> > > + goto err;
> > >  
> > >   for (i = 0; i < arrlen; i++) {
> > >   if (strstr(opts[i], "upperdir=") && (strlen(opts[i]) > (len = 
> > > strlen("upperdir="
> > > @@ -1848,31 +1887,37 @@ static int mount_entry_create_overlay_dirs(const 
> > > struct mntent *mntent,
> > >   }
> > >  
> > >   ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
> > > - if (ret < 0 || ret >= MAXPATHLEN) {
> > > - lxc_free_array((void **)opts, free);
> > > - return -1;
> > > - }
> > > + if (ret < 0 || ret >= MAXPATHLEN)
> > > + goto err;
> > > +
> > > + rootfsdir = ovl_get_rootfs_dir(rootfs->path, );
> > > + if (!rootfsdir)
> > > + goto err;
> > >  
> > >   dirlen = strlen(lxcpath);
> > > - rootfslen = strlen(rootfs->path);
> > >  
> > >   /* We neither allow users to create upperdirs and workdirs outside the
> > >* containerdir nor inside the rootfs. The latter might be debatable. */
> > >   if (upperdir)
> > > - if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> > > (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> > > + if ((strncmp(upperdir,

Re: [lxc-devel] [PATCH v4] Parse rootfs->path

2015-10-21 Thread Serge Hallyn

Quoting Christian Brauner (christianvanbrau...@gmail.com):
> On Tue, Oct 20, 2015 at 09:21:48PM +0200, Christian Brauner wrote:
> > The mount_entry_overlay_dirs() and mount_entry_aufs_dirs() functions create
> > workdirs and upperdirs for overlay and aufs lxc.mount.entry entries. They 
> > try
> > to make sure that the workdirs and upperdirs can only be created under the
> > containerdir (e.g. /path/to/the/container/CONTAINERNAME). In order to do 
> > this
> > the right hand side of
> > 
> > if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> > (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> > 
> > was thought to check if the rootfs->path is not present in the workdir and
> > upperdir mount options. But the current check is bogus since it will be
> > trivially true whenever the container is a block-dev or overlay or aufs 
> > backed
> > since the rootfs->path will then have a form like e.g.
> > 
> > overlayfs:/some/path:/some/other/path
> > 
> > This patch adds the function ovl_get_rootfs_dir() which parses rootfs->path 
> > by
> > searching backwards for the first occurrence of the delimiter pair ":/". We 
> > do
> > not simply search for ":" since it might be used in path names. If ":/" is 
> > not
> > found we assume the container is directory backed and simply return
> > strdup(rootfs->path).
> > 
> > Signed-off-by: Christian Brauner 
> > ---
> >  src/lxc/conf.c | 101 
> > +++--
> >  1 file changed, 76 insertions(+), 25 deletions(-)
> > 
> > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > index 16a62f8..fc072f7 100644
> > --- a/src/lxc/conf.c
> > +++ b/src/lxc/conf.c
> > @@ -1815,12 +1815,51 @@ static void cull_mntent_opt(struct mntent *mntent)
> > }
> >  }
> >  
> > +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t *rootfslen)
> > +{
> > +   char *rootfsdir = NULL;
> > +   char *s1 = NULL;
> > +   char *s2 = NULL;
> > +   char *s3 = NULL;
> > +   size_t s3len = 0;
> > +
> > +   if (!rootfs_path || !rootfslen)
> > +   return NULL;
> > +
> > +   s1 = strdup(rootfs_path);
> > +   if (!s1)
> > +   return NULL;
> > +
> > +   if ((s2 = strstr(s1, ":/"))) {
> > +   s2 = s2 + 1;
> > +   if ((s3 = strstr(s2, ":/"))) {
> > +   s3len = strlen(s3);
> > +   s2[strlen(s2) - s3len] = '\0';
> > +   }
> > +   rootfsdir = strdup(s2);
> > +   if (!rootfsdir) {
> > +   free(s1);
> > +   return NULL;
> > +   }
> > +   }
> > +
> > +   if (!rootfsdir)
> > +   rootfsdir = s1;
> > +   else
> > +   free(s1);
> > +
> > +   *rootfslen = strlen(rootfsdir);
> > +
> > +   return rootfsdir;
> > +}
> > +
> >  static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
> >const struct lxc_rootfs *rootfs,
> >const char *lxc_name,
> >const char *lxc_path)
> >  {
> > char lxcpath[MAXPATHLEN];
> > +   char *rootfsdir = NULL;
> > char *upperdir = NULL;
> > char *workdir = NULL;
> > char **opts = NULL;
> > @@ -1832,13 +1871,13 @@ static int mount_entry_create_overlay_dirs(const 
> > struct mntent *mntent,
> > size_t rootfslen = 0;
> >  
> > if (!rootfs->path || !lxc_name || !lxc_path)
> > -   return -1;
> > +   goto err;
> >  
> > opts = lxc_string_split(mntent->mnt_opts, ',');
> > if (opts)
> > arrlen = lxc_array_len((void **)opts);
> > else
> > -   return -1;
> > +   goto err;
> >  
> > for (i = 0; i < arrlen; i++) {
> > if (strstr(opts[i], "upperdir=") && (strlen(opts[i]) > (len = 
> > strlen("upperdir="
> > @@ -1848,31 +1887,37 @@ static int mount_entry_create_overlay_dirs(const 
> > struct mntent *mntent,
> > }
> >  
> > ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
> > -   if (ret < 0 || ret >= MAXPATHLEN) {
> > -   lxc_free_array((void **)opts, free);
> > -   return -1;
> > -   }
> > +   if (ret < 0 || ret >= MAXPATHLEN)
> > +   goto err;
> > +
> > +   rootfsdir = ovl_get_rootfs_dir(rootfs->path, );
> > +   if (!rootfsdir)
> > +   goto err;
> >  
> > dirlen = strlen(lxcpath);
> > -   rootfslen = strlen(rootfs->path);
> >  
> > /* We neither allow users to create upperdirs and workdirs outside the
> >  * containerdir nor inside the rootfs. The latter might be debatable. */
> > if (upperdir)
> > -   if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> > (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> > +   if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> > (strncmp(upperdir, rootfsdir, rootfslen) != 0))
> > if (mkdir_p(upperdir, 0755) < 0) {
> > WARN("Failed to create upperdir");
> >

Re: [lxc-devel] [PATCH v4] Parse rootfs->path

2015-10-21 Thread Serge Hallyn

Quoting Christian Brauner (christianvanbrau...@gmail.com):
> The mount_entry_overlay_dirs() and mount_entry_aufs_dirs() functions create
> workdirs and upperdirs for overlay and aufs lxc.mount.entry entries. They try
> to make sure that the workdirs and upperdirs can only be created under the
> containerdir (e.g. /path/to/the/container/CONTAINERNAME). In order to do this
> the right hand side of
> 
> if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> 
> was thought to check if the rootfs->path is not present in the workdir and
> upperdir mount options. But the current check is bogus since it will be
> trivially true whenever the container is a block-dev or overlay or aufs backed
> since the rootfs->path will then have a form like e.g.
> 
> overlayfs:/some/path:/some/other/path
> 
> This patch adds the function ovl_get_rootfs_dir() which parses rootfs->path by
> searching backwards for the first occurrence of the delimiter pair ":/". We do
> not simply search for ":" since it might be used in path names. If ":/" is not
> found we assume the container is directory backed and simply return
> strdup(rootfs->path).
> 
> Signed-off-by: Christian Brauner 
> ---
>  src/lxc/conf.c | 101 
> +++--
>  1 file changed, 76 insertions(+), 25 deletions(-)
> 
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index 16a62f8..fc072f7 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -1815,12 +1815,51 @@ static void cull_mntent_opt(struct mntent *mntent)
>   }
>  }
>  
> +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t *rootfslen)
> +{
> + char *rootfsdir = NULL;
> + char *s1 = NULL;
> + char *s2 = NULL;
> + char *s3 = NULL;
> + size_t s3len = 0;
> +
> + if (!rootfs_path || !rootfslen)
> + return NULL;
> +
> + s1 = strdup(rootfs_path);
> + if (!s1)
> + return NULL;
> +
> + if ((s2 = strstr(s1, ":/"))) {
> + s2 = s2 + 1;
> + if ((s3 = strstr(s2, ":/"))) {
> + s3len = strlen(s3);
> + s2[strlen(s2) - s3len] = '\0';

patch looks good, just one q - is there any reaso nnot to
just do

*s3 = '\0';

here?

> + }
> + rootfsdir = strdup(s2);
> + if (!rootfsdir) {
> + free(s1);
> + return NULL;
> + }
> + }
> +
> + if (!rootfsdir)
> + rootfsdir = s1;
> + else
> + free(s1);
> +
> + *rootfslen = strlen(rootfsdir);
> +
> + return rootfsdir;
> +}
> +
>  static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
>  const struct lxc_rootfs *rootfs,
>  const char *lxc_name,
>  const char *lxc_path)
>  {
>   char lxcpath[MAXPATHLEN];
> + char *rootfsdir = NULL;
>   char *upperdir = NULL;
>   char *workdir = NULL;
>   char **opts = NULL;
> @@ -1832,13 +1871,13 @@ static int mount_entry_create_overlay_dirs(const 
> struct mntent *mntent,
>   size_t rootfslen = 0;
>  
>   if (!rootfs->path || !lxc_name || !lxc_path)
> - return -1;
> + goto err;
>  
>   opts = lxc_string_split(mntent->mnt_opts, ',');
>   if (opts)
>   arrlen = lxc_array_len((void **)opts);
>   else
> - return -1;
> + goto err;
>  
>   for (i = 0; i < arrlen; i++) {
>   if (strstr(opts[i], "upperdir=") && (strlen(opts[i]) > (len = 
> strlen("upperdir="
> @@ -1848,31 +1887,37 @@ static int mount_entry_create_overlay_dirs(const 
> struct mntent *mntent,
>   }
>  
>   ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
> - if (ret < 0 || ret >= MAXPATHLEN) {
> - lxc_free_array((void **)opts, free);
> - return -1;
> - }
> + if (ret < 0 || ret >= MAXPATHLEN)
> + goto err;
> +
> + rootfsdir = ovl_get_rootfs_dir(rootfs->path, );
> + if (!rootfsdir)
> + goto err;
>  
>   dirlen = strlen(lxcpath);
> - rootfslen = strlen(rootfs->path);
>  
>   /* We neither allow users to create upperdirs and workdirs outside the
>* containerdir nor inside the rootfs. The latter might be debatable. */
>   if (upperdir)
> - if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> + if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> (strncmp(upperdir, rootfsdir, rootfslen) != 0))
>   if (mkdir_p(upperdir, 0755) < 0) {
>   WARN("Failed to create upperdir");
>   }
>  
> -
>   if (workdir)
> - if ((strncmp(workdir, lxcpath, dirlen) == 0) && 
>

[lxc-devel] [PATCH v4] Parse rootfs->path

2015-10-20 Thread Christian Brauner

The mount_entry_overlay_dirs() and mount_entry_aufs_dirs() functions create
workdirs and upperdirs for overlay and aufs lxc.mount.entry entries. They try
to make sure that the workdirs and upperdirs can only be created under the
containerdir (e.g. /path/to/the/container/CONTAINERNAME). In order to do this
the right hand side of

if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
(strncmp(upperdir, rootfs->path, rootfslen) != 0))

was thought to check if the rootfs->path is not present in the workdir and
upperdir mount options. But the current check is bogus since it will be
trivially true whenever the container is a block-dev or overlay or aufs backed
since the rootfs->path will then have a form like e.g.

overlayfs:/some/path:/some/other/path

This patch adds the function ovl_get_rootfs_dir() which parses rootfs->path by
searching backwards for the first occurrence of the delimiter pair ":/". We do
not simply search for ":" since it might be used in path names. If ":/" is not
found we assume the container is directory backed and simply return
strdup(rootfs->path).

Signed-off-by: Christian Brauner 
---
 src/lxc/conf.c | 101 +++--
 1 file changed, 76 insertions(+), 25 deletions(-)

diff --git a/src/lxc/conf.c b/src/lxc/conf.c
index 16a62f8..fc072f7 100644
--- a/src/lxc/conf.c
+++ b/src/lxc/conf.c
@@ -1815,12 +1815,51 @@ static void cull_mntent_opt(struct mntent *mntent)
}
 }
 
+static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t *rootfslen)
+{
+   char *rootfsdir = NULL;
+   char *s1 = NULL;
+   char *s2 = NULL;
+   char *s3 = NULL;
+   size_t s3len = 0;
+
+   if (!rootfs_path || !rootfslen)
+   return NULL;
+
+   s1 = strdup(rootfs_path);
+   if (!s1)
+   return NULL;
+
+   if ((s2 = strstr(s1, ":/"))) {
+   s2 = s2 + 1;
+   if ((s3 = strstr(s2, ":/"))) {
+   s3len = strlen(s3);
+   s2[strlen(s2) - s3len] = '\0';
+   }
+   rootfsdir = strdup(s2);
+   if (!rootfsdir) {
+   free(s1);
+   return NULL;
+   }
+   }
+
+   if (!rootfsdir)
+   rootfsdir = s1;
+   else
+   free(s1);
+
+   *rootfslen = strlen(rootfsdir);
+
+   return rootfsdir;
+}
+
 static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
   const struct lxc_rootfs *rootfs,
   const char *lxc_name,
   const char *lxc_path)
 {
char lxcpath[MAXPATHLEN];
+   char *rootfsdir = NULL;
char *upperdir = NULL;
char *workdir = NULL;
char **opts = NULL;
@@ -1832,13 +1871,13 @@ static int mount_entry_create_overlay_dirs(const struct 
mntent *mntent,
size_t rootfslen = 0;
 
if (!rootfs->path || !lxc_name || !lxc_path)
-   return -1;
+   goto err;
 
opts = lxc_string_split(mntent->mnt_opts, ',');
if (opts)
arrlen = lxc_array_len((void **)opts);
else
-   return -1;
+   goto err;
 
for (i = 0; i < arrlen; i++) {
if (strstr(opts[i], "upperdir=") && (strlen(opts[i]) > (len = 
strlen("upperdir="
@@ -1848,31 +1887,37 @@ static int mount_entry_create_overlay_dirs(const struct 
mntent *mntent,
}
 
ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
-   if (ret < 0 || ret >= MAXPATHLEN) {
-   lxc_free_array((void **)opts, free);
-   return -1;
-   }
+   if (ret < 0 || ret >= MAXPATHLEN)
+   goto err;
+
+   rootfsdir = ovl_get_rootfs_dir(rootfs->path, );
+   if (!rootfsdir)
+   goto err;
 
dirlen = strlen(lxcpath);
-   rootfslen = strlen(rootfs->path);
 
/* We neither allow users to create upperdirs and workdirs outside the
 * containerdir nor inside the rootfs. The latter might be debatable. */
if (upperdir)
-   if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
(strncmp(upperdir, rootfs->path, rootfslen) != 0))
+   if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
(strncmp(upperdir, rootfsdir, rootfslen) != 0))
if (mkdir_p(upperdir, 0755) < 0) {
WARN("Failed to create upperdir");
}
 
-
if (workdir)
-   if ((strncmp(workdir, lxcpath, dirlen) == 0) && 
(strncmp(workdir, rootfs->path, rootfslen) != 0))
+   if ((strncmp(workdir, lxcpath, dirlen) == 0) && 
(strncmp(workdir, rootfsdir, rootfslen) != 0))
if (mkdir_p(workdir, 0755) < 0) {
WARN("Failed to create workdir");

[lxc-devel] [PATCH v4] Parse rootfs->path

2015-10-20 Thread Christian Brauner

A fresh set of eyes would be nice. :)

New solution:
Use strstr() to search for ":/".
If we find ":/" move the string to "/".
Check for a second ":/".
If we find it replace ':' with '\0'.

If we do not find ":/" return strdup(rootfs->path).

Christian Brauner (1):
  Parse rootfs->path

 src/lxc/conf.c | 101 +++--
 1 file changed, 76 insertions(+), 25 deletions(-)

-- 
2.6.1

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

Re: [lxc-devel] [PATCH v4] Parse rootfs->path

2015-10-20 Thread Christian Brauner

On Tue, Oct 20, 2015 at 09:21:48PM +0200, Christian Brauner wrote:
> The mount_entry_overlay_dirs() and mount_entry_aufs_dirs() functions create
> workdirs and upperdirs for overlay and aufs lxc.mount.entry entries. They try
> to make sure that the workdirs and upperdirs can only be created under the
> containerdir (e.g. /path/to/the/container/CONTAINERNAME). In order to do this
> the right hand side of
> 
> if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> 
> was thought to check if the rootfs->path is not present in the workdir and
> upperdir mount options. But the current check is bogus since it will be
> trivially true whenever the container is a block-dev or overlay or aufs backed
> since the rootfs->path will then have a form like e.g.
> 
> overlayfs:/some/path:/some/other/path
> 
> This patch adds the function ovl_get_rootfs_dir() which parses rootfs->path by
> searching backwards for the first occurrence of the delimiter pair ":/". We do
> not simply search for ":" since it might be used in path names. If ":/" is not
> found we assume the container is directory backed and simply return
> strdup(rootfs->path).
> 
> Signed-off-by: Christian Brauner 
> ---
>  src/lxc/conf.c | 101 
> +++--
>  1 file changed, 76 insertions(+), 25 deletions(-)
> 
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index 16a62f8..fc072f7 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -1815,12 +1815,51 @@ static void cull_mntent_opt(struct mntent *mntent)
>   }
>  }
>  
> +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t *rootfslen)
> +{
> + char *rootfsdir = NULL;
> + char *s1 = NULL;
> + char *s2 = NULL;
> + char *s3 = NULL;
> + size_t s3len = 0;
> +
> + if (!rootfs_path || !rootfslen)
> + return NULL;
> +
> + s1 = strdup(rootfs_path);
> + if (!s1)
> + return NULL;
> +
> + if ((s2 = strstr(s1, ":/"))) {
> + s2 = s2 + 1;
> + if ((s3 = strstr(s2, ":/"))) {
> + s3len = strlen(s3);
> + s2[strlen(s2) - s3len] = '\0';
> + }
> + rootfsdir = strdup(s2);
> + if (!rootfsdir) {
> + free(s1);
> + return NULL;
> + }
> + }
> +
> + if (!rootfsdir)
> + rootfsdir = s1;
> + else
> + free(s1);
> +
> + *rootfslen = strlen(rootfsdir);
> +
> + return rootfsdir;
> +}
> +
>  static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
>  const struct lxc_rootfs *rootfs,
>  const char *lxc_name,
>  const char *lxc_path)
>  {
>   char lxcpath[MAXPATHLEN];
> + char *rootfsdir = NULL;
>   char *upperdir = NULL;
>   char *workdir = NULL;
>   char **opts = NULL;
> @@ -1832,13 +1871,13 @@ static int mount_entry_create_overlay_dirs(const 
> struct mntent *mntent,
>   size_t rootfslen = 0;
>  
>   if (!rootfs->path || !lxc_name || !lxc_path)
> - return -1;
> + goto err;
>  
>   opts = lxc_string_split(mntent->mnt_opts, ',');
>   if (opts)
>   arrlen = lxc_array_len((void **)opts);
>   else
> - return -1;
> + goto err;
>  
>   for (i = 0; i < arrlen; i++) {
>   if (strstr(opts[i], "upperdir=") && (strlen(opts[i]) > (len = 
> strlen("upperdir="
> @@ -1848,31 +1887,37 @@ static int mount_entry_create_overlay_dirs(const 
> struct mntent *mntent,
>   }
>  
>   ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
> - if (ret < 0 || ret >= MAXPATHLEN) {
> - lxc_free_array((void **)opts, free);
> - return -1;
> - }
> + if (ret < 0 || ret >= MAXPATHLEN)
> + goto err;
> +
> + rootfsdir = ovl_get_rootfs_dir(rootfs->path, );
> + if (!rootfsdir)
> + goto err;
>  
>   dirlen = strlen(lxcpath);
> - rootfslen = strlen(rootfs->path);
>  
>   /* We neither allow users to create upperdirs and workdirs outside the
>* containerdir nor inside the rootfs. The latter might be debatable. */
>   if (upperdir)
> - if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> + if ((strncmp(upperdir, lxcpath, dirlen) == 0) && 
> (strncmp(upperdir, rootfsdir, rootfslen) != 0))
>   if (mkdir_p(upperdir, 0755) < 0) {
>   WARN("Failed to create upperdir");
>   }

We should also consider putting

return -1;

here when the check fails. Consider a container named AA which has an
lxc.mount.entry for an overlay mount:

lxc.mount.entry = /lowerdir

Re: [lxc-devel] [PATCH v4] Parse rootfs->path

Re: [lxc-devel] [PATCH v4] Parse rootfs->path

Re: [lxc-devel] [PATCH v4] Parse rootfs->path

Re: [lxc-devel] [PATCH v4] Parse rootfs->path

[lxc-devel] [PATCH v4] Parse rootfs->path

[lxc-devel] [PATCH v4] Parse rootfs->path

Re: [lxc-devel] [PATCH v4] Parse rootfs->path

7 matches

Site Navigation

Mail list logo

Footer information