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 <christianvanbrau...@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
signature.asc
Description: PGP signature
_______________________________________________ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel