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