The following pull request was submitted through Github. It can be accessed and reviewed at: https://github.com/lxc/lxc/pull/2311
This e-mail was sent by the LXC bot, direct replies will not reach the author unless they happen to be subscribed to this list. === Description (from pull-request) === Instead of bind mounting in the init and execing it, let's just use fexecve. There are at least two reasons to prefer this: * We don't pollute the users rootfs (even if only temporarily). Even if we remove the file, it still changes the mtime of /tmp (or wherever we would bind it), which users who are sensitive to any changes in the rootfs will see. * It allows us to get rid of a whole bunch of code. Signed-off-by: Tycho Andersen <[email protected]>
From 0077e205dc6a774893c489cfb5c96ee9af275e34 Mon Sep 17 00:00:00 2001 From: Tycho Andersen <[email protected]> Date: Wed, 9 May 2018 15:47:00 +0000 Subject: [PATCH] execute: don't bind in lxc.init.static Instead of bind mounting in the init and execing it, let's just use fexecve. There are at least two reasons to prefer this: * We don't pollute the users rootfs (even if only temporarily). Even if we remove the file, it still changes the mtime of /tmp (or wherever we would bind it), which users who are sensitive to any changes in the rootfs will see. * It allows us to get rid of a whole bunch of code. Signed-off-by: Tycho Andersen <[email protected]> --- src/lxc/cmd/lxc_init.c | 28 ---------------- src/lxc/conf.c | 75 ++++++++--------------------------------- src/lxc/execute.c | 9 ++--- src/lxc/start.h | 2 +- src/lxc/utils.c | 90 -------------------------------------------------- src/lxc/utils.h | 1 - 6 files changed, 17 insertions(+), 188 deletions(-) diff --git a/src/lxc/cmd/lxc_init.c b/src/lxc/cmd/lxc_init.c index 228a2eb8b..a16745dcd 100644 --- a/src/lxc/cmd/lxc_init.c +++ b/src/lxc/cmd/lxc_init.c @@ -192,32 +192,6 @@ static void kill_children(pid_t pid) fclose(f); } -static void remove_self(void) -{ - int ret; - ssize_t n; - char path[MAXPATHLEN] = {0}; - - n = readlink("/proc/self/exe", path, sizeof(path)); - if (n < 0 || n >= MAXPATHLEN) { - SYSERROR("Failed to readlink \"/proc/self/exe\""); - return; - } - path[n] = '\0'; - - ret = umount2(path, MNT_DETACH); - if (ret < 0) { - SYSERROR("Failed to unmount \"%s\"", path); - return; - } - - ret = unlink(path); - if (ret < 0) { - SYSERROR("Failed to unlink \"%s\"", path); - return; - } -} - int main(int argc, char *argv[]) { int i, ret; @@ -317,8 +291,6 @@ int main(int argc, char *argv[]) } } - remove_self(); - pid = fork(); if (pid < 0) exit(EXIT_FAILURE); diff --git a/src/lxc/conf.c b/src/lxc/conf.c index 253331160..5c6483cc7 100644 --- a/src/lxc/conf.c +++ b/src/lxc/conf.c @@ -3203,64 +3203,6 @@ void remount_all_slave(void) free(line); } -static int lxc_execute_bind_init(struct lxc_handler *handler) -{ - int ret; - char *p; - char path[PATH_MAX], destpath[PATH_MAX]; - struct lxc_conf *conf = handler->conf; - - /* If init exists in the container, don't bind mount a static one */ - p = choose_init(conf->rootfs.mount); - if (p) { - char *old = p; - - p = strdup(old + strlen(conf->rootfs.mount)); - free(old); - if (!p) - return -ENOMEM; - - INFO("Found existing init at \"%s\"", p); - goto out; - } - - ret = snprintf(path, PATH_MAX, SBINDIR "/init.lxc.static"); - if (ret < 0 || ret >= PATH_MAX) - return -1; - - if (!file_exists(path)) { - ERROR("The file \"%s\" does not exist on host", path); - return -1; - } - - ret = snprintf(destpath, PATH_MAX, "%s" P_tmpdir "%s", conf->rootfs.mount, "/.lxc-init"); - if (ret < 0 || ret >= PATH_MAX) - return -1; - - if (!file_exists(destpath)) { - ret = mknod(destpath, S_IFREG | 0000, 0); - if (ret < 0 && errno != EEXIST) { - SYSERROR("Failed to create dummy \"%s\" file as bind mount target", destpath); - return -1; - } - } - - ret = safe_mount(path, destpath, "none", MS_BIND, NULL, conf->rootfs.mount); - if (ret < 0) { - SYSERROR("Failed to bind mount lxc.init.static into container"); - return -1; - } - - p = strdup(destpath + strlen(conf->rootfs.mount)); - if (!p) - return -ENOMEM; - - INFO("Bind mounted lxc.init.static into container at \"%s\"", path); -out: - ((struct execute_args *)handler->data)->init_path = p; - return 0; -} - /* This does the work of remounting / if it is shared, calling the container * pre-mount hooks, and mounting the rootfs. */ @@ -3391,11 +3333,22 @@ int lxc_setup(struct lxc_handler *handler) return -1; if (lxc_conf->is_execute) { - ret = lxc_execute_bind_init(handler); - if (ret < 0) { - ERROR("Failed to bind-mount the lxc init system"); + int fd; + char path[PATH_MAX]; + + ret = snprintf(path, PATH_MAX, SBINDIR "/init.lxc.static"); + if (ret < 0 || ret >= PATH_MAX) { + ERROR("Path to init.lxc.static too long"); return -1; } + + fd = open(path, O_PATH | O_CLOEXEC); + if (fd < 0) { + SYSERROR("Unable to open lxc.init.static"); + return -1; + } + + ((struct execute_args *)handler->data)->init_fd = fd; } /* Now mount only cgroups, if wanted. Before, /sys could not have been diff --git a/src/lxc/execute.c b/src/lxc/execute.c index b436b6a3f..2da338028 100644 --- a/src/lxc/execute.c +++ b/src/lxc/execute.c @@ -66,12 +66,7 @@ static int execute_start(struct lxc_handler *handler, void* data) goto out1; } - if (!my_args->init_path) { - ERROR("Init path missing"); - goto out2; - } - - argv[i++] = my_args->init_path; + argv[i++] = "lxc-init"; argv[i++] = "-n"; argv[i++] = (char *)handler->name; @@ -117,7 +112,7 @@ static int execute_start(struct lxc_handler *handler, void* data) NOTICE("Exec'ing \"%s\"", my_args->argv[0]); - execvp(argv[0], argv); + fexecve(my_args->init_fd, argv, environ); SYSERROR("Failed to exec %s", argv[0]); out3: diff --git a/src/lxc/start.h b/src/lxc/start.h index 5455ca5f3..c83b91e2b 100644 --- a/src/lxc/start.h +++ b/src/lxc/start.h @@ -135,7 +135,7 @@ struct lxc_handler { }; struct execute_args { - char *init_path; + int init_fd; char *const *argv; int quiet; }; diff --git a/src/lxc/utils.c b/src/lxc/utils.c index 3d0f8641c..c1910edeb 100644 --- a/src/lxc/utils.c +++ b/src/lxc/utils.c @@ -1290,96 +1290,6 @@ bool cgns_supported(void) return file_exists("/proc/self/ns/cgroup"); } -/* historically lxc-init has been under /usr/lib/lxc and under - * /usr/lib/$ARCH/lxc. It now lives as $prefix/sbin/init.lxc. - */ -char *choose_init(const char *rootfs) -{ - char *retv = NULL; - const char *empty = "", - *tmp; - int ret, env_set = 0; - - if (!getenv("PATH")) { - if (setenv("PATH", "/usr/local/sbin:/usr/local/bin:/usr/sbin:/usr/bin:/sbin:/bin", 0)) - SYSERROR("Failed to setenv"); - env_set = 1; - } - - retv = on_path("init.lxc", rootfs); - - if (env_set) { - if (unsetenv("PATH")) - SYSERROR("Failed to unsetenv"); - } - - if (retv) - return retv; - - retv = malloc(PATH_MAX); - if (!retv) - return NULL; - - if (rootfs) - tmp = rootfs; - else - tmp = empty; - - ret = snprintf(retv, PATH_MAX, "%s/%s/%s", tmp, SBINDIR, "/init.lxc"); - if (ret < 0 || ret >= PATH_MAX) { - ERROR("pathname too long"); - goto out1; - } - if (access(retv, X_OK) == 0) - return retv; - - ret = snprintf(retv, PATH_MAX, "%s/%s/%s", tmp, LXCINITDIR, "/lxc/lxc-init"); - if (ret < 0 || ret >= PATH_MAX) { - ERROR("pathname too long"); - goto out1; - } - if (access(retv, X_OK) == 0) - return retv; - - ret = snprintf(retv, PATH_MAX, "%s/usr/lib/lxc/lxc-init", tmp); - if (ret < 0 || ret >= PATH_MAX) { - ERROR("pathname too long"); - goto out1; - } - if (access(retv, X_OK) == 0) - return retv; - - ret = snprintf(retv, PATH_MAX, "%s/sbin/lxc-init", tmp); - if (ret < 0 || ret >= PATH_MAX) { - ERROR("pathname too long"); - goto out1; - } - if (access(retv, X_OK) == 0) - return retv; - - /* - * Last resort, look for the statically compiled init.lxc which we - * hopefully bind-mounted in. - * If we are called during container setup, and we get to this point, - * then the init.lxc.static from the host will need to be bind-mounted - * in. So we return NULL here to indicate that. - */ - if (rootfs) - goto out1; - - ret = snprintf(retv, PATH_MAX, "/init.lxc.static"); - if (ret < 0 || ret >= PATH_MAX) { - WARN("Nonsense - name /lxc.init.static too long"); - goto out1; - } - if (access(retv, X_OK) == 0) - return retv; - -out1: - free(retv); - return NULL; -} - int print_to_file(const char *file, const char *content) { FILE *f; diff --git a/src/lxc/utils.h b/src/lxc/utils.h index 62f087311..061c10255 100644 --- a/src/lxc/utils.h +++ b/src/lxc/utils.h @@ -506,7 +506,6 @@ extern bool detect_ramfs_rootfs(void); extern char *on_path(const char *cmd, const char *rootfs); extern bool file_exists(const char *f); extern bool cgns_supported(void); -extern char *choose_init(const char *rootfs); extern int print_to_file(const char *file, const char *content); extern bool switch_to_ns(pid_t pid, const char *ns); extern int is_dir(const char *path);
_______________________________________________ lxc-devel mailing list [email protected] http://lists.linuxcontainers.org/listinfo/lxc-devel
