diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 050b397df8..3a9c20011b 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -1512,16 +1512,30 @@ struct generic_userns_exec_data { static int cgroup_rmdir_wrapper(void *data) { + int ret; struct generic_userns_exec_data *arg = data; uid_t nsuid = (arg->conf->root_nsuid_map != NULL) ? 0 : arg->conf->init_uid; gid_t nsgid = (arg->conf->root_nsgid_map != NULL) ? 0 : arg->conf->init_gid; - if (setresgid(nsgid, nsgid, nsgid) < 0) - SYSERROR("Failed to setgid to 0"); - if (setresuid(nsuid, nsuid, nsuid) < 0) - SYSERROR("Failed to setuid to 0"); - if (setgroups(0, NULL) < 0 && errno != EPERM) - SYSERROR("Failed to clear groups"); + ret = setresgid(nsgid, nsgid, nsgid); + if (ret < 0) { + SYSERROR("Failed to setresgid(%d, %d, %d)", (int)nsgid, + (int)nsgid, (int)nsgid); + return -1; + } + + ret = setresuid(nsuid, nsuid, nsuid); + if (ret < 0) { + SYSERROR("Failed to setresuid(%d, %d, %d)", (int)nsuid, + (int)nsuid, (int)nsuid); + return -1; + } + + ret = setgroups(0, NULL); + if (ret < 0 && errno != EPERM) { + SYSERROR("Failed to setgroups(0, NULL)"); + return -1; + } return cgroup_rmdir(arg->d->container_cgroup); } @@ -1756,8 +1770,29 @@ static bool cgfsng_enter(void *hdata, pid_t pid) return true; } -/* - * chgrp the container cgroups to container group. We leave +static int chowmod(char *path, uid_t chown_uid, gid_t chown_gid, + mode_t chmod_mode) +{ + int ret; + + ret = chown(path, chown_uid, chown_gid); + if (ret < 0) { + WARN("%s - Failed to chown(%s, %d, %d)", strerror(errno), path, + (int)chown_uid, (int)chown_gid); + return -1; + } + + ret = chmod(path, chmod_mode); + if (ret < 0) { + WARN("%s - Failed to chmod(%s, %d)", strerror(errno), path, + (int)chmod_mode); + return -1; + } + + return 0; +} + +/* chgrp the container cgroups to container group. We leave * the container owner as cgroup owner. So we must make the * directories 775 so that the container can create sub-cgroups. * @@ -1766,74 +1801,68 @@ static bool cgfsng_enter(void *hdata, pid_t pid) */ static int chown_cgroup_wrapper(void *data) { - int i; + int i, ret; uid_t destuid; struct generic_userns_exec_data *arg = data; uid_t nsuid = (arg->conf->root_nsuid_map != NULL) ? 0 : arg->conf->init_uid; gid_t nsgid = (arg->conf->root_nsgid_map != NULL) ? 0 : arg->conf->init_gid; - if (setresgid(nsgid, nsgid, nsgid) < 0) - SYSERROR("Failed to setgid to 0"); - if (setresuid(nsuid, nsuid, nsuid) < 0) - SYSERROR("Failed to setuid to 0"); - if (setgroups(0, NULL) < 0 && errno != EPERM) - SYSERROR("Failed to clear groups"); + ret = setresgid(nsgid, nsgid, nsgid); + if (ret < 0) { + SYSERROR("Failed to setresgid(%d, %d, %d)", + (int)nsgid, (int)nsgid, (int)nsgid); + return -1; + } + + ret = setresuid(nsuid, nsuid, nsuid); + if (ret < 0) { + SYSERROR("Failed to setresuid(%d, %d, %d)", + (int)nsuid, (int)nsuid, (int)nsuid); + return -1; + } + + ret = setgroups(0, NULL); + if (ret < 0 && errno != EPERM) { + SYSERROR("Failed to setgroups(0, NULL)"); + return -1; + } destuid = get_ns_uid(arg->origuid); for (i = 0; hierarchies[i]; i++) { - char *fullpath, *path = hierarchies[i]->fullcgpath; + char *fullpath; + char *path = hierarchies[i]->fullcgpath; - if (chown(path, destuid, nsgid) < 0) { - SYSERROR("Error chowning %s to %d", path, (int) destuid); + ret = chowmod(path, destuid, nsgid, 0755); + if (ret < 0) return -1; - } - if (chmod(path, 0775) < 0) { - SYSERROR("Error chmoding %s", path); - return -1; - } - - /* - * Failures to chown these are inconvenient but not detrimental - * We leave these owned by the container launcher, so that container - * root can write to the files to attach. We chmod them 664 so that - * container systemd can write to the files (which systemd in wily - * insists on doing) + /* Failures to chown() these are inconvenient but not + * detrimental We leave these owned by the container launcher, + * so that container root can write to the files to attach. We + * chmod() them 664 so that container systemd can write to the + * files (which systemd in wily insists on doing). */ - fullpath = must_make_path(path, "tasks", NULL); - if (chown(fullpath, destuid, nsgid) < 0 && errno != ENOENT) - WARN("Failed chowning %s to %d: %s", fullpath, (int) destuid, - strerror(errno)); - if (chmod(fullpath, 0664) < 0) - WARN("Error chmoding %s: %s", path, strerror(errno)); - free(fullpath); + + if (hierarchies[i]->version == CGROUP_SUPER_MAGIC) { + fullpath = must_make_path(path, "tasks", NULL); + (void)chowmod(fullpath, destuid, nsgid, 0664); + free(fullpath); + } fullpath = must_make_path(path, "cgroup.procs", NULL); - if (chown(fullpath, destuid, 0) < 0 && errno != ENOENT) - WARN("Failed chowning %s to %d: %s", fullpath, (int) destuid, - strerror(errno)); - if (chmod(fullpath, 0664) < 0) - WARN("Error chmoding %s: %s", path, strerror(errno)); + (void)chowmod(fullpath, destuid, 0, 0664); free(fullpath); if (hierarchies[i]->version != CGROUP2_SUPER_MAGIC) continue; fullpath = must_make_path(path, "cgroup.subtree_control", NULL); - if (chown(fullpath, destuid, nsgid) < 0 && errno != ENOENT) - WARN("Failed chowning %s to %d: %s", fullpath, (int) destuid, - strerror(errno)); - if (chmod(fullpath, 0664) < 0) - WARN("Error chmoding %s: %s", path, strerror(errno)); + (void)chowmod(fullpath, destuid, nsgid, 0664); free(fullpath); fullpath = must_make_path(path, "cgroup.threads", NULL); - if (chown(fullpath, destuid, nsgid) < 0 && errno != ENOENT) - WARN("Failed chowning %s to %d: %s", fullpath, (int) destuid, - strerror(errno)); - if (chmod(fullpath, 0664) < 0) - WARN("Error chmoding %s: %s", path, strerror(errno)); + (void)chowmod(fullpath, destuid, nsgid, 0664); free(fullpath); } diff --git a/src/lxc/start.c b/src/lxc/start.c index f9befb827e..b90d5ea00a 100644 --- a/src/lxc/start.c +++ b/src/lxc/start.c @@ -1559,7 +1559,7 @@ static int lxc_spawn(struct lxc_handler *handler) * again. */ if (wants_to_map_ids) { - if (!handler->conf->ns_share[LXC_NS_USER] || + if (!handler->conf->ns_share[LXC_NS_USER] && (handler->conf->ns_keep & CLONE_NEWUSER) == 0) { ret = lxc_map_ids(id_map, handler->pid); if (ret < 0) {