[lxc-devel] [ACK] [PATCH 2/2 v4] Update absolute paths for overlay and aufs mounts

[Serge Hallyn]

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_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 <christianvanbrauner at gmail.com>

Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>

> ---
>  src/lxc/lxccontainer.c | 100 +++++++++++++++++++++++++++++++++++++++++++++++++
>  1 file changed, 100 insertions(+)
> 
> diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c
> index 42e23e7..a0868eb 100644
> --- a/src/lxc/lxccontainer.c
> +++ b/src/lxc/lxccontainer.c
> @@ -2894,6 +2894,102 @@ 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;
> +
> +		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);
> +			else
> +				new_mnt_entry = lxc_string_replace(old_work, new_work, mnt_entry);
> +		}
> +
> +		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);
> +		}
> +
> +		free(new_mnt_entry);
> +		free(tmp_mnt_entry);
> +	}
> +
> +	fret = 0;
> +err:
> +	free(cleanpath);
> +	return fret;
> +}
> +
>  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,
> @@ -3009,6 +3105,10 @@ static struct lxc_container *do_lxcapi_clone(struct lxc_container *c, const char
>  		}
>  	}
>  
> +	// update absolute paths for overlay mount directories
> +	if (update_ovl_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;
> -- 
> 2.6.2
>