[lxc-devel] [PATCH] Make mount_entry_create_*_dirs() more robust

[Serge Hallyn]

Quoting Christian Brauner (christianvanbrauner at gmail.com):
> On Thu, Oct 15, 2015 at 08:32:25PM +0000, Serge Hallyn wrote:
> > Quoting Christian Brauner (christianvanbrauner at gmail.com):
> > > The mount_entry_create_*_dirs() functions currently assume that the rootfs of
> > > the container is actually named "rootfs". This has the consequence that
> > > 
> > > 	del = strstr(lxcpath, "/rootfs");
> > > 	if (!del) {
> > > 		free(lxcpath);
> > > 		lxc_free_array((void **)opts, free);
> > > 		return -1;
> > > 	}
> > > 	*del = '\0';
> > > 
> > > will return NULL when the rootfs of a container is not actually named "rootfs".
> > > This means the we return -1 and do not create the necessary upperdir/workdir
> > > directories required for the overlay/aufs mount to work. Hence, let's not make
> > > that assumption. We now pass lxc_path and lxc_name to
> > > mount_entry_create_*_dirs() and create the path directly. To prevent failure we
> > > also have mount_entry_create_*_dirs() check that lxc_name and lxc_path are not
> > > empty when they are passed in.
> > > 
> > > Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
> > 
> > Yeah this was bugging me a few years ago.  Overall the patch looks fine
> > to me, I'm running a full testsuite to ease my mind about it.  Will ack
> > after that passes and I look over it again.
> 
> We should also consider parsing path->rootfs when the container is an overlay or
> aufs backed container. Because in this case the right hand side of the check:
> 
> 		if ((strncmp(upperdir, lxcpath, dirlen) == 0) && (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> 
> will be trivially true since path->rootfs will e.g. be overlayfs:/path1:path2.
> Parsing path->rootfs to extract path2 before doing the second check would be
> safer... Thoughts?
> 

True that the current check is bogus.  But I think you just want to
use rootfs->mount instead of rootfs->path.  By the time this code
hits we have converted whatever target path the user gave us into
concat(rootfs->mount, process(target)) where process(x) will take
off a leading $lxcpath/$lxcname/rootfs or rootfs->path.

-serge