Re: [lxc-devel] [PATCH v4] Parse rootfs->path
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 > --- > 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
Re: [lxc-devel] [PATCH v2] Update absolute paths for overlay and aufs mounts
On Tue, Oct 20, 2015 at 04:17:18PM +, Serge Hallyn wrote: > Quoting Christian Brauner (christianvanbrau...@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_union_mount_entry_paths() in > > lxccontainer.c. The function updates the mounts in two locations: > > > > 1) lxc_conf->mount_list > > > > and > > > > 2) lxc_conf->unexpanded_config > > > > 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, > > that an entry in lxc_conf->mount_list will only be updated if it is also > > found > > in the clones config. Mounts from other files are hence not updated. In > > short, > > automatic overlay and aufs mounts with lxc.mount.entry should only be > > specified > > in the containers own config. > > > > 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_union_mount_entry_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 > > --- > > src/lxc/lxccontainer.c | 133 > > - > > 1 file changed, 132 insertions(+), 1 deletion(-) > > > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c > > index 42e23e7..5ed7697 100644 > > --- a/src/lxc/lxccontainer.c > > +++ b/src/lxc/lxccontainer.c > > @@ -2894,6 +2894,130 @@ static int create_file_dirname(char *path, struct > > lxc_conf *conf) > > return ret; > > } > > > > +/* 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_union_mount_entry_paths() in lxccontainer.c. The function > > operates on > > +* c->lxc_conf->unexpanded_config instead of the intuitively plausible > > +* c->lxc_conf->mount_list because the latter also contains mounts from > > other > > +* files as well as generic mounts. */ > > +static int update_union_mount_entry_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; > > + char *mnt_entry = NULL; > > + char *new_mnt_entry = NULL; > > + char *new_unexpanded_config = NULL; > > + char *tmp_mnt_entry = NULL; > > + char *tmp_unexpanded_config = NULL; > > + char *tmp = NULL; > > + int ret = 0; > > + size_t len = 0; > > + struct lxc_list *iterator; > > + > > + cleanpath = strdup(newpath); > > + if (!cleanpath) > > + goto err; > > + > > + remove_trailing_slashes(cleanpath); > > + > > + 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) { > > put > > char *tmp = NULL, new_mnt_entry = NULL; > char *mnt_entry; > > here (and remove from above etc). >
[lxc-devel] [PATCH v4] Parse rootfs->path
A fresh set of eyes would be nice. :) New solution: Use strstr() to search for ":/". If we find ":/" move the string to "/". Check for a second ":/". If we find it replace ':' with '\0'. If we do not find ":/" return strdup(rootfs->path). Christian Brauner (1): Parse rootfs->path src/lxc/conf.c | 101 +++-- 1 file changed, 76 insertions(+), 25 deletions(-) -- 2.6.1 ___ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
[lxc-devel] [PATCH v4] Parse rootfs->path
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 --- 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"); } - 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"); } +
[lxc-devel] LXC Console issue.
Hi All, I am running LXC in embedded machine which uses busybox and it make use of ttyHSL (Qualcomm UART with baud rate 115200) for serial console. I find all other tty[0-7] of the embedded machine were set to baud rate of 38400; I am trying to run LXC busybox inside this embedded machine and I set inittab file of Container C1 as show below # cat C1/rootfs/etc/inittab ::respawn:/bin/ash Problem: Running busybox commands in launched container terminal is not working as they were misbehaving # ls # /bin/ash 's' command not found # # pwd # /bin/ash 'd' command not found # # # cd # /bin/ash 'cpw' command not found I find LXC make use of tty1 terminal under C1/rootfs/dev/lxc/tty1 The 'lxc' is added in the above path because I configured config file as below for container C1 . lxc.tty = 1 lxc.pts = 1 lxc.devttydir = lxc ... I configure to make use of devttydir because the embedded box host is set to read-only for '/' file-system; What could be reason of this issue? Is this issue specific to LXC termnial? or minicon setting specific to LXC? (I was able to see lxc busybox boot without any issue) Please let me know, If you need any further information on this. Thanks -- With regards, Viswesn ___ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
Re: [lxc-devel] [PATCH v3] Parse rootfs->path
Quoting Christian Brauner (christianvanbrau...@gmail.com): > On Tue, Oct 20, 2015 at 04:31:19PM +, Serge Hallyn wrote: > > Quoting Christian Brauner (christianvanbrau...@gmail.com): > > > On Tue, Oct 20, 2015 at 03:30:19PM +, 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 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 > > > > > --- > > > > > src/lxc/conf.c | 115 > > > > > - > > > > > 1 file changed, 90 insertions(+), 25 deletions(-) > > > > > > > > > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c > > > > > index 16a62f8..301fe50 100644 > > > > > --- a/src/lxc/conf.c > > > > > +++ b/src/lxc/conf.c > > > > > @@ -1815,12 +1815,65 @@ static void cull_mntent_opt(struct mntent > > > > > *mntent) > > > > > } > > > > > } > > > > > > > > > > +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t > > > > > *rootfslen) > > > > > +{ > > > > > + char *end = NULL; > > > > > + char *s1 = NULL; > > > > > + char *s2 = NULL; > > > > > + char *rootfsdir = NULL; > > > > > + char *tmp = NULL; > > > > > + size_t len = 0; > > > > > + size_t slen = 0; > > > > > + > > > > > + if (!rootfs_path || !rootfslen ) > > > > > + return NULL; > > > > > + > > > > > + *rootfslen = 0; > > > > > + > > > > > + s1 = strdup(rootfs_path); > > > > > + if (!s1) > > > > > + return NULL; > > > > > + > > > > > + s2 = malloc(strlen(rootfs_path) + 1); > > > > > + if (!s2) { > > > > > + free(s1); > > > > > + return NULL; > > > > > + } > > > > > + end = stpcpy(s2, rootfs_path); > > > > > + > > > > > + /* If we find :/ in rootfs_path it means we either have a > > > > > block-dev or > > > > > + * overlay or aufs container. */ > > > > > + while ((tmp = strrchr(s1, ':'))) { > > > > > + len = strlen(tmp); > > > > > + *rootfslen += len; > > > > > > > > If it is "overlay:/some/rootfs:/some/delta0", you will find the > > > > delta0 and return it as rootfsdir? > > > > > > > > (or am i misreading?) > > > Nope, I thought that in the case where the /some/rootfs refers to another > > > containers rootfs it doesn't make sense to check against that path since > > > this is > > > covered by the left hand side of the check further down. But in the case > > > of > > > purely overlay-backed container it doesn't make sense... I'll rewrite that > > > part. Are there any more special cases apart from aufs and overlay. We > > > have > > > > > > blockdev:/some/path > > > > > > overlayfs:/some/path:/some/delta0 > > > aufs:/some/path:/some/delta0 > > > > > > Any more? > > > > well bdev.c also looks for > > > > loop:, lvm:, nbd:, and even dir:. Not sure anyone uses these. > > > > But do they all have the rootfs path after the first ":/" delimiter? If so we > can abstract here and don't need to check for every backing storage type. > (Sorry, I'm more familiar with the codebase now but not so intimate as to be > able to answer that with certainty.) Yeah, well the rootfs source (i.e. nbd:/dev/nbd0) Like I said I think t his is redundant for these, and noone afaik has ever used them, somaybe we should drop them. ___ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
Re: [lxc-devel] [PATCH v3] Parse rootfs->path
On Tue, Oct 20, 2015 at 04:31:19PM +, Serge Hallyn wrote: > Quoting Christian Brauner (christianvanbrau...@gmail.com): > > On Tue, Oct 20, 2015 at 03:30:19PM +, 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 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 > > > > --- > > > > src/lxc/conf.c | 115 > > > > - > > > > 1 file changed, 90 insertions(+), 25 deletions(-) > > > > > > > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c > > > > index 16a62f8..301fe50 100644 > > > > --- a/src/lxc/conf.c > > > > +++ b/src/lxc/conf.c > > > > @@ -1815,12 +1815,65 @@ static void cull_mntent_opt(struct mntent > > > > *mntent) > > > > } > > > > } > > > > > > > > +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t > > > > *rootfslen) > > > > +{ > > > > + char *end = NULL; > > > > + char *s1 = NULL; > > > > + char *s2 = NULL; > > > > + char *rootfsdir = NULL; > > > > + char *tmp = NULL; > > > > + size_t len = 0; > > > > + size_t slen = 0; > > > > + > > > > + if (!rootfs_path || !rootfslen ) > > > > + return NULL; > > > > + > > > > + *rootfslen = 0; > > > > + > > > > + s1 = strdup(rootfs_path); > > > > + if (!s1) > > > > + return NULL; > > > > + > > > > + s2 = malloc(strlen(rootfs_path) + 1); > > > > + if (!s2) { > > > > + free(s1); > > > > + return NULL; > > > > + } > > > > + end = stpcpy(s2, rootfs_path); > > > > + > > > > + /* If we find :/ in rootfs_path it means we either have a > > > > block-dev or > > > > +* overlay or aufs container. */ > > > > + while ((tmp = strrchr(s1, ':'))) { > > > > + len = strlen(tmp); > > > > + *rootfslen += len; > > > > > > If it is "overlay:/some/rootfs:/some/delta0", you will find the > > > delta0 and return it as rootfsdir? > > > > > > (or am i misreading?) > > Nope, I thought that in the case where the /some/rootfs refers to another > > containers rootfs it doesn't make sense to check against that path since > > this is > > covered by the left hand side of the check further down. But in the case of > > purely overlay-backed container it doesn't make sense... I'll rewrite that > > part. Are there any more special cases apart from aufs and overlay. We have > > > > blockdev:/some/path > > > > overlayfs:/some/path:/some/delta0 > > aufs:/some/path:/some/delta0 > > > > Any more? > > well bdev.c also looks for > > loop:, lvm:, nbd:, and even dir:. Not sure anyone uses these. > But do they all have the rootfs path after the first ":/" delimiter? If so we can abstract here and don't need to check for every backing storage type. (Sorry, I'm more familiar with the codebase now but not so intimate as to be able to answer that with certainty.) signature.asc Description: PGP signature ___ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
Re: [lxc-devel] [PATCH v3] Parse rootfs->path
Quoting Christian Brauner (christianvanbrau...@gmail.com): > On Tue, Oct 20, 2015 at 03:30:19PM +, 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 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 > > > --- > > > src/lxc/conf.c | 115 > > > - > > > 1 file changed, 90 insertions(+), 25 deletions(-) > > > > > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c > > > index 16a62f8..301fe50 100644 > > > --- a/src/lxc/conf.c > > > +++ b/src/lxc/conf.c > > > @@ -1815,12 +1815,65 @@ static void cull_mntent_opt(struct mntent *mntent) > > > } > > > } > > > > > > +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t > > > *rootfslen) > > > +{ > > > + char *end = NULL; > > > + char *s1 = NULL; > > > + char *s2 = NULL; > > > + char *rootfsdir = NULL; > > > + char *tmp = NULL; > > > + size_t len = 0; > > > + size_t slen = 0; > > > + > > > + if (!rootfs_path || !rootfslen ) > > > + return NULL; > > > + > > > + *rootfslen = 0; > > > + > > > + s1 = strdup(rootfs_path); > > > + if (!s1) > > > + return NULL; > > > + > > > + s2 = malloc(strlen(rootfs_path) + 1); > > > + if (!s2) { > > > + free(s1); > > > + return NULL; > > > + } > > > + end = stpcpy(s2, rootfs_path); > > > + > > > + /* If we find :/ in rootfs_path it means we either have a block-dev or > > > + * overlay or aufs container. */ > > > + while ((tmp = strrchr(s1, ':'))) { > > > + len = strlen(tmp); > > > + *rootfslen += len; > > > > If it is "overlay:/some/rootfs:/some/delta0", you will find the > > delta0 and return it as rootfsdir? > > > > (or am i misreading?) > Nope, I thought that in the case where the /some/rootfs refers to another > containers rootfs it doesn't make sense to check against that path since this > is > covered by the left hand side of the check further down. But in the case of > purely overlay-backed container it doesn't make sense... I'll rewrite that > part. Are there any more special cases apart from aufs and overlay. We have > > blockdev:/some/path > > overlayfs:/some/path:/some/delta0 > aufs:/some/path:/some/delta0 > > Any more? well bdev.c also looks for loop:, lvm:, nbd:, and even dir:. Not sure anyone uses these. > > > > > + if (strncmp(tmp, ":/", 2) == 0) { > > > + rootfsdir = strdup(end - *rootfslen + 1); > > > + break; > > > + } else { > > > + slen = strlen(s1); > > > + s1[slen - len] = '\0'; > > > + } > > > + } > > > + free(s1); > > > + > > > + if (!*rootfslen && !tmp) > > > + rootfsdir = s2; > > > + else > > > + free(s2); > > > + > > > + if (rootfsdir) > > > + *rootfslen = strlen(rootfsdir); > > > + > > > + return rootfsdir; > > > +} > > ___ > > lxc-devel mailing list > > lxc-devel@lists.linuxcontainers.org > > http://lists.linuxcontainers.org/listinfo/lxc-devel > ___ > lxc-devel mailing list > lxc-devel@lists.linuxcontainers.org > http://lists.linuxcontainers.org/listinfo/lxc-devel ___ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
Re: [lxc-devel] [PATCH v2] Update absolute paths for overlay and aufs mounts
Quoting Christian Brauner (christianvanbrau...@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_union_mount_entry_paths() in > lxccontainer.c. The function updates the mounts in two locations: > > 1) lxc_conf->mount_list > > and > > 2) lxc_conf->unexpanded_config > > 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, > that an entry in lxc_conf->mount_list will only be updated if it is also found > in the clones config. Mounts from other files are hence not updated. In short, > automatic overlay and aufs mounts with lxc.mount.entry should only be > specified > in the containers own config. > > 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_union_mount_entry_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 > --- > src/lxc/lxccontainer.c | 133 > - > 1 file changed, 132 insertions(+), 1 deletion(-) > > diff --git a/src/lxc/lxccontainer.c b/src/lxc/lxccontainer.c > index 42e23e7..5ed7697 100644 > --- a/src/lxc/lxccontainer.c > +++ b/src/lxc/lxccontainer.c > @@ -2894,6 +2894,130 @@ static int create_file_dirname(char *path, struct > lxc_conf *conf) > return ret; > } > > +/* 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_union_mount_entry_paths() in lxccontainer.c. The function operates > on > +* c->lxc_conf->unexpanded_config instead of the intuitively plausible > +* c->lxc_conf->mount_list because the latter also contains mounts from other > +* files as well as generic mounts. */ > +static int update_union_mount_entry_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; > + char *mnt_entry = NULL; > + char *new_mnt_entry = NULL; > + char *new_unexpanded_config = NULL; > + char *tmp_mnt_entry = NULL; > + char *tmp_unexpanded_config = NULL; > + char *tmp = NULL; > + int ret = 0; > + size_t len = 0; > + struct lxc_list *iterator; > + > + cleanpath = strdup(newpath); > + if (!cleanpath) > + goto err; > + > + remove_trailing_slashes(cleanpath); > + > + 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) { put char *tmp = NULL, new_mnt_entry = NULL; char *mnt_entry; here (and remove from above etc). > + mnt_entry = iterator->elem; > + > + if (strstr(mnt_entry, "overlay")) > + tmp = "upperdir"; > + else if (strstr(mnt_entry, "aufs")) > + tmp = "br"; > + > + if (!tmp) > +
Re: [lxc-devel] [PATCH v3] Parse rootfs->path
On Tue, Oct 20, 2015 at 03:30:19PM +, 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 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 > > --- > > src/lxc/conf.c | 115 > > - > > 1 file changed, 90 insertions(+), 25 deletions(-) > > > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c > > index 16a62f8..301fe50 100644 > > --- a/src/lxc/conf.c > > +++ b/src/lxc/conf.c > > @@ -1815,12 +1815,65 @@ static void cull_mntent_opt(struct mntent *mntent) > > } > > } > > > > +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t *rootfslen) > > +{ > > + char *end = NULL; > > + char *s1 = NULL; > > + char *s2 = NULL; > > + char *rootfsdir = NULL; > > + char *tmp = NULL; > > + size_t len = 0; > > + size_t slen = 0; > > + > > + if (!rootfs_path || !rootfslen ) > > + return NULL; > > + > > + *rootfslen = 0; > > + > > + s1 = strdup(rootfs_path); > > + if (!s1) > > + return NULL; > > + > > + s2 = malloc(strlen(rootfs_path) + 1); > > + if (!s2) { > > + free(s1); > > + return NULL; > > + } > > + end = stpcpy(s2, rootfs_path); > > + > > + /* If we find :/ in rootfs_path it means we either have a block-dev or > > +* overlay or aufs container. */ > > + while ((tmp = strrchr(s1, ':'))) { > > + len = strlen(tmp); > > + *rootfslen += len; > > If it is "overlay:/some/rootfs:/some/delta0", you will find the > delta0 and return it as rootfsdir? > > (or am i misreading?) Nope, I thought that in the case where the /some/rootfs refers to another containers rootfs it doesn't make sense to check against that path since this is covered by the left hand side of the check further down. But in the case of purely overlay-backed container it doesn't make sense... I'll rewrite that part. Are there any more special cases apart from aufs and overlay. We have blockdev:/some/path overlayfs:/some/path:/some/delta0 aufs:/some/path:/some/delta0 Any more? > > > + if (strncmp(tmp, ":/", 2) == 0) { > > + rootfsdir = strdup(end - *rootfslen + 1); > > + break; > > + } else { > > + slen = strlen(s1); > > + s1[slen - len] = '\0'; > > + } > > + } > > + free(s1); > > + > > + if (!*rootfslen && !tmp) > > + rootfsdir = s2; > > + else > > + free(s2); > > + > > + if (rootfsdir) > > + *rootfslen = strlen(rootfsdir); > > + > > + return rootfsdir; > > +} > ___ > lxc-devel mailing list > lxc-devel@lists.linuxcontainers.org > http://lists.linuxcontainers.org/listinfo/lxc-devel signature.asc Description: PGP signature ___ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
Re: [lxc-devel] [PATCH v3] 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 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 > --- > src/lxc/conf.c | 115 > - > 1 file changed, 90 insertions(+), 25 deletions(-) > > diff --git a/src/lxc/conf.c b/src/lxc/conf.c > index 16a62f8..301fe50 100644 > --- a/src/lxc/conf.c > +++ b/src/lxc/conf.c > @@ -1815,12 +1815,65 @@ static void cull_mntent_opt(struct mntent *mntent) > } > } > > +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t *rootfslen) > +{ > + char *end = NULL; > + char *s1 = NULL; > + char *s2 = NULL; > + char *rootfsdir = NULL; > + char *tmp = NULL; > + size_t len = 0; > + size_t slen = 0; > + > + if (!rootfs_path || !rootfslen ) > + return NULL; > + > + *rootfslen = 0; > + > + s1 = strdup(rootfs_path); > + if (!s1) > + return NULL; > + > + s2 = malloc(strlen(rootfs_path) + 1); > + if (!s2) { > + free(s1); > + return NULL; > + } > + end = stpcpy(s2, rootfs_path); > + > + /* If we find :/ in rootfs_path it means we either have a block-dev or > + * overlay or aufs container. */ > + while ((tmp = strrchr(s1, ':'))) { > + len = strlen(tmp); > + *rootfslen += len; If it is "overlay:/some/rootfs:/some/delta0", you will find the delta0 and return it as rootfsdir? (or am i misreading?) > + if (strncmp(tmp, ":/", 2) == 0) { > + rootfsdir = strdup(end - *rootfslen + 1); > + break; > + } else { > + slen = strlen(s1); > + s1[slen - len] = '\0'; > + } > + } > + free(s1); > + > + if (!*rootfslen && !tmp) > + rootfsdir = s2; > + else > + free(s2); > + > + if (rootfsdir) > + *rootfslen = strlen(rootfsdir); > + > + return rootfsdir; > +} ___ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel
[lxc-devel] [PATCH v3] Parse rootfs->path
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 --- src/lxc/conf.c | 115 - 1 file changed, 90 insertions(+), 25 deletions(-) diff --git a/src/lxc/conf.c b/src/lxc/conf.c index 16a62f8..301fe50 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -1815,12 +1815,65 @@ static void cull_mntent_opt(struct mntent *mntent) } } +static char *ovl_get_rootfs_dir(const char *rootfs_path, size_t *rootfslen) +{ + char *end = NULL; + char *s1 = NULL; + char *s2 = NULL; + char *rootfsdir = NULL; + char *tmp = NULL; + size_t len = 0; + size_t slen = 0; + + if (!rootfs_path || !rootfslen ) + return NULL; + + *rootfslen = 0; + + s1 = strdup(rootfs_path); + if (!s1) + return NULL; + + s2 = malloc(strlen(rootfs_path) + 1); + if (!s2) { + free(s1); + return NULL; + } + end = stpcpy(s2, rootfs_path); + + /* If we find :/ in rootfs_path it means we either have a block-dev or +* overlay or aufs container. */ + while ((tmp = strrchr(s1, ':'))) { + len = strlen(tmp); + *rootfslen += len; + if (strncmp(tmp, ":/", 2) == 0) { + rootfsdir = strdup(end - *rootfslen + 1); + break; + } else { + slen = strlen(s1); + s1[slen - len] = '\0'; + } + } + free(s1); + + if (!*rootfslen && !tmp) + rootfsdir = s2; + else + free(s2); + + if (rootfsdir) + *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 +1885,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 +1901,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");
[lxc-devel] [PATCH v3] Parse rootfs->path
- add NULL-pointer check - initialize rootfslen Christian Brauner (1): Parse rootfs->path src/lxc/conf.c | 115 - 1 file changed, 90 insertions(+), 25 deletions(-) -- 2.6.1 ___ lxc-devel mailing list lxc-devel@lists.linuxcontainers.org http://lists.linuxcontainers.org/listinfo/lxc-devel