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

[Serge Hallyn]

Quoting Christian Brauner (christianvanbrauner at gmail.com):
> On Mon, Oct 19, 2015 at 02:50:11PM +0000, Serge Hallyn wrote:
> > Quoting Christian Brauner (christianvanbrauner at 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 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.
> > > 
> > > Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
> > > ---
> > >  src/lxc/conf.c | 126 ++++++++++++++++++++++++++++++++++++++++++++++-----------
> > >  1 file changed, 103 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > > index 16a62f8..a730a67 100644
> > > --- a/src/lxc/conf.c
> > > +++ b/src/lxc/conf.c
> > > @@ -1821,6 +1821,11 @@ static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
> > >  					   const char *lxc_path)
> > >  {
> > >  	char lxcpath[MAXPATHLEN];
> > > +	char *end = NULL;
> > > +	char *rootfsdir = NULL;
> > > +	char *s = NULL;
> > > +	char *s1 = NULL;
> > > +	char *tmp = NULL;
> > >  	char *upperdir = NULL;
> > >  	char *workdir = NULL;
> > >  	char **opts = NULL;
> > > @@ -1830,15 +1835,16 @@ static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
> > >  	size_t i;
> > >  	size_t len = 0;
> > >  	size_t rootfslen = 0;
> > > +	size_t slen = 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 +1854,66 @@ 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;
> > >  
> > >  	dirlen = strlen(lxcpath);
> > > -	rootfslen = strlen(rootfs->path);
> > > +
> > 
> > Could you put this stuff into a helper, i.e.
> Can do. Do you mean putting it as a static helper function in conf.c, or in
> utils.c?

That I'm not sure about.  It actually seems like the
mount_entry_create_{overlay,aufs}_dirs functions themselves could
stand to go elsewhere.  But that's probably best done after we
split src/lxc/bdev.c into src/lxc/bdev/file-per-backingstore

So for now probably just right here.  It keeps each fn simpler
and easier to read.