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 <christianvanbrau...@gmail.com>
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 @@ static int mount_entry_create_aufs_dirs(const struct
> mntent *mntent,
> WARN("Failed to create upperdir");
> }
>
> - free(lxcpath);
> lxc_free_array((void **)opts, free);
> return 0;
> }
>
> +
> static int mount_entry_create_dir_file(const struct mntent *mntent,
> - const char* path, const struct
> lxc_rootfs *rootfs)
> + const char* path, const struct
> lxc_rootfs *rootfs,
> + const char *lxc_name, const char
> *lxc_path)
> {
> char *pathdirname = NULL;
> int ret = 0;
> FILE *pathfile = NULL;
>
> if (strncmp(mntent->mnt_type, "overlay", 7) == 0) {
> - if (mount_entry_create_overlay_dirs(mntent, rootfs) < 0)
> + if (mount_entry_create_overlay_dirs(mntent, rootfs, lxc_name,
> lxc_path) < 0)
> return -1;
> } else if (strncmp(mntent->mnt_type, "aufs", 4) == 0) {
> - if (mount_entry_create_aufs_dirs(mntent, rootfs) < 0)
> + if (mount_entry_create_aufs_dirs(mntent, rootfs, lxc_name,
> lxc_path) < 0)
> return -1;
> }
>
> @@ -1986,14 +1974,15 @@ static int mount_entry_create_dir_file(const struct
> mntent *mntent,
> }
>
> static inline int mount_entry_on_generic(struct mntent *mntent,
> - const char* path, const struct lxc_rootfs *rootfs)
> + const char* path, const struct lxc_rootfs *rootfs,
> + const char *lxc_name, const char *lxc_path)
> {
> unsigned long mntflags;
> char *mntdata;
> int ret;
> bool optional = hasmntopt(mntent, "optional") != NULL;
>
> - ret = mount_entry_create_dir_file(mntent, path, rootfs);
> + ret = mount_entry_create_dir_file(mntent, path, rootfs, lxc_name,
> lxc_path);
>
> if (ret < 0)
> return optional ? 0 : -1;
> @@ -2015,12 +2004,13 @@ static inline int mount_entry_on_generic(struct
> mntent *mntent,
>
> static inline int mount_entry_on_systemfs(struct mntent *mntent)
> {
> - return mount_entry_on_generic(mntent, mntent->mnt_dir, NULL);
> + return mount_entry_on_generic(mntent, mntent->mnt_dir, NULL, NULL, NULL);
> }
>
> static int mount_entry_on_absolute_rootfs(struct mntent *mntent,
> const struct lxc_rootfs *rootfs,
> - const char *lxc_name)
> + const char *lxc_name,
> + const char *lxc_path)
> {
> char *aux;
> char path[MAXPATHLEN];
> @@ -2062,11 +2052,13 @@ skipabs:
> return -1;
> }
>
> - return mount_entry_on_generic(mntent, path, rootfs);
> + return mount_entry_on_generic(mntent, path, rootfs, lxc_name, lxc_path);
> }
>
> static int mount_entry_on_relative_rootfs(struct mntent *mntent,
> - const struct lxc_rootfs *rootfs)
> + const struct lxc_rootfs *rootfs,
> + const char *lxc_name,
> + const char *lxc_path)
> {
> char path[MAXPATHLEN];
> int ret;
> @@ -2078,11 +2070,11 @@ static int mount_entry_on_relative_rootfs(struct
> mntent *mntent,
> return -1;
> }
>
> - return mount_entry_on_generic(mntent, path, rootfs);
> + return mount_entry_on_generic(mntent, path, rootfs, lxc_name, lxc_path);
> }
>
> static int mount_file_entries(const struct lxc_rootfs *rootfs, FILE *file,
> - const char *lxc_name)
> + const char *lxc_name, const char *lxc_path)
> {
> struct mntent mntent;
> char buf[4096];
> @@ -2098,13 +2090,12 @@ static int mount_file_entries(const struct lxc_rootfs
> *rootfs, FILE *file,
>
> /* We have a separate root, mounts are relative to it */
> if (mntent.mnt_dir[0] != '/') {
> - if (mount_entry_on_relative_rootfs(&mntent,
> - rootfs))
> + if (mount_entry_on_relative_rootfs(&mntent, rootfs,
> lxc_name, lxc_path))
> goto out;
> continue;
> }
>
> - if (mount_entry_on_absolute_rootfs(&mntent, rootfs, lxc_name))
> + if (mount_entry_on_absolute_rootfs(&mntent, rootfs, lxc_name,
> lxc_path))
> goto out;
> }
>
> @@ -2116,7 +2107,7 @@ out:
> }
>
> static int setup_mount(const struct lxc_rootfs *rootfs, const char *fstab,
> - const char *lxc_name)
> + const char *lxc_name, const char *lxc_path)
> {
> FILE *file;
> int ret;
> @@ -2130,7 +2121,7 @@ static int setup_mount(const struct lxc_rootfs *rootfs,
> const char *fstab,
> return -1;
> }
>
> - ret = mount_file_entries(rootfs, file, lxc_name);
> + ret = mount_file_entries(rootfs, file, lxc_name, lxc_path);
>
> endmntent(file);
> return ret;
> @@ -2158,7 +2149,7 @@ FILE *write_mount_file(struct lxc_list *mount)
> }
>
> static int setup_mount_entries(const struct lxc_rootfs *rootfs, struct
> lxc_list *mount,
> - const char *lxc_name)
> + const char *lxc_name, const char *lxc_path)
> {
> FILE *file;
> int ret;
> @@ -2167,7 +2158,7 @@ static int setup_mount_entries(const struct lxc_rootfs
> *rootfs, struct lxc_list
> if (!file)
> return -1;
>
> - ret = mount_file_entries(rootfs, file, lxc_name);
> + ret = mount_file_entries(rootfs, file, lxc_name, lxc_path);
>
> fclose(file);
> return ret;
> @@ -3891,12 +3882,12 @@ int lxc_setup(struct lxc_handler *handler)
> return -1;
> }
>
> - if (setup_mount(&lxc_conf->rootfs, lxc_conf->fstab, name)) {
> + if (setup_mount(&lxc_conf->rootfs, lxc_conf->fstab, name, lxcpath)) {
> ERROR("failed to setup the mounts for '%s'", name);
> return -1;
> }
>
> - if (!lxc_list_empty(&lxc_conf->mount_list) &&
> setup_mount_entries(&lxc_conf->rootfs, &lxc_conf->mount_list, name)) {
> + if (!lxc_list_empty(&lxc_conf->mount_list) &&
> setup_mount_entries(&lxc_conf->rootfs, &lxc_conf->mount_list, name, lxcpath))
> {
> ERROR("failed to setup the mount entries for '%s'", name);
> return -1;
> }
> --
> 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
