From 6437f1c1738d9ab0a51a75bccf3450c1352ccc8f Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Tue, 30 Oct 2018 15:38:50 +0100 Subject: [PATCH] conf: don't setup shared mountpoint for shmounts Leave it up to the caller for now until we have a clear way to do this without causing double mounts etc. Needed-by: https://github.com/lxc/lxd/issues/5227 Signed-off-by: Christian Brauner --- src/lxc/confile.c | 23 +++++++---- src/lxc/start.c | 80 ------------------------------------- src/tests/mount_injection.c | 46 +++++++++++++++++++-- 3 files changed, 57 insertions(+), 92 deletions(-) diff --git a/src/lxc/confile.c b/src/lxc/confile.c index 1d5e966e18..d4fdaf23d9 100644 --- a/src/lxc/confile.c +++ b/src/lxc/confile.c @@ -1788,24 +1788,31 @@ static int set_config_mount_auto(const char *key, const char *value, lxc_conf->auto_mounts |= allowed_auto_mounts[i].flag; if (is_shmounts) { - char *slide = token + STRLITERALLEN("shmounts:"); + char *container_path; + char *host_path; - if (*slide == '\0') { + host_path = token + STRLITERALLEN("shmounts:"); + if (*host_path == '\0') { SYSERROR("Failed to copy shmounts host path"); goto on_error; } - lxc_conf->shmount.path_host = strdup(slide); + container_path = strchr(host_path, ':'); + if (!container_path || *(container_path + 1) == '\0') + container_path = "/dev/.lxc-mounts"; + else + *container_path++ = '\0'; + + ERROR("AAAA: %s", host_path); + ERROR("BBBB: %s", container_path); + + lxc_conf->shmount.path_host = strdup(host_path); if (!lxc_conf->shmount.path_host) { SYSERROR("Failed to copy shmounts host path"); goto on_error; } - slide = strchr(slide, ':'); - if (!slide || *(++slide) == '\0') - slide = "/dev/.lxc-mounts"; - - lxc_conf->shmount.path_cont = strdup(slide); + lxc_conf->shmount.path_cont = strdup(container_path); if(!lxc_conf->shmount.path_cont) { SYSERROR("Failed to copy shmounts container path"); goto on_error; diff --git a/src/lxc/start.c b/src/lxc/start.c index da942a6e83..1cdae6c7a9 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -1578,75 +1578,6 @@ static inline int do_share_ns(void *arg) return 0; } -static int lxc_setup_shmount(struct lxc_conf *conf) -{ - size_t len_cont; - char *full_cont_path; - int ret = -1; - - /* Construct the shmount path under the container root. */ - len_cont = strlen(conf->rootfs.mount) + 1 + strlen(conf->shmount.path_cont); - /* +1 for the terminating '\0' */ - full_cont_path = malloc(len_cont + 1); - if (!full_cont_path) { - SYSERROR("Not enough memory"); - return -ENOMEM; - } - - ret = snprintf(full_cont_path, len_cont + 1, "%s/%s", - conf->rootfs.mount, conf->shmount.path_cont); - if (ret < 0 || ret >= len_cont + 1) { - SYSERROR("Failed to create filename"); - free(full_cont_path); - return -1; - } - - /* Check if shmount point is already set up. */ - if (is_shared_mountpoint(conf->shmount.path_host)) { - INFO("Path \"%s\" is already MS_SHARED. Reusing", - conf->shmount.path_host); - free(full_cont_path); - return 0; - } - - /* Create host and cont mount paths */ - ret = mkdir_p(conf->shmount.path_host, 0711); - if (ret < 0 && errno != EEXIST) { - SYSERROR("Failed to create directory \"%s\"", - conf->shmount.path_host); - free(full_cont_path); - return ret; - } - - ret = mkdir_p(full_cont_path, 0711); - if (ret < 0 && errno != EEXIST) { - SYSERROR("Failed to create directory \"%s\"", full_cont_path); - free(full_cont_path); - return ret; - } - - /* Prepare host mountpoint */ - ret = mount("tmpfs", conf->shmount.path_host, "tmpfs", 0, - "size=100k,mode=0711"); - if (ret < 0) { - SYSERROR("Failed to mount \"%s\"", conf->shmount.path_host); - free(full_cont_path); - return ret; - } - - ret = mount(conf->shmount.path_host, conf->shmount.path_host, "none", - MS_REC | MS_SHARED, ""); - if (ret < 0) { - SYSERROR("Failed to make shared \"%s\"", conf->shmount.path_host); - free(full_cont_path); - return ret; - } - - INFO("Setup shared mount point \"%s\"", conf->shmount.path_host); - free(full_cont_path); - return 0; -} - /* lxc_spawn() performs crucial setup tasks and clone()s the new process which * exec()s the requested container binary. * Note that lxc_spawn() runs in the parent namespaces. Any operations performed @@ -1693,17 +1624,6 @@ static int lxc_spawn(struct lxc_handler *handler) if (ret < 0) goto out_sync_fini; - if (conf->shmount.path_host) { - if (!conf->shmount.path_cont) - goto out_sync_fini; - - ret = lxc_setup_shmount(conf); - if (ret < 0) { - ERROR("Failed to setup shared mount point"); - goto out_sync_fini; - } - } - if (handler->ns_clone_flags & CLONE_NEWNET) { if (!lxc_list_empty(&conf->network)) { diff --git a/src/tests/mount_injection.c b/src/tests/mount_injection.c index 3d58b9c3ff..3ea15f4331 100644 --- a/src/tests/mount_injection.c +++ b/src/tests/mount_injection.c @@ -386,16 +386,54 @@ static int do_unpriv_container_test() return perform_container_test(NAME"unprivileged", config_items); } +static bool lxc_setup_shmount(const char *shmount_path) +{ + int ret; + + ret = mkdir_p(shmount_path, 0711); + if (ret < 0 && errno != EEXIST) { + fprintf(stderr, "Failed to create directory \"%s\"\n", shmount_path); + return false; + } + + /* Prepare host mountpoint */ + ret = mount("tmpfs", shmount_path, "tmpfs", 0, "size=100k,mode=0711"); + if (ret < 0) { + fprintf(stderr, "Failed to mount \"%s\"\n", shmount_path); + return false; + } + + ret = mount(shmount_path, shmount_path, "none", MS_REC | MS_SHARED, ""); + if (ret < 0) { + fprintf(stderr, "Failed to make shared \"%s\"\n", shmount_path); + return false; + } + + return true; +} + +static void lxc_teardown_shmount(char *shmount_path) +{ + (void)umount2(shmount_path, MNT_DETACH); + (void)recursive_destroy(shmount_path); +} + int main(int argc, char *argv[]) { + if (!lxc_setup_shmount("/tmp/mount_injection_test")) + exit(EXIT_FAILURE); + if (do_priv_container_test()) { fprintf(stderr, "Privileged mount injection test failed\n"); - return -1; + exit(EXIT_FAILURE); } - if(do_unpriv_container_test()) { + if (do_unpriv_container_test()) { fprintf(stderr, "Unprivileged mount injection test failed\n"); - return -1; + exit(EXIT_FAILURE); } - return 0; + + lxc_teardown_shmount("/tmp/mount_injection_test"); + + exit(EXIT_SUCCESS); }