[lxc-devel] [PATCH v4] Parse rootfs->path

[Christian Brauner]

On Tue, Oct 20, 2015 at 09:21:48PM +0200, Christian Brauner wrote:
> The mount_entry_overlay_dirs() and mount_entry_aufs_dirs() functions create
> workdirs and upperdirs for overlay and aufs lxc.mount.entry entries. They try
> to make sure that the workdirs and upperdirs can only be created under the
> containerdir (e.g. /path/to/the/container/CONTAINERNAME). In order to do this
> the right hand side of
> 
>                 if ((strncmp(upperdir, lxcpath, dirlen) == 0) && (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> 
> was thought to check if the rootfs->path is not present in the workdir and
> upperdir mount options. But the current check is bogus since it will be
> trivially true whenever the container is a block-dev or overlay or aufs backed
> since the rootfs->path will then have a form like e.g.
> 
>         overlayfs:/some/path:/some/other/path
> 
> This patch adds the function ovl_get_rootfs_dir() which parses rootfs->path by
> searching backwards for the first occurrence of the delimiter pair ":/". We do
> not simply search for ":" since it might be used in path names. If ":/" is not
> found we assume the container is directory backed and simply return
> strdup(rootfs->path).
> 
> Signed-off-by: Christian Brauner <christianvanbrauner at gmail.com>
> ---
>  src/lxc/conf.c | 101 +++++++++++++++++++++++++++++++++++++++++++--------------
>  1 file changed, 76 insertions(+), 25 deletions(-)
> 
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index 16a62f8..fc072f7 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -1815,12 +1815,51 @@ static void cull_mntent_opt(struct mntent *mntent)
>  	}
>  }
>  
> +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t *rootfslen)
> +{
> +	char *rootfsdir = NULL;
> +	char *s1 = NULL;
> +	char *s2 = NULL;
> +	char *s3 = NULL;
> +	size_t s3len = 0;
> +
> +	if (!rootfs_path || !rootfslen)
> +		return NULL;
> +
> +	s1 = strdup(rootfs_path);
> +	if (!s1)
> +		return NULL;
> +
> +	if ((s2 = strstr(s1, ":/"))) {
> +		s2 = s2 + 1;
> +		if ((s3 = strstr(s2, ":/"))) {
> +			s3len = strlen(s3);
> +			s2[strlen(s2) - s3len] = '\0';
> +		}
> +		rootfsdir = strdup(s2);
> +		if (!rootfsdir) {
> +			free(s1);
> +			return NULL;
> +		}
> +	}
> +
> +	if (!rootfsdir)
> +		rootfsdir = s1;
> +	else
> +		free(s1);
> +
> +	*rootfslen = strlen(rootfsdir);
> +
> +	return rootfsdir;
> +}
> +
>  static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
>  					   const struct lxc_rootfs *rootfs,
>  					   const char *lxc_name,
>  					   const char *lxc_path)
>  {
>  	char lxcpath[MAXPATHLEN];
> +	char *rootfsdir = NULL;
>  	char *upperdir = NULL;
>  	char *workdir = NULL;
>  	char **opts = NULL;
> @@ -1832,13 +1871,13 @@ static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
>  	size_t rootfslen = 0;
>  
>  	if (!rootfs->path || !lxc_name || !lxc_path)
> -		return -1;
> +		goto err;
>  
>  	opts = lxc_string_split(mntent->mnt_opts, ',');
>  	if (opts)
>  		arrlen = lxc_array_len((void **)opts);
>  	else
> -		return -1;
> +		goto err;
>  
>  	for (i = 0; i < arrlen; i++) {
>  		if (strstr(opts[i], "upperdir=") && (strlen(opts[i]) > (len = strlen("upperdir="))))
> @@ -1848,31 +1887,37 @@ static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
>  	}
>  
>  	ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
> -	if (ret < 0 || ret >= MAXPATHLEN) {
> -		lxc_free_array((void **)opts, free);
> -		return -1;
> -	}
> +	if (ret < 0 || ret >= MAXPATHLEN)
> +		goto err;
> +
> +	rootfsdir = ovl_get_rootfs_dir(rootfs->path, &rootfslen);
> +	if (!rootfsdir)
> +		goto err;
>  
>  	dirlen = strlen(lxcpath);
> -	rootfslen = strlen(rootfs->path);
>  
>  	/* We neither allow users to create upperdirs and workdirs outside the
>  	 * containerdir nor inside the rootfs. The latter might be debatable. */
>  	if (upperdir)
> -		if ((strncmp(upperdir, lxcpath, dirlen) == 0) && (strncmp(upperdir, rootfs->path, rootfslen) != 0))
> +		if ((strncmp(upperdir, lxcpath, dirlen) == 0) && (strncmp(upperdir, rootfsdir, rootfslen) != 0))
>  			if (mkdir_p(upperdir, 0755) < 0) {
>  				WARN("Failed to create upperdir");
>  			}

We should also consider putting

        return -1;

here when the check fails. Consider a container named AA which has an
lxc.mount.entry for an overlay mount:

        lxc.mount.entry = /lowerdir dest overlay lowerdir=/lowerdir,upperdir=/path/to/BB/delta,workdir=/path/to/BB/workdir,create=dir

whose upperdir and workdir refer to another container BB. Both, upperdir and
workdir already exist under BB's containerdir. If we do not return -1 when the
check fails the mount will succeed and we will write changes to workdir/upperdir
of BB. That should not happen. Once you let me know that you're fine with the
reimplementation of ovl_get_rootfs_dir() I would like to change that too before
you ack it.

Christian
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://lists.linuxcontainers.org/pipermail/lxc-devel/attachments/20151021/b3c45d13/attachment-0001.sig>