From: 2xsec Date: Sat, 21 Jul 2018 07:04:01 +0000 (+0900) Subject: attach: fix return value & cleanups X-Git-Tag: lxc-3.1.0~198^2~1 X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=ea918412a796ba346c34c091030b6ce0c8221e2b;p=thirdparty%2Flxc.git attach: fix return value & cleanups Signed-off-by: 2xsec --- diff --git a/src/lxc/attach.c b/src/lxc/attach.c index 9bc3e23d1..993341961 100644 --- a/src/lxc/attach.c +++ b/src/lxc/attach.c @@ -108,18 +108,19 @@ static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid) proc_file = fopen(proc_fn, "r"); if (!proc_file) { - SYSERROR("Could not open %s.", proc_fn); + SYSERROR("Could not open %s", proc_fn); goto on_error; } info = calloc(1, sizeof(*info)); if (!info) { - SYSERROR("Could not allocate memory."); + SYSERROR("Could not allocate memory"); fclose(proc_file); return NULL; } found = false; + while (getline(&line, &line_bufsz, proc_file) != -1) { ret = sscanf(line, "CapBnd: %llx", &info->capability_mask); if (ret != EOF && ret == 1) { @@ -132,9 +133,8 @@ static struct lxc_proc_context_info *lxc_proc_get_context_info(pid_t pid) fclose(proc_file); if (!found) { - SYSERROR("Could not read capability bounding set from %s.", - proc_fn); - errno = ENOENT; + ERROR("Could not read capability bounding set from %s", + proc_fn); goto on_error; } @@ -156,6 +156,7 @@ static inline void lxc_proc_close_ns_fd(struct lxc_proc_context_info *ctx) for (i = 0; i < LXC_NS_MAX; i++) { if (ctx->ns_fd[i] < 0) continue; + close(ctx->ns_fd[i]); ctx->ns_fd[i] = -EBADF; } @@ -189,6 +190,7 @@ static void lxc_proc_put_context_info(struct lxc_proc_context_info *ctx) static int in_same_namespace(pid_t pid1, pid_t pid2, const char *ns) { int ns_fd1 = -1, ns_fd2 = -1, ret = -1; + int saved_errno = errno; struct stat ns_st1, ns_st2; ns_fd1 = lxc_preserve_ns(pid1, ns); @@ -199,52 +201,60 @@ static int in_same_namespace(pid_t pid1, pid_t pid2, const char *ns) if (errno == ENOENT) return -EINVAL; + saved_errno = errno; goto out; } ns_fd2 = lxc_preserve_ns(pid2, ns); - if (ns_fd2 < 0) + if (ns_fd2 < 0) { + saved_errno = errno; goto out; + } ret = fstat(ns_fd1, &ns_st1); - if (ret < 0) + if (ret < 0) { + saved_errno = errno; goto out; + } ret = fstat(ns_fd2, &ns_st2); - if (ret < 0) + if (ret < 0) { + saved_errno = errno; goto out; + } /* processes are in the same namespace */ - ret = -EINVAL; - if ((ns_st1.st_dev == ns_st2.st_dev ) && (ns_st1.st_ino == ns_st2.st_ino)) + if ((ns_st1.st_dev == ns_st2.st_dev ) && (ns_st1.st_ino == ns_st2.st_ino)) { + ret = -EINVAL; goto out; + } /* processes are in different namespaces */ ret = ns_fd2; ns_fd2 = -1; out: - if (ns_fd1 >= 0) close(ns_fd1); + if (ns_fd2 >= 0) close(ns_fd2); + errno = saved_errno; return ret; } static int lxc_attach_to_ns(pid_t pid, struct lxc_proc_context_info *ctx) { - int i, ret; + int i; for (i = 0; i < LXC_NS_MAX; i++) { if (ctx->ns_fd[i] < 0) continue; - ret = setns(ctx->ns_fd[i], ns_info[i].clone_flag); - if (ret < 0) { + if (setns(ctx->ns_fd[i], ns_info[i].clone_flag) < 0) { SYSERROR("Failed to attach to %s namespace of %d", - ns_info[i].proc_name, pid); + ns_info[i].proc_name, pid); return -1; } @@ -260,13 +270,13 @@ static int lxc_attach_remount_sys_proc(void) ret = unshare(CLONE_NEWNS); if (ret < 0) { - SYSERROR("Failed to unshare mount namespace."); + SYSERROR("Failed to unshare mount namespace"); return -1; } if (detect_shared_rootfs()) { if (mount(NULL, "/", NULL, MS_SLAVE | MS_REC, NULL)) { - SYSERROR("Failed to make / rslave."); + SYSERROR("Failed to make / rslave"); ERROR("Continuing..."); } } @@ -274,13 +284,13 @@ static int lxc_attach_remount_sys_proc(void) /* Assume /proc is always mounted, so remount it. */ ret = umount2("/proc", MNT_DETACH); if (ret < 0) { - SYSERROR("Failed to unmount /proc."); + SYSERROR("Failed to unmount /proc"); return -1; } ret = mount("none", "/proc", "proc", 0, NULL); if (ret < 0) { - SYSERROR("Failed to remount /proc."); + SYSERROR("Failed to remount /proc"); return -1; } @@ -289,13 +299,13 @@ static int lxc_attach_remount_sys_proc(void) */ ret = umount2("/sys", MNT_DETACH); if (ret < 0 && errno != EINVAL) { - SYSERROR("Failed to unmount /sys."); + SYSERROR("Failed to unmount /sys"); return -1; } else if (ret == 0) { /* Remount it. */ ret = mount("none", "/sys", "sysfs", 0, NULL); if (ret < 0) { - SYSERROR("Failed to remount /sys."); + SYSERROR("Failed to remount /sys"); return -1; } } @@ -316,6 +326,7 @@ static int lxc_attach_drop_privs(struct lxc_proc_context_info *ctx) SYSERROR("Failed to drop capability %d", cap); return -1; } + TRACE("Dropped capability %d", cap); } @@ -350,6 +361,7 @@ static int lxc_attach_set_environment(struct lxc_proc_context_info *init_ctx, if (!extra_keep_store[i]) { while (i > 0) free(extra_keep_store[--i]); + free(extra_keep_store); return -1; } @@ -370,7 +382,7 @@ static int lxc_attach_set_environment(struct lxc_proc_context_info *init_ctx, free(extra_keep_store); } - SYSERROR("Failed to clear environment"); + ERROR("Failed to clear environment"); return -1; } @@ -383,8 +395,10 @@ static int lxc_attach_set_environment(struct lxc_proc_context_info *init_ctx, if (ret < 0) SYSWARN("Failed to set environment variable"); } + free(extra_keep_store[i]); } + free(extra_keep_store); } @@ -427,6 +441,7 @@ static int lxc_attach_set_environment(struct lxc_proc_context_info *init_ctx, if (extra_env) { for (; *extra_env; extra_env++) { char *p; + /* We just assume the user knows what they are doing, so * we don't do any checks. */ @@ -482,7 +497,7 @@ static char *lxc_attach_getpwshell(uid_t uid) ret = dup2(pipes[1], STDOUT_FILENO); close(pipes[1]); if (ret < 0) - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); /* Get rid of stdin/stderr, so we try to associate it with * /dev/null. @@ -500,11 +515,11 @@ static char *lxc_attach_getpwshell(uid_t uid) /* Finish argument list. */ ret = snprintf(uid_buf, sizeof(uid_buf), "%ld", (long)uid); if (ret <= 0 || ret >= sizeof(uid_buf)) - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); /* Try to run getent program. */ (void)execvp("getent", arguments); - exit(EXIT_FAILURE); + _exit(EXIT_FAILURE); } close(pipes[1]); @@ -541,7 +556,7 @@ static char *lxc_attach_getpwshell(uid_t uid) token = strtok_r(NULL, ":", &saveptr); value = token ? strtol(token, &endptr, 10) : 0; if (!token || !endptr || *endptr || value == LONG_MIN || - value == LONG_MAX) + value == LONG_MAX) continue; /* dummy sanity check: user id matches */ @@ -554,8 +569,10 @@ static char *lxc_attach_getpwshell(uid_t uid) if (!token) continue; } + if (!token) continue; + free(result); result = strdup(token); @@ -566,6 +583,7 @@ static char *lxc_attach_getpwshell(uid_t uid) found = true; } + free(line); fclose(pipe_f); @@ -614,6 +632,7 @@ static void lxc_attach_get_init_uidgid(uid_t *init_uid, gid_t *init_gid) if (ret != EOF && ret == 1) gid = (gid_t)value; } + if (uid != (uid_t)-1 && gid != (gid_t)-1) break; } @@ -624,6 +643,7 @@ static void lxc_attach_get_init_uidgid(uid_t *init_uid, gid_t *init_gid) /* Only override arguments if we found something. */ if (uid != (uid_t)-1) *init_uid = uid; + if (gid != (gid_t)-1) *init_gid = gid; @@ -658,14 +678,14 @@ static bool fetch_seccomp(struct lxc_container *c, lxc_attach_options_t *options /* Remove current setting. */ if (!c->set_config_item(c, "lxc.seccomp.profile", "") && - !c->set_config_item(c, "lxc.seccomp", "")) { + !c->set_config_item(c, "lxc.seccomp", "")) return false; - } /* Fetch the current profile path over the cmd interface. */ path = c->get_running_config_item(c, "lxc.seccomp.profile"); if (!path) { INFO("Failed to retrieve lxc.seccomp.profile"); + path = c->get_running_config_item(c, "lxc.seccomp"); if (!path) { INFO("Failed to retrieve lxc.seccomp"); @@ -778,6 +798,7 @@ static int attach_child_main(struct attach_clone_payload *payload) ret = lxc_attach_remount_sys_proc(); if (ret < 0) goto on_error; + TRACE("Remounted \"/proc\" and \"/sys\""); } @@ -790,9 +811,11 @@ static int attach_child_main(struct attach_clone_payload *payload) new_personality = init_ctx->personality; else new_personality = options->personality; + ret = personality(new_personality); if (ret < 0) goto on_error; + TRACE("Set new personality"); } #endif @@ -801,6 +824,7 @@ static int attach_child_main(struct attach_clone_payload *payload) ret = lxc_attach_drop_privs(init_ctx); if (ret < 0) goto on_error; + TRACE("Dropped capabilities"); } @@ -813,6 +837,7 @@ static int attach_child_main(struct attach_clone_payload *payload) options->extra_keep_env); if (ret < 0) goto on_error; + TRACE("Set up environment"); /* This remark only affects fully unprivileged containers: @@ -847,6 +872,7 @@ static int attach_child_main(struct attach_clone_payload *payload) /* Set {u,g}id. */ new_uid = 0; new_gid = 0; + /* Ignore errors, we will fall back to root in that case (/proc was not * mounted etc.). */ @@ -855,6 +881,7 @@ static int attach_child_main(struct attach_clone_payload *payload) if (options->uid != (uid_t)-1) new_uid = options->uid; + if (options->gid != (gid_t)-1) new_gid = options->gid; @@ -875,6 +902,7 @@ static int attach_child_main(struct attach_clone_payload *payload) ret = prctl(PR_SET_NO_NEW_PRIVS, 1, 0, 0, 0); if (ret < 0) goto on_error; + TRACE("Set PR_SET_NO_NEW_PRIVS"); } @@ -883,10 +911,12 @@ static int attach_child_main(struct attach_clone_payload *payload) /* Change into our new LSM profile. */ on_exec = options->attach_flags & LXC_ATTACH_LSM_EXEC ? true : false; + ret = lsm_process_label_set_at(lsm_fd, init_ctx->lsm_label, on_exec); close(lsm_fd); if (ret < 0) goto on_error; + TRACE("Set %s LSM label to \"%s\"", lsm_name(), init_ctx->lsm_label); } @@ -895,8 +925,10 @@ static int attach_child_main(struct attach_clone_payload *payload) ret = lxc_seccomp_load(init_ctx->container->lxc_conf); if (ret < 0) goto on_error; + TRACE("Loaded seccomp profile"); } + shutdown(payload->ipc_socket, SHUT_RDWR); close(payload->ipc_socket); payload->ipc_socket = -EBADF; @@ -948,6 +980,7 @@ static int attach_child_main(struct attach_clone_payload *payload) SYSERROR("Failed to prepare terminal file descriptor %d", payload->terminal_slave_fd); goto on_error; } + TRACE("Prepared terminal file descriptor %d", payload->terminal_slave_fd); } @@ -968,7 +1001,7 @@ static int lxc_attach_terminal(struct lxc_conf *conf, ret = lxc_terminal_create(terminal); if (ret < 0) { - SYSERROR("Failed to create terminal"); + ERROR("Failed to create terminal"); return -1; } @@ -1060,7 +1093,7 @@ int lxc_attach(const char *name, const char *lxcpath, ret = access("/proc/self/ns", X_OK); if (ret) { - ERROR("Does this kernel version support namespaces?"); + SYSERROR("Does this kernel version support namespaces?"); return -1; } @@ -1097,7 +1130,7 @@ int lxc_attach(const char *name, const char *lxcpath, init_ctx->container->lxc_conf = lxc_conf_init(); if (!init_ctx->container->lxc_conf) { lxc_proc_put_context_info(init_ctx); - return -ENOMEM; + return -1; } } conf = init_ctx->container->lxc_conf; @@ -1138,8 +1171,9 @@ int lxc_attach(const char *name, const char *lxcpath, } pid = lxc_raw_getpid(); + for (i = 0; i < LXC_NS_MAX; i++) { - int j, saved_errno; + int j; if (options->namespaces & ns_info[i].clone_flag) init_ctx->ns_fd[i] = lxc_preserve_ns(init_pid, ns_info[i].proc_name); @@ -1147,6 +1181,7 @@ int lxc_attach(const char *name, const char *lxcpath, init_ctx->ns_fd[i] = in_same_namespace(pid, init_pid, ns_info[i].proc_name); else continue; + if (init_ctx->ns_fd[i] >= 0) continue; @@ -1158,16 +1193,15 @@ int lxc_attach(const char *name, const char *lxcpath, } /* We failed to preserve the namespace. */ - saved_errno = errno; + SYSERROR("Failed to attach to %s namespace of %d", + ns_info[i].proc_name, pid); + /* Close all already opened file descriptors before we return an * error, so we don't leak them. */ for (j = 0; j < i; j++) close(init_ctx->ns_fd[j]); - errno = saved_errno; - SYSERROR("Failed to attach to %s namespace of %d", - ns_info[i].proc_name, pid); free(cwd); lxc_proc_put_context_info(init_ctx); return -1; @@ -1288,6 +1322,7 @@ int lxc_attach(const char *name, const char *lxcpath, ret = lxc_attach_terminal_mainloop_init(&terminal, &descr); if (ret < 0) goto on_error; + TRACE("Initialized terminal mainloop"); } @@ -1296,12 +1331,14 @@ int lxc_attach(const char *name, const char *lxcpath, ret = lxc_write_nointr(ipc_sockets[0], &status, sizeof(status)); if (ret != sizeof(status)) goto close_mainloop; + TRACE("Told intermediate process to start initializing"); /* Get pid of attached process from intermediate process. */ ret = lxc_read_nointr(ipc_sockets[0], &attached_pid, sizeof(attached_pid)); if (ret != sizeof(attached_pid)) goto close_mainloop; + TRACE("Received pid %d of attached process in parent pid namespace", attached_pid); /* Ignore SIGKILL (CTRL-C) and SIGQUIT (CTRL-\) - issue #313. */ @@ -1314,6 +1351,7 @@ int lxc_attach(const char *name, const char *lxcpath, ret = wait_for_pid(pid); if (ret < 0) goto close_mainloop; + TRACE("Intermediate process %d exited", pid); /* We will always have to reap the attached process now. */ @@ -1331,6 +1369,7 @@ int lxc_attach(const char *name, const char *lxcpath, labelfd = lsm_process_label_fd_get(attached_pid, on_exec); if (labelfd < 0) goto close_mainloop; + TRACE("Opened LSM label file descriptor %d", labelfd); /* Send child fd of the LSM security module to write to. */ @@ -1361,6 +1400,7 @@ int lxc_attach(const char *name, const char *lxcpath, ret_parent = 0; to_cleanup_pid = -1; + if (options->attach_flags & LXC_ATTACH_TERMINAL) { ret = lxc_mainloop(&descr, -1); if (ret < 0) { @@ -1386,6 +1426,7 @@ int lxc_attach(const char *name, const char *lxcpath, lxc_terminal_delete(&terminal); lxc_terminal_conf_free(&terminal); } + lxc_proc_put_context_info(init_ctx); return ret_parent; } @@ -1393,6 +1434,7 @@ int lxc_attach(const char *name, const char *lxcpath, /* close unneeded file descriptors */ close(ipc_sockets[0]); ipc_sockets[0] = -EBADF; + if (options->attach_flags & LXC_ATTACH_TERMINAL) { lxc_attach_terminal_close_master(&terminal); lxc_attach_terminal_close_peer(&terminal); @@ -1406,6 +1448,7 @@ int lxc_attach(const char *name, const char *lxcpath, lxc_proc_put_context_info(init_ctx); rexit(-1); } + TRACE("Intermediate process starting to initialize"); /* Attach now, create another subprocess later, since pid namespaces @@ -1418,6 +1461,7 @@ int lxc_attach(const char *name, const char *lxcpath, lxc_proc_put_context_info(init_ctx); rexit(-1); } + /* close namespace file descriptors */ lxc_proc_close_ns_fd(init_ctx); @@ -1453,8 +1497,10 @@ int lxc_attach(const char *name, const char *lxcpath, ret = attach_child_main(&payload); if (ret < 0) ERROR("Failed to exec"); + _exit(EXIT_FAILURE); } + if (options->attach_flags & LXC_ATTACH_TERMINAL) lxc_attach_terminal_close_slave(&terminal); @@ -1471,6 +1517,7 @@ int lxc_attach(const char *name, const char *lxcpath, lxc_proc_put_context_info(init_ctx); rexit(-1); } + TRACE("Sending pid %d of attached process", pid); /* The rest is in the hands of the initial and the attached process. */ @@ -1483,7 +1530,8 @@ int lxc_attach_run_command(void* payload) lxc_attach_command_t* cmd = (lxc_attach_command_t*)payload; execvp(cmd->program, cmd->argv); - SYSERROR("Failed to exec \"%s\".", cmd->program); + + SYSERROR("Failed to exec \"%s\"", cmd->program); return -1; } @@ -1511,7 +1559,7 @@ int lxc_attach_run_shell(void* payload) ret = getpwuid_r(uid, &pwent, buf, bufsize, &pwentp); if (!pwentp) { if (ret == 0) - WARN("Could not find matched password record."); + WARN("Could not find matched password record"); WARN("Failed to get password record - %u", uid); } @@ -1527,6 +1575,7 @@ int lxc_attach_run_shell(void* payload) user_shell = lxc_attach_getpwshell(uid); else user_shell = pwent.pw_shell; + if (user_shell) execlp(user_shell, user_shell, (char *)NULL); @@ -1534,9 +1583,11 @@ int lxc_attach_run_shell(void* payload) * on /bin/sh as a default shell. */ execlp("/bin/sh", "/bin/sh", (char *)NULL); + SYSERROR("Failed to execute shell"); if (!pwentp) free(user_shell); + free(buf); return -1; } diff --git a/src/lxc/start.c b/src/lxc/start.c index a970d3c15..1d22a6a59 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -868,12 +868,16 @@ int lxc_init(const char *name, struct lxc_handler *handler) out_delete_terminal: lxc_terminal_delete(&handler->conf->console); + out_restore_sigmask: (void)pthread_sigmask(SIG_SETMASK, &handler->oldmask, NULL); + out_delete_tty: lxc_delete_tty(&conf->ttys); + out_aborting: (void)lxc_set_state(name, handler, ABORTING); + out_close_maincmd_fd: lxc_abstract_unix_close(conf->maincmd_fd); conf->maincmd_fd = -1; diff --git a/src/lxc/utils.c b/src/lxc/utils.c index 6bb05df00..dd6cdc91a 100644 --- a/src/lxc/utils.c +++ b/src/lxc/utils.c @@ -2038,9 +2038,10 @@ int lxc_preserve_ns(const int pid, const char *ns) ret = snprintf(path, __NS_PATH_LEN, "/proc/%d/ns%s%s", pid, !ns || strcmp(ns, "") == 0 ? "" : "/", !ns || strcmp(ns, "") == 0 ? "" : ns); - errno = EFBIG; - if (ret < 0 || (size_t)ret >= __NS_PATH_LEN) - return -EFBIG; + if (ret < 0 || (size_t)ret >= __NS_PATH_LEN) { + errno = EFBIG; + return -1; + } return open(path, O_RDONLY | O_CLOEXEC); }