On Mon, Nov 02, 2015 at 03:12:02PM +0000, 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_ovl_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_ovl_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 <christianvanbrau...@gmail.com>
> > ---
> > src/lxc/lxccontainer.c | 98
> > ++++++++++++++++++++++++++++++++++++++++++++++++++
> > 1 file changed, 98 insertions(+)
> >
> > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> > index 42e23e7..7150103 100644
> > --- a/src/lxc/lxccontainer.c
> > +++ b/src/lxc/lxccontainer.c
> > @@ -2894,6 +2894,100 @@ static int create_file_dirname(char *path, struct
> > lxc_conf *conf)
> > return ret;
> > }
> >
> > +/* When we clone a container with overlay lxc.mount.entry entries we need
> > to
> > +* update absolute paths for upper- and workdir. This update is done in two
> > +* locations: lxc_conf->unexpanded_config and lxc_conf->mount_list. Both
> > updates
> > +* are done independent of each other since lxc_conf->mountlist may
> > container
> > +* more mount entries (e.g. from other included files) than
> > +* lxc_conf->unexpanded_config . */
> > +static int update_ovl_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);
> > +
> > + /* We have to update lxc_conf->unexpanded_config separately from
> > + * lxc_conf->mount_list. */
> > + for (i = 0; i < sizeof(ovl_dirs) / sizeof(ovl_dirs[0]); i++) {
> > + if (!clone_update_unexp_ovl_paths(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, &lxc_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;
> > +
>
> The rest of this block needs to be un-indented by one level.
>
> > + ret = snprintf(old_upper, MAXPATHLEN, "%s=%s/%s", tmp,
> > lxc_path, lxc_name);
> > + if (ret < 0 || ret >= MAXPATHLEN)
> > + goto err;
> > +
> > + ret = snprintf(new_upper, MAXPATHLEN, "%s=%s/%s", tmp,
> > cleanpath, newname);
> > + if (ret < 0 || ret >= MAXPATHLEN)
> > + goto err;
> > +
> > + if (strstr(mnt_entry, old_upper)) {
> > + tmp_mnt_entry = lxc_string_replace(old_upper,
> > new_upper, mnt_entry);
> > + }
> > +
> > + if (strstr(mnt_entry, old_work)) {
> > + if (tmp_mnt_entry)
> > + new_mnt_entry =
> > lxc_string_replace(old_work, new_work, tmp_mnt_entry);
>
> Don't you want old_work replaced even if old_upper wasn't found?
>
> (or are we requiring limitations on the old upper that guarantee that
> won't happen?)
I don't think we currently are. We should:
if (strstr(mnt_entry, old_work)) {
if (tmp_mnt_entry)
new_mnt_entry = lxc_string_replace(old_work, new_work,
tmp_mnt_entry);
else (tmp_mnt_entry)
new_mnt_entry = lxc_string_replace(old_work, new_work,
mnt_entry);
}
_______________________________________________
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
