On Thu, Oct 22, 2015 at 03:33:30AM +0000, Serge Hallyn wrote: > Quoting Christian Brauner ([email protected]): > > 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 <[email protected]> > > --- > > 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'; > > patch looks good, just one q - is there any reaso nnot to > just do > > *s3 = '\0'; > > here?
Nope. (That's what staring at the screen for x hours gets you. :) Updated
version to follow.
>
> > + }
> > + 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");
> > }
> >
> > -
> > 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) {
> > WARN("Failed to create workdir");
> > }
> >
> > + free(rootfsdir);
> > lxc_free_array((void **)opts, free);
> > return 0;
> > +
> > +err:
> > + free(rootfsdir);
> > + lxc_free_array((void **)opts, free);
> > + return -1;
> > }
> >
> > static int mount_entry_create_aufs_dirs(const struct mntent *mntent,
> > @@ -1881,6 +1926,7 @@ static int mount_entry_create_aufs_dirs(const struct
> > mntent *mntent,
> > const char *lxc_path)
> > {
> > char lxcpath[MAXPATHLEN];
> > + char *rootfsdir = NULL;
> > char *scratch = NULL;
> > char *tmp = NULL;
> > char *upperdir = NULL;
> > @@ -1889,46 +1935,51 @@ static int mount_entry_create_aufs_dirs(const
> > struct mntent *mntent,
> > size_t arrlen = 0;
> > size_t i;
> > size_t len = 0;
> > + 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], "br=") && (strlen(opts[i]) > (len =
> > strlen("br="))))
> > tmp = opts[i] + len;
> > }
> > - if (!tmp) {
> > - lxc_free_array((void **)opts, free);
> > - return -1;
> > - }
> > + if (!tmp)
> > + goto err;
> >
> > upperdir = strtok_r(tmp, ":=", &scratch);
> > - if (!upperdir) {
> > - lxc_free_array((void **)opts, free);
> > - return -1;
> > - }
> > + if (!upperdir)
> > + goto err;
> >
> > 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;
> >
> > /* We neither allow users to create upperdirs outside the containerdir
> > * nor inside the rootfs. The latter might be debatable. */
> > - if ((strncmp(upperdir, lxcpath, strlen(lxcpath)) == 0) &&
> > (strncmp(upperdir, rootfs->path, strlen(rootfs->path)) != 0))
> > + if ((strncmp(upperdir, lxcpath, strlen(lxcpath)) == 0) &&
> > (strncmp(upperdir, rootfsdir, rootfslen) != 0))
> > if (mkdir_p(upperdir, 0755) < 0) {
> > WARN("Failed to create upperdir");
> > }
> >
> > + free(rootfsdir);
> > lxc_free_array((void **)opts, free);
> > return 0;
> > +
> > +err:
> > + free(rootfsdir);
> > + lxc_free_array((void **)opts, free);
> > + return -1;
> > }
> >
> >
> > --
> > 2.6.1
> >
signature.asc
Description: PGP signature
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
