cgroups/cgfsng: rework cgroup removal

Signed-off-by: Christian Brauner <christian.brauner@ubuntu.com>

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)

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;

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"

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);
+	if (ret < 0)
+		return log_warn_errno(-1, errno, "Failed to delete \"%s\"", dirname);
 
-		r = -1;
-	}
-
-	ret = closedir(dir);
-	if (ret < 0) {
-		if (!r)
-			SYSWARN("Failed to delete \"%s\"", dirname);
-
-		r = -1;
-	}
-
-	return r;
+	return fret;
 }
 
 int lxc_setup_keyring(void)

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 */