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

[Christian Brauner]

On Oct 16, 2015 19:11, "Serge Hallyn" <serge.hallyn at ubuntu.com> wrote:
>
> Quoting Christian Brauner (christianvanbrauner at gmail.com):
> > On Fri, Oct 16, 2015 at 03:52:04PM +0000, Serge Hallyn wrote:
> > > 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
> >
> > It's not bogus. It just misses the single case where the container
itself is an
> > overlay container. :)
>
> Or blockd-dev-backed, or any case where lxc.rootfs is in a nonstandard
> location - but that's ok,
>
Right, I always forget the block-devs...

> > Let's merge this patch as it is.
>
> Yeah, that's fine.
>
> @Stgraber - please do apply this :)
>
> > I will test whether rootfs->mount really is what we want and send a
follow-up
> > patch that is based off of this one. (I'll probably send it tomorrow.)
>
> Great, thanks.
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20151016/978c8e8d/attachment.html>