From 8e64b6736fea25fe7492b4b7c3aa2d00e9e62701 Mon Sep 17 00:00:00 2001 From: Christian Brauner Date: Tue, 10 Dec 2019 21:00:59 +0100 Subject: [PATCH] cgroups/cgfsng: rework cgroup removal Signed-off-by: Christian Brauner --- src/lxc/cgroups/cgfsng.c | 95 ++++++++++++---------------------------- src/lxc/cgroups/cgroup.h | 1 - src/lxc/file_utils.c | 1 + src/lxc/utils.c | 46 ++++++------------- src/lxc/utils.h | 2 +- 5 files changed, 43 insertions(+), 102 deletions(-) diff --git a/src/lxc/cgroups/cgfsng.c b/src/lxc/cgroups/cgfsng.c index 7ddde0252..b5f64cc77 100644 --- a/src/lxc/cgroups/cgfsng.c +++ b/src/lxc/cgroups/cgfsng.c @@ -382,7 +382,6 @@ static bool cg_legacy_filter_and_set_cpus(const char *parent_cgroup, ssize_t maxisol = 0, maxoffline = 0, maxposs = 0; bool flipped_bit = false; - SYSERROR("AAAA: %s | %s", parent_cgroup, child_cgroup); fpath = must_make_path(parent_cgroup, "cpuset.cpus", NULL); posscpus = read_file(fpath); if (!posscpus) @@ -998,12 +997,10 @@ static void lxc_cgfsng_print_basecg_debuginfo(char *basecginfo, char **klist, static int cgroup_rmdir(struct hierarchy **hierarchies, const char *container_cgroup) { - int i; - if (!container_cgroup || !hierarchies) return 0; - for (i = 0; hierarchies[i]; i++) { + for (int i = 0; hierarchies[i]; i++) { int ret; struct hierarchy *h = hierarchies[i]; @@ -1031,30 +1028,26 @@ 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; + int ret; ret = setresgid(nsgid, nsgid, nsgid); - if (ret < 0) { - SYSERROR("Failed to setresgid(%d, %d, %d)", (int)nsgid, - (int)nsgid, (int)nsgid); - return -1; - } + if (ret < 0) + return log_error_errno(-1, errno, + "Failed to setresgid(%d, %d, %d)", + (int)nsgid, (int)nsgid, (int)nsgid); ret = setresuid(nsuid, nsuid, nsuid); - if (ret < 0) { - SYSERROR("Failed to setresuid(%d, %d, %d)", (int)nsuid, - (int)nsuid, (int)nsuid); - return -1; - } + if (ret < 0) + return log_error_errno(-1, errno, + "Failed to setresuid(%d, %d, %d)", + (int)nsuid, (int)nsuid, (int)nsuid); ret = setgroups(0, NULL); - if (ret < 0 && errno != EPERM) { - SYSERROR("Failed to setgroups(0, NULL)"); - return -1; - } + if (ret < 0 && errno != EPERM) + return log_error_errno(-1, errno, "Failed to setgroups(0, NULL)"); return cgroup_rmdir(arg->hierarchies, arg->container_cgroup); } @@ -1063,7 +1056,6 @@ __cgfsng_ops static void cgfsng_payload_destroy(struct cgroup_ops *ops, struct lxc_handler *handler) { int ret; - struct generic_userns_exec_data wrap; if (!ops) log_error_errno(return, ENOENT, "Called with uninitialized cgroup operations"); @@ -1077,26 +1069,26 @@ __cgfsng_ops static void cgfsng_payload_destroy(struct cgroup_ops *ops, if (!handler->conf) log_error_errno(return, EINVAL, "Called with uninitialized conf"); - wrap.origuid = 0; - wrap.container_cgroup = ops->container_cgroup; - wrap.hierarchies = ops->hierarchies; - wrap.conf = handler->conf; - #ifdef HAVE_STRUCT_BPF_CGROUP_DEV_CTX ret = bpf_program_cgroup_detach(handler->conf->cgroup2_devices); if (ret < 0) WARN("Failed to detach bpf program from cgroup"); #endif - if (handler->conf && !lxc_list_empty(&handler->conf->id_map)) + if (handler->conf && !lxc_list_empty(&handler->conf->id_map)) { + struct generic_userns_exec_data wrap = { + .origuid = 0, + .container_cgroup = ops->container_cgroup, + .hierarchies = ops->hierarchies, + .conf = handler->conf, + }; ret = userns_exec_1(handler->conf, cgroup_rmdir_wrapper, &wrap, "cgroup_rmdir_wrapper"); - else + } else { ret = cgroup_rmdir(ops->hierarchies, ops->container_cgroup); - if (ret < 0) { - WARN("Failed to destroy cgroups"); - return; } + if (ret < 0) + log_warn_errno(return, errno, "Failed to destroy cgroups"); } __cgfsng_ops static void cgfsng_monitor_destroy(struct cgroup_ops *ops, @@ -1104,7 +1096,6 @@ __cgfsng_ops static void cgfsng_monitor_destroy(struct cgroup_ops *ops, { int len; char pidstr[INTTYPE_TO_STRLEN(pid_t)]; - struct lxc_conf *conf; if (!ops) log_error_errno(return, ENOENT, "Called with uninitialized cgroup operations"); @@ -1115,56 +1106,24 @@ __cgfsng_ops static void cgfsng_monitor_destroy(struct cgroup_ops *ops, if (!handler) log_error_errno(return, EINVAL, "Called with uninitialized handler"); - if (!handler->conf) - log_error_errno(return, EINVAL, "Called with uninitialized conf"); - - conf = handler->conf; - len = snprintf(pidstr, sizeof(pidstr), "%d", handler->monitor_pid); if (len < 0 || (size_t)len >= sizeof(pidstr)) return; for (int i = 0; ops->hierarchies[i]; i++) { - __do_free char *pivot_path = NULL; - char pivot_cgroup[] = CGROUP_PIVOT; + __do_free char *base_path = NULL; struct hierarchy *h = ops->hierarchies[i]; int ret; if (!h->monitor_full_path) continue; - if (conf && conf->cgroup_meta.dir) - pivot_path = must_make_path(h->mountpoint, - h->container_base_path, - conf->cgroup_meta.dir, - CGROUP_PIVOT, NULL); - else - pivot_path = must_make_path(h->mountpoint, - h->container_base_path, - CGROUP_PIVOT, NULL); - - /* - * Make sure not to pass in the ro string literal CGROUP_PIVOT - * here. - */ - if (cg_legacy_handle_cpuset_hierarchy(h, pivot_cgroup) < 0) - log_warn_errno(continue, errno, "Failed to handle legacy cpuset controller"); - - ret = mkdir_p(pivot_path, 0755); - if (ret < 0 && errno != EEXIST) - log_warn_errno(continue, errno, - "Failed to create cgroup \"%s\"\n", - pivot_path); - - /* - * Move ourselves into the pivot cgroup to delete our own - * cgroup. - */ - ret = lxc_write_openat(pivot_path, "cgroup.procs", pidstr, len); + base_path = must_make_path(h->mountpoint, h->container_base_path, NULL); + ret = lxc_write_openat(base_path, "cgroup.procs", pidstr, len); if (ret != 0) log_warn_errno(continue, errno, - "Failed to move monitor %s to \"%s\"\n", - pidstr, pivot_path); + "Failed to move monitor %s to \"%s\"", + pidstr, base_path); ret = recursive_destroy(h->monitor_full_path); if (ret < 0) diff --git a/src/lxc/cgroups/cgroup.h b/src/lxc/cgroups/cgroup.h index 14daa1fc2..dccc45446 100644 --- a/src/lxc/cgroups/cgroup.h +++ b/src/lxc/cgroups/cgroup.h @@ -14,7 +14,6 @@ #define DEFAULT_MONITOR_CGROUP_PREFIX "lxc.monitor." #define CGROUP_CREATE_RETRY "-NNNN" #define CGROUP_CREATE_RETRY_LEN (STRLITERALLEN(CGROUP_CREATE_RETRY)) -#define CGROUP_PIVOT "lxc.pivot" struct lxc_handler; struct lxc_conf; diff --git a/src/lxc/file_utils.c b/src/lxc/file_utils.c index b6003a3a5..14ff7a93a 100644 --- a/src/lxc/file_utils.c +++ b/src/lxc/file_utils.c @@ -13,6 +13,7 @@ #include "config.h" #include "file_utils.h" +#include "log.h" #include "macro.h" #include "memory_utils.h" #include "string_utils.h" diff --git a/src/lxc/utils.c b/src/lxc/utils.c index 06011f96b..da2d3112b 100644 --- a/src/lxc/utils.c +++ b/src/lxc/utils.c @@ -1791,21 +1791,19 @@ int fd_cloexec(int fd, bool cloexec) return 0; } -int recursive_destroy(char *dirname) +int recursive_destroy(const char *dirname) { + __do_closedir DIR *dir = NULL; + int fret = 0; int ret; struct dirent *direntp; - DIR *dir; - int r = 0; dir = opendir(dirname); - if (!dir) { - SYSERROR("Failed to open dir \"%s\"", dirname); - return -1; - } + if (!dir) + return log_error_errno(-1, errno, "Failed to open dir \"%s\"", dirname); while ((direntp = readdir(dir))) { - char *pathname; + __do_free char *pathname = NULL; struct stat mystat; if (!strcmp(direntp->d_name, ".") || @@ -1813,44 +1811,28 @@ int recursive_destroy(char *dirname) continue; pathname = must_make_path(dirname, direntp->d_name, NULL); - ret = lstat(pathname, &mystat); if (ret < 0) { - if (!r) + if (!fret) SYSWARN("Failed to stat \"%s\"", pathname); - r = -1; - goto next; + fret = -1; + continue; } if (!S_ISDIR(mystat.st_mode)) - goto next; + continue; ret = recursive_destroy(pathname); if (ret < 0) - r = -1; - - next: - free(pathname); + fret = -1; } ret = rmdir(dirname); - if (ret < 0) { - if (!r) - SYSWARN("Failed to delete \"%s\"", dirname); - - r = -1; - } - - ret = closedir(dir); - if (ret < 0) { - if (!r) - SYSWARN("Failed to delete \"%s\"", dirname); - - r = -1; - } + if (ret < 0) + return log_warn_errno(-1, errno, "Failed to delete \"%s\"", dirname); - return r; + return fret; } int lxc_setup_keyring(void) diff --git a/src/lxc/utils.h b/src/lxc/utils.h index 5f07150db..8b3cfdfa0 100644 --- a/src/lxc/utils.h +++ b/src/lxc/utils.h @@ -231,7 +231,7 @@ extern uint64_t lxc_find_next_power2(uint64_t n); /* Set a signal the child process will receive after the parent has died. */ extern int lxc_set_death_signal(int signal, pid_t parent, int parent_status_fd); extern int fd_cloexec(int fd, bool cloexec); -extern int recursive_destroy(char *dirname); +extern int recursive_destroy(const char *dirname); extern int lxc_setup_keyring(void); #endif /* __LXC_UTILS_H */ -- 2.47.2