[lxc-devel] [PATCH v2] Update absolute paths for overlay and aufs mounts
Serge Hallyn
serge.hallyn at ubuntu.com
Tue Oct 20 16:17:18 UTC 2015
Quoting Christian Brauner (christianvanbrauner at 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_union_mount_entry_paths() in
> lxccontainer.c. The function updates the mounts in two locations:
>
> 1) lxc_conf->mount_list
>
> and
>
> 2) lxc_conf->unexpanded_config
>
> 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,
> that an entry in lxc_conf->mount_list will only be updated if it is also found
> in the clones config. Mounts from other files are hence not updated. In short,
> automatic overlay and aufs mounts with lxc.mount.entry should only be specified
> in the containers own config.
>
> 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_union_mount_entry_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 <christianvanbrauner at gmail.com>
> ---
> src/lxc/lxccontainer.c | 133 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 132 insertions(+), 1 deletion(-)
>
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 42e23e7..5ed7697 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -2894,6 +2894,130 @@ static int create_file_dirname(char *path, struct lxc_conf *conf)
> return ret;
> }
>
> +/* 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_union_mount_entry_paths() in lxccontainer.c. The function operates on
> +* c->lxc_conf->unexpanded_config instead of the intuitively plausible
> +* c->lxc_conf->mount_list because the latter also contains mounts from other
> +* files as well as generic mounts. */
> +static int update_union_mount_entry_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;
> + char *mnt_entry = NULL;
> + char *new_mnt_entry = NULL;
> + char *new_unexpanded_config = NULL;
> + char *tmp_mnt_entry = NULL;
> + char *tmp_unexpanded_config = NULL;
> + char *tmp = NULL;
> + int ret = 0;
> + size_t len = 0;
> + struct lxc_list *iterator;
> +
> + cleanpath = strdup(newpath);
> + if (!cleanpath)
> + goto err;
> +
> + remove_trailing_slashes(cleanpath);
> +
> + 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) {
put
char *tmp = NULL, new_mnt_entry = NULL;
char *mnt_entry;
here (and remove from above etc).
> + mnt_entry = iterator->elem;
> +
> + if (strstr(mnt_entry, "overlay"))
> + tmp = "upperdir";
> + else if (strstr(mnt_entry, "aufs"))
> + tmp = "br";
> +
> + if (!tmp)
> + continue;
> +
> + if (strstr(lxc_conf->unexpanded_config, mnt_entry)) {
> + ret = snprintf(old_upper, MAXPATHLEN, "%s=%s/%s", tmp, lxc_path, lxc_name);
> + if (ret < 0 || ret >= MAXPATHLEN)
> + goto err;
Alternate suggestion. How about using clear_unexp_config_line() to remove all
lxc.mount.entry lines from unexpanded config first, then run through this loop
and simply add the new ones?
The lxc_string_replace approach feels problematic.
> + 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);
> + tmp_unexpanded_config = lxc_string_replace(old_upper, new_upper, lxc_conf->unexpanded_config);
> + }
> +
> + if (strstr(mnt_entry, old_work)) {
> + if (tmp_mnt_entry)
> + new_mnt_entry = lxc_string_replace(old_work, new_work, tmp_mnt_entry);
> + if (tmp_unexpanded_config)
> + new_unexpanded_config = lxc_string_replace(old_work, new_work, tmp_unexpanded_config);
> + }
> +
> + if (new_mnt_entry) {
> + free(iterator->elem);
> + iterator->elem = strdup(new_mnt_entry);
> + } else if (tmp_mnt_entry) {
> + free(iterator->elem);
> + iterator->elem = strdup(tmp_mnt_entry);
> + }
> +
> + if (new_unexpanded_config) {
> + free(lxc_conf->unexpanded_config);
> + lxc_conf->unexpanded_config = strdup(new_unexpanded_config);
> + } else if (tmp_unexpanded_config) {
> + free(lxc_conf->unexpanded_config);
> + lxc_conf->unexpanded_config = strdup(tmp_unexpanded_config);
> + }
> +
> + if ((new_unexpanded_config || tmp_unexpanded_config) && lxc_conf->unexpanded_config) {
> + len = strlen(lxc_conf->unexpanded_config);
> + lxc_conf->unexpanded_len = len;
> + lxc_conf->unexpanded_alloced = len + 1;
> + }
> +
> + free(new_mnt_entry);
> + free(new_unexpanded_config);
> + free(tmp_mnt_entry);
> + free(tmp_unexpanded_config);
In general, when you have to free all the same things as you do in the 'out'
label, it's better to keep a function return variable which stays -1, then
below where you do 'return 0' do 'ret = 0' instead and fall through the label.
> + mnt_entry = NULL;
> + new_mnt_entry = NULL;
> + new_unexpanded_config = NULL;
> + tmp_mnt_entry = NULL;
> + tmp_unexpanded_config = NULL;
> + tmp = NULL;
> +
> + if (!iterator->elem || !lxc_conf->unexpanded_config)
> + goto err;
> + }
> + }
> + free(cleanpath);
> + return 0;
> +
> +err:
> + free(cleanpath);
> + free(new_mnt_entry);
> + free(new_unexpanded_config);
> + free(tmp_mnt_entry);
> + free(tmp_unexpanded_config);
> + INFO("%s", "Failed to update config file");
> + return -1;
> +}
> +
> static struct lxc_container *do_lxcapi_clone(struct lxc_container *c, const char *newname,
> const char *lxcpath, int flags,
> const char *bdevtype, const char *bdevdata, uint64_t newsize,
> @@ -2953,7 +3077,9 @@ static struct lxc_container *do_lxcapi_clone(struct lxc_container *c, const char
> fclose(fout);
> c->lxc_conf->rootfs.path = origroot;
>
> - sprintf(newpath, "%s/%s/rootfs", lxcpath, newname);
> + ret = snprintf(newpath, MAXPATHLEN, "%s/%s/rootfs", lxcpath, newname);
> + if (ret < 0 || ret >= MAXPATHLEN)
> + goto out;
> if (mkdir(newpath, 0755) < 0) {
> SYSERROR("error creating %s", newpath);
> goto out;
> @@ -3009,10 +3135,15 @@ static struct lxc_container *do_lxcapi_clone(struct lxc_container *c, const char
> }
> }
>
> + // update absolute paths for union mount directories
> + if (update_union_mount_entry_paths(c2->lxc_conf, c->config_path, c->name, lxcpath, newname) < 0)
> + goto out;
> +
> // We've now successfully created c2's storage, so clear it out if we
> // fail after this
> storage_copied = 1;
>
> +
> if (!c2->save_config(c2, NULL))
> goto out;
>
> --
> 2.6.1
>
> _______________________________________________
> lxc-devel mailing list
> lxc-devel at lists.linuxcontainers.org
> http://lists.linuxcontainers.org/listinfo/lxc-devel
More information about the lxc-devel
mailing list