Re: [lxc-devel] [PATCH] Parse rootfs->path
Quoting Christian Brauner (christianvanbrau...@gmail.com): > 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 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. > > Signed-off-by: Christian Brauner> --- > src/lxc/conf.c | 126 > ++--- > 1 file changed, 103 insertions(+), 23 deletions(-) > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c > index 16a62f8..a730a67 100644 > --- a/src/lxc/conf.c > +++ b/src/lxc/conf.c > @@ -1821,6 +1821,11 @@ static int mount_entry_create_overlay_dirs(const > struct mntent *mntent, > const char *lxc_path) > { > char lxcpath[MAXPATHLEN]; > + char *end = NULL; > + char *rootfsdir = NULL; > + char *s = NULL; > + char *s1 = NULL; > + char *tmp = NULL; > char *upperdir = NULL; > char *workdir = NULL; > char **opts = NULL; > @@ -1830,15 +1835,16 @@ static int mount_entry_create_overlay_dirs(const > struct mntent *mntent, > size_t i; > size_t len = 0; > size_t rootfslen = 0; > + size_t slen = 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 +1854,66 @@ 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; > > dirlen = strlen(lxcpath); > - rootfslen = strlen(rootfs->path); > + Could you put this stuff into a helper, i.e. rootfsdir = ovl_get_rootfs_dir(rootfs->path, ); > + s = strdup(rootfs->path); > + if (!s) > + goto err; > + > + s1 = malloc(strlen(rootfs->path) + 1); > + if (!s1) > + goto err; > + end = stpcpy(s1, rootfs->path); > + > + /* If we find :/ in rootfs->path it means we either have a block-dev or > + * overlay or aufs container. */ > + while ((tmp = strrchr(s, ':'))) { > + len = strlen(tmp); > + rootfslen += len; > + if (strncmp(tmp, ":/", 2) == 0) { > + rootfsdir = end - rootfslen + 1; > + rootfslen = strlen(rootfsdir); > + break; > + } else { > + slen = strlen(s); > + s[slen - len] = '\0'; > + } > + } > + > + /* If rootfsdir is empty at this point we have a simple directory > + * container. */ > + if (!rootfsdir) { > + rootfsdir = rootfs->path; > + 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"); > } > > > if (workdir) > - if ((strncmp(workdir, lxcpath, dirlen) == 0) && > (strncmp(workdir, rootfs->path, rootfslen) != 0)) > + if ((strncmp(workdir, lxcpath, dirlen) == 0) && > (strncmp(workdir, rootfsdir, rootfslen) != 0)) > if (mkdir_p(workdir, 0755) < 0) { >
Re: [lxc-devel] [PATCH] Parse rootfs->path
On Mon, Oct 19, 2015 at 02:50:11PM +, Serge Hallyn wrote: > Quoting Christian Brauner (christianvanbrau...@gmail.com): > > 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 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. > > > > Signed-off-by: Christian Brauner> > --- > > src/lxc/conf.c | 126 > > ++--- > > 1 file changed, 103 insertions(+), 23 deletions(-) > > > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c > > index 16a62f8..a730a67 100644 > > --- a/src/lxc/conf.c > > +++ b/src/lxc/conf.c > > @@ -1821,6 +1821,11 @@ static int mount_entry_create_overlay_dirs(const > > struct mntent *mntent, > >const char *lxc_path) > > { > > char lxcpath[MAXPATHLEN]; > > + char *end = NULL; > > + char *rootfsdir = NULL; > > + char *s = NULL; > > + char *s1 = NULL; > > + char *tmp = NULL; > > char *upperdir = NULL; > > char *workdir = NULL; > > char **opts = NULL; > > @@ -1830,15 +1835,16 @@ static int mount_entry_create_overlay_dirs(const > > struct mntent *mntent, > > size_t i; > > size_t len = 0; > > size_t rootfslen = 0; > > + size_t slen = 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 +1854,66 @@ 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; > > > > dirlen = strlen(lxcpath); > > - rootfslen = strlen(rootfs->path); > > + > > Could you put this stuff into a helper, i.e. Can do. Do you mean putting it as a static helper function in conf.c, or in utils.c? > > rootfsdir = ovl_get_rootfs_dir(rootfs->path, ); > > + s = strdup(rootfs->path); > > + if (!s) > > + goto err; > > + > > + s1 = malloc(strlen(rootfs->path) + 1); > > + if (!s1) > > + goto err; > > + end = stpcpy(s1, rootfs->path); > > + > > + /* If we find :/ in rootfs->path it means we either have a block-dev or > > +* overlay or aufs container. */ > > + while ((tmp = strrchr(s, ':'))) { > > + len = strlen(tmp); > > + rootfslen += len; > > + if (strncmp(tmp, ":/", 2) == 0) { > > + rootfsdir = end - rootfslen + 1; > > + rootfslen = strlen(rootfsdir); > > + break; > > + } else { > > + slen = strlen(s); > > + s[slen - len] = '\0'; > > + } > > + } > > + > > + /* If rootfsdir is empty at this point we have a simple directory > > +* container. */ > > + if (!rootfsdir) { > > + rootfsdir = rootfs->path; > > + 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"); > > } > > > > > > if (workdir) > > - if ((strncmp(workdir, lxcpath,
Re: [lxc-devel] [PATCH] Parse rootfs->path
Quoting Christian Brauner (christianvanbrau...@gmail.com): > On Mon, Oct 19, 2015 at 02:50:11PM +, Serge Hallyn wrote: > > Quoting Christian Brauner (christianvanbrau...@gmail.com): > > > 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 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. > > > > > > Signed-off-by: Christian Brauner> > > --- > > > src/lxc/conf.c | 126 > > > ++--- > > > 1 file changed, 103 insertions(+), 23 deletions(-) > > > > > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c > > > index 16a62f8..a730a67 100644 > > > --- a/src/lxc/conf.c > > > +++ b/src/lxc/conf.c > > > @@ -1821,6 +1821,11 @@ static int mount_entry_create_overlay_dirs(const > > > struct mntent *mntent, > > > const char *lxc_path) > > > { > > > char lxcpath[MAXPATHLEN]; > > > + char *end = NULL; > > > + char *rootfsdir = NULL; > > > + char *s = NULL; > > > + char *s1 = NULL; > > > + char *tmp = NULL; > > > char *upperdir = NULL; > > > char *workdir = NULL; > > > char **opts = NULL; > > > @@ -1830,15 +1835,16 @@ static int mount_entry_create_overlay_dirs(const > > > struct mntent *mntent, > > > size_t i; > > > size_t len = 0; > > > size_t rootfslen = 0; > > > + size_t slen = 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 +1854,66 @@ 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; > > > > > > dirlen = strlen(lxcpath); > > > - rootfslen = strlen(rootfs->path); > > > + > > > > Could you put this stuff into a helper, i.e. > Can do. Do you mean putting it as a static helper function in conf.c, or in > utils.c? That I'm not sure about. It actually seems like the mount_entry_create_{overlay,aufs}_dirs functions themselves could stand to go elsewhere. But that's probably best done after we split src/lxc/bdev.c into src/lxc/bdev/file-per-backingstore So for now probably just right here. It keeps each fn simpler and easier to read. ___ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel