Re: [lxc-devel] [PATCH] lxc_start: report error message when config is not a regular file
Quoting Yang Shi (yang@linaro.org):
> When run the below command:
> lxc-start -n name -f /path/to/config
>
> If config is not a regular file, lxc-start will be killed by SEG FAULT
> directly.
What exactly was the file to get a segfault? When I try with a device
or a directory I do not get a segfault.
> Exit with some error message to improve user experience.
This is not the place to put that check (if it is needed). It should go
into either lxc_container_new or load_config, wherever it is failing.
> Signed-off-by: Yang Shi
> ---
> src/lxc/lxc_start.c | 13 +
> 1 file changed, 13 insertions(+)
>
> diff --git a/src/lxc/lxc_start.c b/src/lxc/lxc_start.c
> index 6b942ac..d1cf779 100644
> --- a/src/lxc/lxc_start.c
> +++ b/src/lxc/lxc_start.c
> @@ -212,6 +212,7 @@ int main(int argc, char *argv[])
> NULL,
> };
> struct lxc_container *c;
> + struct stat st_stat = {0};
>
> lxc_list_init(&defines);
>
> @@ -242,6 +243,18 @@ int main(int argc, char *argv[])
> /* rcfile is specified in the cli option */
> if (my_args.rcfile) {
> rcfile = (char *)my_args.rcfile;
> + tmp = stat(rcfile, &st_stat);
> +
> + if (tmp && errno == ENOENT) {
> + ERROR("The config path doesn't exist.\n");
> + return err;
> + }
> +
> + if (!tmp && !S_ISREG(st_stat.st_mode)) {
> + ERROR("The config is not a regular file.\n");
> + return err;
> + }
> +
> c = lxc_container_new(my_args.name, lxcpath);
> if (!c) {
> ERROR("Failed to create lxc_container");
> --
> 2.0.2
>
> ___
> 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] lxc_start: report error message when config is not a regular file
On 10/15/2015 7:31 AM, Serge Hallyn wrote:
Quoting Yang Shi (yang@linaro.org):
When run the below command:
lxc-start -n name -f /path/to/config
If config is not a regular file, lxc-start will be killed by SEG FAULT
directly.
What exactly was the file to get a segfault? When I try with a device
or a directory I do not get a segfault.
I'm using lxc 1.1.3 stable release on armv8 target, when I run the below
command I got segment fault.
# lxc-start -n test -f /var/lib/lxc/test/
lxc-start[309]: unhandled level 2 translation fault (11) at 0x,
esr 0x9206
pgd = 80834780e000
[] *pgd=0083cb385003, *pud=0083c99e3003,
*pmd=
CPU: 2 PID: 309 Comm: lxc-start Not tainted 4.1.10 #25
Hardware name: Freescale Layerscape 2085a RDB Board (DT)
task: 80007b944d00 ti: 808347874000 task.ti: 808347874000
PC is at 0x9d1ff2d0
LR is at 0x9d1bc0d0
pc : [<9d1ff2d0>] lr : [<9d1bc0d0>] pstate: 8000
sp : fd9f3af0
x29: fd9f3af0 x28: 00413000
x27: x26: 0001
x25: x24: 00414370
x23: fd9f3be0 x22: 9d35d3b0
x21: 000f x20: 9d37c000
x19: 004166d0 x18: 0001
x17: 9d1bc1e8 x16: 9d37a458
x15: 12010011 x14: 004167e0
x13: x12: 0023
x11: 0023 x10: 0101010101010101
x9 : 7f7f7f7f7f7f7f7f x8 : 0101010101010101
x7 : 7f7f7f7f7f7f7f7f x6 : ff4753404f5e5245
x5 : 8000 x4 :
x3 : 00485441505f5346 x2 : 544f4f525f43584c
x1 : x0 :
Exit with some error message to improve user experience.
This is not the place to put that check (if it is needed). It should go
into either lxc_container_new or load_config, wherever it is failing.
Thanks. If you think the segment fault error sounds valid, I will try to
move the check to the right place.
Regards,
Yang
Signed-off-by: Yang Shi
---
src/lxc/lxc_start.c | 13 +
1 file changed, 13 insertions(+)
diff --git a/src/lxc/lxc_start.c b/src/lxc/lxc_start.c
index 6b942ac..d1cf779 100644
--- a/src/lxc/lxc_start.c
+++ b/src/lxc/lxc_start.c
@@ -212,6 +212,7 @@ int main(int argc, char *argv[])
NULL,
};
struct lxc_container *c;
+ struct stat st_stat = {0};
lxc_list_init(&defines);
@@ -242,6 +243,18 @@ int main(int argc, char *argv[])
/* rcfile is specified in the cli option */
if (my_args.rcfile) {
rcfile = (char *)my_args.rcfile;
+ tmp = stat(rcfile, &st_stat);
+
+ if (tmp && errno == ENOENT) {
+ ERROR("The config path doesn't exist.\n");
+ return err;
+ }
+
+ if (!tmp && !S_ISREG(st_stat.st_mode)) {
+ ERROR("The config is not a regular file.\n");
+ return err;
+ }
+
c = lxc_container_new(my_args.name, lxcpath);
if (!c) {
ERROR("Failed to create lxc_container");
--
2.0.2
___
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] ubuntu-cloud: Replace .tar.gz by .tar.xz and don't auto-generate missing tarballs
Quoting Stéphane Graber (stgra...@ubuntu.com):
> Signed-off-by: Stéphane Graber
Acked-by: Serge E. Hallyn
> ---
> templates/lxc-ubuntu-cloud.in | 46
> ---
> 1 file changed, 4 insertions(+), 42 deletions(-)
>
> diff --git a/templates/lxc-ubuntu-cloud.in b/templates/lxc-ubuntu-cloud.in
> index 4addf7b..5bdd758 100644
> --- a/templates/lxc-ubuntu-cloud.in
> +++ b/templates/lxc-ubuntu-cloud.in
> @@ -297,7 +297,7 @@ else
> [ "$stream" = "daily" ] || echo "You may try with '--stream=daily'"
> exit
> fi
> -url2=`echo $url1 | sed -e 's/.tar.gz/-root\0/'`
> +url2=`echo $url1 | sed -e 's/.tar.gz/-root\0/' -e 's/.tar.gz/.tar.xz/'`
> fi
>
> filename=`basename $url2`
> @@ -307,44 +307,6 @@ wgetcleanup()
> rm -f $filename
> }
>
> -buildcleanup()
> -{
> -cd $rootfs
> -umount -l $cache/$xdir || true
> -rm -rf $cache
> -}
> -
> -# if the release doesn't have a *-rootfs.tar.gz, then create one from the
> -# cloudimg.tar.gz by extracting the .img, mounting it loopback, and creating
> -# a tarball from the mounted image.
> -build_root_tgz()
> -{
> -url=$1
> -filename=$2
> -
> -xdir=`mktemp -d -p .`
> -tarname=`basename $url`
> -imgname="$release-*-cloudimg-$arch.img"
> -trap buildcleanup EXIT SIGHUP SIGINT SIGTERM
> -if [ $flushcache -eq 1 -o ! -f $cache/$tarname ]; then
> -rm -f $tarname
> -echo "Downloading cloud image from $url"
> -wget $url || { echo "Couldn't find cloud image $url."; exit 1; }
> -fi
> -echo "Creating new cached cloud image rootfs"
> -tar --wildcards -zxf "$tarname" "$imgname"
> -mount -o loop $imgname $xdir
> -(cd $xdir; tar --numeric-owner -cpzf "../$filename" .)
> -umount $xdir
> -rm -f $tarname $imgname
> -rmdir $xdir
> -echo "New cloud image cache created"
> -trap EXIT
> -trap SIGHUP
> -trap SIGINT
> -trap SIGTERM
> -}
> -
> do_extract_rootfs() {
>
> cd $cache
> @@ -355,7 +317,7 @@ do_extract_rootfs() {
>
> trap wgetcleanup EXIT SIGHUP SIGINT SIGTERM
> if [ ! -f $filename ]; then
> -wget $url2 || build_root_tgz $url1 $filename
> +wget $url2
> fi
> trap EXIT
> trap SIGHUP
> @@ -366,10 +328,10 @@ do_extract_rootfs() {
> mkdir -p $rootfs
> cd $rootfs
> if [ $in_userns -eq 1 ]; then
> -tar --anchored --exclude="dev/*" --numeric-owner -xpzf
> "$cache/$filename"
> +tar --anchored --exclude="dev/*" --numeric-owner -xpJf
> "$cache/$filename"
> mkdir -p $rootfs/dev/pts/
> else
> -tar --numeric-owner -xpzf "$cache/$filename"
> +tar --numeric-owner -xpJf "$cache/$filename"
> fi
> }
>
> --
> 2.5.0
>
> ___
> 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] [PATCH] Ignore trailing /init.scope in init cgroups
The lxc monitor does not store the container's cgroups, rather it
recalculates them whenever needed.
Systemd moves itself into a /init.scope cgroup for the systemd
controller.
It might be worth changing that (by storing all cgroup info in the
lxc_handler), but for now go the hacky route and chop off any
trailing /init.scope.
I definately thinkg we want to switch to storing as that will be
more bullet-proof, but for now we need a quick backportable fix
for systemd 226 guests.
Signed-off-by: Serge Hallyn
---
src/lxc/cgfs.c | 1 +
src/lxc/cgmanager.c | 1 +
src/lxc/cgroup.c| 14 ++
src/lxc/cgroup.h| 2 ++
4 files changed, 18 insertions(+)
diff --git a/src/lxc/cgfs.c b/src/lxc/cgfs.c
index df2e6b2..d65f2d7 100644
--- a/src/lxc/cgfs.c
+++ b/src/lxc/cgfs.c
@@ -1220,6 +1220,7 @@ static char *lxc_cgroup_get_hierarchy_path_data(const
char *subsystem, struct cg
info = find_info_for_subsystem(info, subsystem);
if (!info)
return NULL;
+ prune_init_scope(info->cgroup_path);
return info->cgroup_path;
}
diff --git a/src/lxc/cgmanager.c b/src/lxc/cgmanager.c
index a677c22..f868903 100644
--- a/src/lxc/cgmanager.c
+++ b/src/lxc/cgmanager.c
@@ -776,6 +776,7 @@ static char *try_get_abs_cgroup(const char *name, const
char *lxcpath,
nerr = nih_error_get();
nih_free(nerr);
}
+ prune_init_scope(cgroup);
return cgroup;
}
diff --git a/src/lxc/cgroup.c b/src/lxc/cgroup.c
index 2362ad8..b1c764f 100644
--- a/src/lxc/cgroup.c
+++ b/src/lxc/cgroup.c
@@ -194,3 +194,17 @@ cgroup_driver_t cgroup_driver(void)
{
return ops->driver;
}
+
+#define INIT_SCOPE "/init.scope"
+void prune_init_scope(char *cg)
+{
+ char *point = cg + strlen(cg) - strlen(INIT_SCOPE);
+ if (point < cg)
+ return;
+ if (strcmp(point, INIT_SCOPE) == 0) {
+ if (point == cg)
+ *(point+1) = '\0';
+ else
+ *point = '\0';
+ }
+}
diff --git a/src/lxc/cgroup.h b/src/lxc/cgroup.h
index 6706939..7704c04 100644
--- a/src/lxc/cgroup.h
+++ b/src/lxc/cgroup.h
@@ -80,4 +80,6 @@ extern bool cgroup_unfreeze(struct lxc_handler *handler);
extern void cgroup_disconnect(void);
extern cgroup_driver_t cgroup_driver(void);
+extern void prune_init_scope(char *cg);
+
#endif
--
2.5.0
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
[lxc-devel] [PATCH] Trivial: fix missing space before closing bracket
Signed-off-by: Christian Brauner
---
share/lxc.mount.hook.in | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/share/lxc.mount.hook.in b/share/lxc.mount.hook.in
index 7194ab8..5ed2d60 100755
--- a/share/lxc.mount.hook.in
+++ b/share/lxc.mount.hook.in
@@ -30,7 +30,7 @@ if [ -d "${LXC_ROOTFS_MOUNT}/sys/fs/cgroup" ]; then
continue
fi
- if [ ! -d ${LXC_ROOTFS_MOUNT}/sys/fs/cgroup/$DEST]; then
+ if [ ! -d ${LXC_ROOTFS_MOUNT}/sys/fs/cgroup/$DEST ]; then
mkdir ${LXC_ROOTFS_MOUNT}/sys/fs/cgroup/$DEST
fi
--
2.6.1
___
lxc-devel mailing list
lxc-devel@lists.linuxcontainers.org
http://lists.linuxcontainers.org/listinfo/lxc-devel
Re: [lxc-devel] [PATCH] Trivial: fix missing space before closing bracket
Quoting Christian Brauner (christianvanbrau...@gmail.com):
> Signed-off-by: Christian Brauner
Acked-by: Serge E. Hallyn
> ---
> share/lxc.mount.hook.in | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/share/lxc.mount.hook.in b/share/lxc.mount.hook.in
> index 7194ab8..5ed2d60 100755
> --- a/share/lxc.mount.hook.in
> +++ b/share/lxc.mount.hook.in
> @@ -30,7 +30,7 @@ if [ -d "${LXC_ROOTFS_MOUNT}/sys/fs/cgroup" ]; then
> continue
> fi
>
> - if [ ! -d ${LXC_ROOTFS_MOUNT}/sys/fs/cgroup/$DEST]; then
> + if [ ! -d ${LXC_ROOTFS_MOUNT}/sys/fs/cgroup/$DEST ]; then
> mkdir ${LXC_ROOTFS_MOUNT}/sys/fs/cgroup/$DEST
> fi
>
> --
> 2.6.1
>
> ___
> 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] Make mount_entry_create_*_dirs() more robust
Quoting Christian Brauner (christianvanbrau...@gmail.com):
> The mount_entry_create_*_dirs() functions currently assume that the rootfs of
> the container is actually named "rootfs". This has the consequence that
>
> del = strstr(lxcpath, "/rootfs");
> if (!del) {
> free(lxcpath);
> lxc_free_array((void **)opts, free);
> return -1;
> }
> *del = '\0';
>
> will return NULL when the rootfs of a container is not actually named
> "rootfs".
> This means the we return -1 and do not create the necessary upperdir/workdir
> directories required for the overlay/aufs mount to work. Hence, let's not make
> that assumption. We now pass lxc_path and lxc_name to
> mount_entry_create_*_dirs() and create the path directly. To prevent failure
> we
> also have mount_entry_create_*_dirs() check that lxc_name and lxc_path are not
> empty when they are passed in.
>
> Signed-off-by: Christian Brauner
Yeah this was bugging me a few years ago. Overall the patch looks fine
to me, I'm running a full testsuite to ease my mind about it. Will ack
after that passes and I look over it again.
> ---
> src/lxc/conf.c | 91
> ++
> 1 file changed, 41 insertions(+), 50 deletions(-)
>
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index 0e3421b..16a62f8 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -1816,20 +1816,22 @@ static void cull_mntent_opt(struct mntent *mntent)
> }
>
> static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
> -const struct lxc_rootfs *rootfs)
> +const struct lxc_rootfs *rootfs,
> +const char *lxc_name,
> +const char *lxc_path)
> {
> - char *del = NULL;
> - char *lxcpath = NULL;
> + char lxcpath[MAXPATHLEN];
> char *upperdir = NULL;
> char *workdir = NULL;
> char **opts = NULL;
> + int ret = 0;
> size_t arrlen = 0;
> size_t dirlen = 0;
> size_t i;
> size_t len = 0;
> size_t rootfslen = 0;
>
> - if (!rootfs->path)
> + if (!rootfs->path || !lxc_name || !lxc_path)
> return -1;
>
> opts = lxc_string_split(mntent->mnt_opts, ',');
> @@ -1845,19 +1847,11 @@ static int mount_entry_create_overlay_dirs(const
> struct mntent *mntent,
> workdir = opts[i] + len;
> }
>
> - lxcpath = strdup(rootfs->path);
> - if (!lxcpath) {
> - lxc_free_array((void **)opts, free);
> - return -1;
> - }
> -
> - del = strstr(lxcpath, "/rootfs");
> - if (!del) {
> - free(lxcpath);
> + ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
> + if (ret < 0 || ret >= MAXPATHLEN) {
> lxc_free_array((void **)opts, free);
> return -1;
> }
> - *del = '\0';
>
> dirlen = strlen(lxcpath);
> rootfslen = strlen(rootfs->path);
> @@ -1877,25 +1871,26 @@ static int mount_entry_create_overlay_dirs(const
> struct mntent *mntent,
> WARN("Failed to create workdir");
> }
>
> - free(lxcpath);
> lxc_free_array((void **)opts, free);
> return 0;
> }
>
> static int mount_entry_create_aufs_dirs(const struct mntent *mntent,
> - const struct lxc_rootfs *rootfs)
> + const struct lxc_rootfs *rootfs,
> + const char *lxc_name,
> + const char *lxc_path)
> {
> - char *del = NULL;
> - char *lxcpath = NULL;
> + char lxcpath[MAXPATHLEN];
> char *scratch = NULL;
> char *tmp = NULL;
> char *upperdir = NULL;
> char **opts = NULL;
> + int ret = 0;
> size_t arrlen = 0;
> size_t i;
> size_t len = 0;
>
> - if (!rootfs->path)
> + if (!rootfs->path || !lxc_name || !lxc_path)
> return -1;
>
> opts = lxc_string_split(mntent->mnt_opts, ',');
> @@ -1919,19 +1914,11 @@ static int mount_entry_create_aufs_dirs(const struct
> mntent *mntent,
> return -1;
> }
>
> - lxcpath = strdup(rootfs->path);
> - if (!lxcpath) {
> - lxc_free_array((void **)opts, free);
> - return -1;
> - }
> -
> - del = strstr(lxcpath, "/rootfs");
> - if (!del) {
> - free(lxcpath);
> + ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
> + if (ret < 0 || ret >= MAXPATHLEN) {
> lxc_free_array((void **)opts, free);
> return -1;
> }
> - *del = '\0';
>
> /* We neither allow users to create upperdirs outside the containerdir
>* nor inside the rootfs. The latter might be debatable. */
> @@ -1940,23 +1927,24 @@ stati
Re: [lxc-devel] [PATCH] Make mount_entry_create_*_dirs() more robust
Quoting Christian Brauner (christianvanbrau...@gmail.com):
> The mount_entry_create_*_dirs() functions currently assume that the rootfs of
> the container is actually named "rootfs". This has the consequence that
>
> del = strstr(lxcpath, "/rootfs");
> if (!del) {
> free(lxcpath);
> lxc_free_array((void **)opts, free);
> return -1;
> }
> *del = '\0';
>
> will return NULL when the rootfs of a container is not actually named
> "rootfs".
> This means the we return -1 and do not create the necessary upperdir/workdir
> directories required for the overlay/aufs mount to work. Hence, let's not make
> that assumption. We now pass lxc_path and lxc_name to
> mount_entry_create_*_dirs() and create the path directly. To prevent failure
> we
> also have mount_entry_create_*_dirs() check that lxc_name and lxc_path are not
> empty when they are passed in.
>
> Signed-off-by: Christian Brauner
Acked-by: Serge E. Hallyn
> ---
> src/lxc/conf.c | 91
> ++
> 1 file changed, 41 insertions(+), 50 deletions(-)
>
> diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> index 0e3421b..16a62f8 100644
> --- a/src/lxc/conf.c
> +++ b/src/lxc/conf.c
> @@ -1816,20 +1816,22 @@ static void cull_mntent_opt(struct mntent *mntent)
> }
>
> static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
> -const struct lxc_rootfs *rootfs)
> +const struct lxc_rootfs *rootfs,
> +const char *lxc_name,
> +const char *lxc_path)
> {
> - char *del = NULL;
> - char *lxcpath = NULL;
> + char lxcpath[MAXPATHLEN];
> char *upperdir = NULL;
> char *workdir = NULL;
> char **opts = NULL;
> + int ret = 0;
> size_t arrlen = 0;
> size_t dirlen = 0;
> size_t i;
> size_t len = 0;
> size_t rootfslen = 0;
>
> - if (!rootfs->path)
> + if (!rootfs->path || !lxc_name || !lxc_path)
> return -1;
>
> opts = lxc_string_split(mntent->mnt_opts, ',');
> @@ -1845,19 +1847,11 @@ static int mount_entry_create_overlay_dirs(const
> struct mntent *mntent,
> workdir = opts[i] + len;
> }
>
> - lxcpath = strdup(rootfs->path);
> - if (!lxcpath) {
> - lxc_free_array((void **)opts, free);
> - return -1;
> - }
> -
> - del = strstr(lxcpath, "/rootfs");
> - if (!del) {
> - free(lxcpath);
> + ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
> + if (ret < 0 || ret >= MAXPATHLEN) {
> lxc_free_array((void **)opts, free);
> return -1;
> }
> - *del = '\0';
>
> dirlen = strlen(lxcpath);
> rootfslen = strlen(rootfs->path);
> @@ -1877,25 +1871,26 @@ static int mount_entry_create_overlay_dirs(const
> struct mntent *mntent,
> WARN("Failed to create workdir");
> }
>
> - free(lxcpath);
> lxc_free_array((void **)opts, free);
> return 0;
> }
>
> static int mount_entry_create_aufs_dirs(const struct mntent *mntent,
> - const struct lxc_rootfs *rootfs)
> + const struct lxc_rootfs *rootfs,
> + const char *lxc_name,
> + const char *lxc_path)
> {
> - char *del = NULL;
> - char *lxcpath = NULL;
> + char lxcpath[MAXPATHLEN];
> char *scratch = NULL;
> char *tmp = NULL;
> char *upperdir = NULL;
> char **opts = NULL;
> + int ret = 0;
> size_t arrlen = 0;
> size_t i;
> size_t len = 0;
>
> - if (!rootfs->path)
> + if (!rootfs->path || !lxc_name || !lxc_path)
> return -1;
>
> opts = lxc_string_split(mntent->mnt_opts, ',');
> @@ -1919,19 +1914,11 @@ static int mount_entry_create_aufs_dirs(const struct
> mntent *mntent,
> return -1;
> }
>
> - lxcpath = strdup(rootfs->path);
> - if (!lxcpath) {
> - lxc_free_array((void **)opts, free);
> - return -1;
> - }
> -
> - del = strstr(lxcpath, "/rootfs");
> - if (!del) {
> - free(lxcpath);
> + ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
> + if (ret < 0 || ret >= MAXPATHLEN) {
> lxc_free_array((void **)opts, free);
> return -1;
> }
> - *del = '\0';
>
> /* We neither allow users to create upperdirs outside the containerdir
>* nor inside the rootfs. The latter might be debatable. */
> @@ -1940,23 +1927,24 @@ static int mount_entry_create_aufs_dirs(const struct
> mntent *mntent,
> WARN("Failed to create upperdir");
> }
>
> - free(
Re: [lxc-devel] [PATCH] Make mount_entry_create_*_dirs() more robust
On Thu, Oct 15, 2015 at 08:32:25PM +, Serge Hallyn wrote:
> Quoting Christian Brauner (christianvanbrau...@gmail.com):
> > The mount_entry_create_*_dirs() functions currently assume that the rootfs
> > of
> > the container is actually named "rootfs". This has the consequence that
> >
> > del = strstr(lxcpath, "/rootfs");
> > if (!del) {
> > free(lxcpath);
> > lxc_free_array((void **)opts, free);
> > return -1;
> > }
> > *del = '\0';
> >
> > will return NULL when the rootfs of a container is not actually named
> > "rootfs".
> > This means the we return -1 and do not create the necessary upperdir/workdir
> > directories required for the overlay/aufs mount to work. Hence, let's not
> > make
> > that assumption. We now pass lxc_path and lxc_name to
> > mount_entry_create_*_dirs() and create the path directly. To prevent
> > failure we
> > also have mount_entry_create_*_dirs() check that lxc_name and lxc_path are
> > not
> > empty when they are passed in.
> >
> > Signed-off-by: Christian Brauner
>
> Yeah this was bugging me a few years ago. Overall the patch looks fine
> to me, I'm running a full testsuite to ease my mind about it. Will ack
> after that passes and I look over it again.
We should also consider parsing path->rootfs when the container is an overlay or
aufs backed container. Because in this case the right hand side of the check:
if ((strncmp(upperdir, lxcpath, dirlen) == 0) &&
(strncmp(upperdir, rootfs->path, rootfslen) != 0))
will be trivially true since path->rootfs will e.g. be overlayfs:/path1:path2.
Parsing path->rootfs to extract path2 before doing the second check would be
safer... Thoughts?
>
> > ---
> > src/lxc/conf.c | 91
> > ++
> > 1 file changed, 41 insertions(+), 50 deletions(-)
> >
> > diff --git a/src/lxc/conf.c b/src/lxc/conf.c
> > index 0e3421b..16a62f8 100644
> > --- a/src/lxc/conf.c
> > +++ b/src/lxc/conf.c
> > @@ -1816,20 +1816,22 @@ static void cull_mntent_opt(struct mntent *mntent)
> > }
> >
> > static int mount_entry_create_overlay_dirs(const struct mntent *mntent,
> > - const struct lxc_rootfs *rootfs)
> > + const struct lxc_rootfs *rootfs,
> > + const char *lxc_name,
> > + const char *lxc_path)
> > {
> > - char *del = NULL;
> > - char *lxcpath = NULL;
> > + char lxcpath[MAXPATHLEN];
> > char *upperdir = NULL;
> > char *workdir = NULL;
> > char **opts = NULL;
> > + int ret = 0;
> > size_t arrlen = 0;
> > size_t dirlen = 0;
> > size_t i;
> > size_t len = 0;
> > size_t rootfslen = 0;
> >
> > - if (!rootfs->path)
> > + if (!rootfs->path || !lxc_name || !lxc_path)
> > return -1;
> >
> > opts = lxc_string_split(mntent->mnt_opts, ',');
> > @@ -1845,19 +1847,11 @@ static int mount_entry_create_overlay_dirs(const
> > struct mntent *mntent,
> > workdir = opts[i] + len;
> > }
> >
> > - lxcpath = strdup(rootfs->path);
> > - if (!lxcpath) {
> > - lxc_free_array((void **)opts, free);
> > - return -1;
> > - }
> > -
> > - del = strstr(lxcpath, "/rootfs");
> > - if (!del) {
> > - free(lxcpath);
> > + ret = snprintf(lxcpath, MAXPATHLEN, "%s/%s", lxc_path, lxc_name);
> > + if (ret < 0 || ret >= MAXPATHLEN) {
> > lxc_free_array((void **)opts, free);
> > return -1;
> > }
> > - *del = '\0';
> >
> > dirlen = strlen(lxcpath);
> > rootfslen = strlen(rootfs->path);
> > @@ -1877,25 +1871,26 @@ static int mount_entry_create_overlay_dirs(const
> > struct mntent *mntent,
> > WARN("Failed to create workdir");
> > }
> >
> > - free(lxcpath);
> > lxc_free_array((void **)opts, free);
> > return 0;
> > }
> >
> > static int mount_entry_create_aufs_dirs(const struct mntent *mntent,
> > - const struct lxc_rootfs *rootfs)
> > + const struct lxc_rootfs *rootfs,
> > + const char *lxc_name,
> > + const char *lxc_path)
> > {
> > - char *del = NULL;
> > - char *lxcpath = NULL;
> > + char lxcpath[MAXPATHLEN];
> > char *scratch = NULL;
> > char *tmp = NULL;
> > char *upperdir = NULL;
> > char **opts = NULL;
> > + int ret = 0;
> > size_t arrlen = 0;
> > size_t i;
> > size_t len = 0;
> >
> > - if (!rootfs->path)
> > + if (!rootfs->path || !lxc_name || !lxc_path)
> > return -1;
> >
> > opts = lxc_string_split(mntent->mnt_opts, ',');
> > @@ -1919,19 +1914,11 @@ static int mount_entry_create_aufs_dirs(const
> > struct mntent *mntent,
> > return -1;
> > }
> >
> > - lxcpath = strd
