Merge pull request #3677 from brauner/2021-02-17/cgroup_pruning

src/lxc/cgroups/cgfsng.c

@@ -782,9 +782,9 @@ static void lxc_cgfsng_print_basecg_debuginfo(char *basecginfo, char **klist,
 		TRACE("named subsystem %d: %s", k, *it);
 }
 
-static int cgroup_tree_remove(struct hierarchy **hierarchies, const char *container_cgroup)
+static int cgroup_tree_remove(struct hierarchy **hierarchies, const char *path_prune)
 {
-	if (!container_cgroup || !hierarchies)
+	if (!path_prune || !hierarchies)
 		return 0;
 
 	for (int i = 0; hierarchies[i]; i++) {
@@ -794,9 +794,11 @@ static int cgroup_tree_remove(struct hierarchy **hierarchies, const char *contai
 		if (!h->container_limit_path)
 			continue;
 
-		ret = lxc_rm_rf(h->container_limit_path);
+		ret = cgroup_tree_prune(h->dfd_base, path_prune);
 		if (ret < 0)
-			WARN("Failed to destroy \"%s\"", h->container_limit_path);
+			SYSWARN("Failed to destroy %d(%s)", h->dfd_base, path_prune);
+		else
+			TRACE("Removed cgroup tree %d(%s)", h->dfd_base, path_prune);
 
 		if (h->container_limit_path != h->container_full_path)
 			free_disarm(h->container_limit_path);
@@ -808,7 +810,7 @@ static int cgroup_tree_remove(struct hierarchy **hierarchies, const char *contai
 
 struct generic_userns_exec_data {
 	struct hierarchy **hierarchies;
-	const char *container_cgroup;
+	const char *path_prune;
 	struct lxc_conf *conf;
 	uid_t origuid; /* target uid in parent namespace */
 	char *path;
@@ -834,7 +836,7 @@ static int cgroup_tree_remove_wrapper(void *data)
 		return log_error_errno(-1, errno, "Failed to setresuid(%d, %d, %d)",
 				       (int)nsuid, (int)nsuid, (int)nsuid);
 
-	return cgroup_tree_remove(arg->hierarchies, arg->container_cgroup);
+	return cgroup_tree_remove(arg->hierarchies, arg->path_prune);
 }
 
 __cgfsng_ops static void cgfsng_payload_destroy(struct cgroup_ops *ops,
@@ -869,14 +871,14 @@ __cgfsng_ops static void cgfsng_payload_destroy(struct cgroup_ops *ops,
 	if (!lxc_list_empty(&handler->conf->id_map)) {
 		struct generic_userns_exec_data wrap = {
 			.conf			= handler->conf,
-			.container_cgroup	= ops->container_cgroup,
+			.path_prune		= ops->container_limit_cgroup,
 			.hierarchies		= ops->hierarchies,
 			.origuid		= 0,
 		};
 		ret = userns_exec_1(handler->conf, cgroup_tree_remove_wrapper,
 				    &wrap, "cgroup_tree_remove_wrapper");
 	} else {
-		ret = cgroup_tree_remove(ops->hierarchies, ops->container_cgroup);
+		ret = cgroup_tree_remove(ops->hierarchies, ops->container_limit_cgroup);
 	}
 	if (ret < 0)
 		SYSWARN("Failed to destroy cgroups");
@@ -1098,9 +1100,8 @@ static int __cgroup_tree_create(int dfd_base, const char *path, mode_t mode,
 }
 
 static bool cgroup_tree_create(struct cgroup_ops *ops, struct lxc_conf *conf,
-			       struct hierarchy *h, const char *cgroup_tree,
-			       const char *cgroup_leaf, bool payload,
-			       const char *cgroup_limit_dir)
+			       struct hierarchy *h, const char *cgroup_leaf,
+			       bool payload, const char *cgroup_limit_dir)
 {
 	__do_close int fd_limit = -EBADF, fd_final = -EBADF;
 	__do_free char *path = NULL, *limit_path = NULL;
@@ -1226,7 +1227,7 @@ __cgfsng_ops static void cgfsng_monitor_destroy(struct cgroup_ops *ops,
 		/* Monitor might have died before we entered the cgroup. */
 		if (handler->monitor_pid <= 0) {
 			WARN("No valid monitor process found while destroying cgroups");
-			goto try_lxc_rm_rf;
+			goto cgroup_prune_tree;
 		}
 
 		if (conf->cgroup_meta.monitor_pivot_dir)
@@ -1252,10 +1253,12 @@ __cgfsng_ops static void cgfsng_monitor_destroy(struct cgroup_ops *ops,
 			continue;
 		}
 
-try_lxc_rm_rf:
-		ret = lxc_rm_rf(h->monitor_full_path);
+cgroup_prune_tree:
+		ret = cgroup_tree_prune(h->dfd_base, ops->monitor_cgroup);
 		if (ret < 0)
-			WARN("Failed to destroy \"%s\"", h->monitor_full_path);
+			SYSWARN("Failed to destroy %d(%s)", h->dfd_base, ops->monitor_cgroup);
+		else
+			TRACE("Removed cgroup tree %d(%s)", h->dfd_base, ops->monitor_cgroup);
 	}
 }
 
@@ -1291,8 +1294,7 @@ static bool check_cgroup_dir_config(struct lxc_conf *conf)
 
 __cgfsng_ops static bool cgfsng_monitor_create(struct cgroup_ops *ops, struct lxc_handler *handler)
 {
-	__do_free char *monitor_cgroup = NULL, *__cgroup_tree = NULL;
-	const char *cgroup_tree;
+	__do_free char *monitor_cgroup = NULL;
 	int idx = 0;
 	int i;
 	size_t len;
@@ -1317,25 +1319,23 @@ __cgfsng_ops static bool cgfsng_monitor_create(struct cgroup_ops *ops, struct lx
 		return false;
 
 	if (conf->cgroup_meta.monitor_dir) {
-		cgroup_tree = NULL;
 		monitor_cgroup = strdup(conf->cgroup_meta.monitor_dir);
 	} else if (conf->cgroup_meta.dir) {
-		cgroup_tree = conf->cgroup_meta.dir;
 		monitor_cgroup = must_concat(&len, conf->cgroup_meta.dir, "/",
 					     DEFAULT_MONITOR_CGROUP_PREFIX,
 					     handler->name,
 					     CGROUP_CREATE_RETRY, NULL);
 	} else if (ops->cgroup_pattern) {
-		__cgroup_tree = lxc_string_replace("%n", handler->name, ops->cgroup_pattern);
-		if (!__cgroup_tree)
+		__do_free char *cgroup_tree = NULL;
+
+		cgroup_tree = lxc_string_replace("%n", handler->name, ops->cgroup_pattern);
+		if (!cgroup_tree)
 			return ret_set_errno(false, ENOMEM);
 
-		cgroup_tree = __cgroup_tree;
 		monitor_cgroup = must_concat(&len, cgroup_tree, "/",
 					     DEFAULT_MONITOR_CGROUP,
 					     CGROUP_CREATE_RETRY, NULL);
 	} else {
-		cgroup_tree = NULL;
 		monitor_cgroup = must_concat(&len, DEFAULT_MONITOR_CGROUP_PREFIX,
 					     handler->name,
 					     CGROUP_CREATE_RETRY, NULL);
@@ -1353,8 +1353,8 @@ __cgfsng_ops static bool cgfsng_monitor_create(struct cgroup_ops *ops, struct lx
 
 		for (i = 0; ops->hierarchies[i]; i++) {
 			if (cgroup_tree_create(ops, handler->conf,
-				               ops->hierarchies[i], cgroup_tree,
-				               monitor_cgroup, false, NULL))
+					       ops->hierarchies[i],
+					       monitor_cgroup, false, NULL))
 				continue;
 
 			DEBUG("Failed to create cgroup \"%s\"", maybe_empty(ops->hierarchies[i]->monitor_full_path));
@@ -1379,10 +1379,7 @@ __cgfsng_ops static bool cgfsng_monitor_create(struct cgroup_ops *ops, struct lx
  */
 __cgfsng_ops static bool cgfsng_payload_create(struct cgroup_ops *ops, struct lxc_handler *handler)
 {
-	__do_free char *container_cgroup = NULL,
-		       *__cgroup_tree = NULL,
-		       *limiting_cgroup = NULL;
-	const char *cgroup_tree;
+	__do_free char *container_cgroup = NULL, *limiting_cgroup = NULL;
 	int idx = 0;
 	int i;
 	size_t len;
@@ -1407,8 +1404,6 @@ __cgfsng_ops static bool cgfsng_payload_create(struct cgroup_ops *ops, struct lx
 		return false;
 
 	if (conf->cgroup_meta.container_dir) {
-		cgroup_tree = NULL;
-
 		limiting_cgroup = strdup(conf->cgroup_meta.container_dir);
 		if (!limiting_cgroup)
 			return ret_set_errno(false, ENOMEM);
@@ -1422,22 +1417,21 @@ __cgfsng_ops static bool cgfsng_payload_create(struct cgroup_ops *ops, struct lx
 			container_cgroup = move_ptr(limiting_cgroup);
 		}
 	} else if (conf->cgroup_meta.dir) {
-		cgroup_tree = conf->cgroup_meta.dir;
-		container_cgroup = must_concat(&len, cgroup_tree, "/",
+		container_cgroup = must_concat(&len, conf->cgroup_meta.dir, "/",
 					     DEFAULT_PAYLOAD_CGROUP_PREFIX,
 					     handler->name,
 					     CGROUP_CREATE_RETRY, NULL);
 	} else if (ops->cgroup_pattern) {
-		__cgroup_tree = lxc_string_replace("%n", handler->name, ops->cgroup_pattern);
-		if (!__cgroup_tree)
+		__do_free char *cgroup_tree = NULL;
+
+		cgroup_tree = lxc_string_replace("%n", handler->name, ops->cgroup_pattern);
+		if (!cgroup_tree)
 			return ret_set_errno(false, ENOMEM);
 
-		cgroup_tree = __cgroup_tree;
 		container_cgroup = must_concat(&len, cgroup_tree, "/",
 					       DEFAULT_PAYLOAD_CGROUP,
 					       CGROUP_CREATE_RETRY, NULL);
 	} else {
-		cgroup_tree = NULL;
 		container_cgroup = must_concat(&len, DEFAULT_PAYLOAD_CGROUP_PREFIX,
 					     handler->name,
 					     CGROUP_CREATE_RETRY, NULL);
@@ -1455,7 +1449,7 @@ __cgfsng_ops static bool cgfsng_payload_create(struct cgroup_ops *ops, struct lx
 
 		for (i = 0; ops->hierarchies[i]; i++) {
 			if (cgroup_tree_create(ops, handler->conf,
-					       ops->hierarchies[i], cgroup_tree,
+					       ops->hierarchies[i],
 					       container_cgroup, true,
 					       limiting_cgroup))
 				continue;
@@ -1473,6 +1467,10 @@ __cgfsng_ops static bool cgfsng_payload_create(struct cgroup_ops *ops, struct lx
 		return log_error_errno(false, ERANGE, "Failed to create container cgroup");
 
 	ops->container_cgroup = move_ptr(container_cgroup);
+	if (limiting_cgroup)
+		ops->container_limit_cgroup = move_ptr(limiting_cgroup);
+	else
+		ops->container_limit_cgroup = ops->container_cgroup;
 	INFO("The container process uses \"%s\" as cgroup", ops->container_cgroup);
 	return true;
 }

src/lxc/cgroups/cgroup.c

@@ -66,9 +66,14 @@ void cgroup_exit(struct cgroup_ops *ops)
 		free(*cur);
 
 	free(ops->cgroup_pattern);
-	free(ops->container_cgroup);
 	free(ops->monitor_cgroup);
 
+	{
+		if (ops->container_cgroup != ops->container_limit_cgroup)
+			free(ops->container_limit_cgroup);
+		free(ops->container_cgroup);
+	}
+
 	if (ops->cgroup2_devices)
 		bpf_program_free(ops->cgroup2_devices);
 

src/lxc/cgroups/cgroup.h

@@ -134,6 +134,7 @@ struct cgroup_ops {
 	char **cgroup_use;
 	char *cgroup_pattern;
 	char *container_cgroup;
+	char *container_limit_cgroup;
 	char *monitor_cgroup;
 
 	/* @hierarchies

src/lxc/cgroups/cgroup_utils.c

@@ -13,10 +13,13 @@
 #include "cgroup_utils.h"
 #include "config.h"
 #include "file_utils.h"
+#include "log.h"
 #include "macro.h"
 #include "memory_utils.h"
 #include "utils.h"
 
+lxc_log_define(cgroup_utils, lxc);
+
 int get_cgroup_version(char *line)
 {
 	if (is_cgroupfs_v1(line))
@@ -95,3 +98,64 @@ int unified_cgroup_fd(int fd)
 
 	return false;
 }
+
+int cgroup_tree_prune(int dfd, const char *path)
+{
+	__do_close int dfd_disown = -EBADF, dfd_dup = -EBADF;
+	__do_closedir DIR *dir = NULL;
+	int ret;
+	struct dirent *direntp;
+
+	/*
+	 * The unlinkat() syscall doesn't work with empty paths, i.e. it isn't
+	 * possible to remove the fd itself.
+	 */
+	if (is_empty_string(path) || strequal(path, "."))
+		return ret_errno(EINVAL);
+
+	/*
+	 * Note that O_PATH file descriptors can't be used with getdents() and
+	 * therefore with readdir().
+	 */
+	dfd_disown = open_at(dfd, path, PROTECT_OPEN,
+			     PROTECT_LOOKUP_BENEATH_WITH_SYMLINKS, 0);
+	if (dfd_disown < 0)
+		return -errno;
+
+	dfd_dup = dup_cloexec(dfd_disown);
+	if (dfd_dup < 0)
+		return -errno;
+
+	dir = fdopendir(dfd_disown);
+	if (!dir)
+		return -errno;
+
+	/* Transfer ownership to fdopendir(). */
+	move_fd(dfd_disown);
+
+	while ((direntp = readdir(dir))) {
+		struct stat st;
+
+		if (strequal(direntp->d_name, ".") ||
+		    strequal(direntp->d_name, ".."))
+			continue;
+
+		ret = fstatat(dfd_dup, direntp->d_name, &st,
+			      AT_NO_AUTOMOUNT | AT_SYMLINK_NOFOLLOW);
+		if (ret < 0)
+			continue;
+
+		if (!S_ISDIR(st.st_mode))
+			continue;
+
+		ret = cgroup_tree_prune(dfd_dup, direntp->d_name);
+		if (ret < 0)
+			return -errno;
+	}
+
+	ret = unlinkat(dfd, path, AT_REMOVEDIR);
+	if (ret < 0)
+		return -errno;
+
+	return 0;
+}

src/lxc/cgroups/cgroup_utils.h

@@ -41,4 +41,6 @@ static inline bool cgns_supported(void)
 	return supported == 1;
 }
 
+__hidden extern int cgroup_tree_prune(int dfd, const char *path);
+
 #endif /* __LXC_CGROUP_UTILS_H */

src/lxc/file_utils.c

@@ -564,20 +564,6 @@ int fd_cloexec(int fd, bool cloexec)
 	return 0;
 }
 
-static inline int dup_cloexec(int fd)
-{
-	__do_close int fd_dup = -EBADF;
-
-	fd_dup = dup(fd);
-	if (fd_dup < 0)
-		return -errno;
-
-	if (fd_cloexec(fd_dup, true))
-		return -errno;
-
-	return move_fd(fd_dup);
-}
-
 FILE *fdopen_at(int dfd, const char *path, const char *mode,
 		unsigned int o_flags, unsigned int resolve_flags)
 {

src/lxc/file_utils.h

@@ -13,6 +13,7 @@
 #include <unistd.h>
 
 #include "compiler.h"
+#include "memory_utils.h"
 #include "syscall_wrappers.h"
 
 /* read and write whole files */
@@ -105,4 +106,18 @@ __hidden extern ssize_t lxc_read_try_buf_at(int dfd, const char *path,
  */
 __hidden extern bool same_file_lax(int fda, int fdb);
 
+static inline int dup_cloexec(int fd)
+{
+	__do_close int fd_dup = -EBADF;
+
+	fd_dup = dup(fd);
+	if (fd_dup < 0)
+		return -errno;
+
+	if (fd_cloexec(fd_dup, true))
+		return -errno;
+
+	return move_fd(fd_dup);
+}
+
 #endif /* __LXC_FILE_UTILS_H */