Merge pull request #3688 from brauner/2021-02-19/fixes_2

src/lxc/cgroups/cgroup.c

@@ -21,7 +21,7 @@
 
 lxc_log_define(cgroup, lxc);
 
-__hidden extern struct cgroup_ops *cgfsng_ops_init(struct lxc_conf *conf);
+__hidden extern struct cgroup_ops *cgroup_ops_init(struct lxc_conf *conf);
 
 struct cgroup_ops *cgroup_init(struct lxc_conf *conf)
 {
@@ -30,7 +30,7 @@ struct cgroup_ops *cgroup_init(struct lxc_conf *conf)
 	if (!conf)
 		return log_error_errno(NULL, EINVAL, "No valid conf given");
 
-	cgroup_ops = cgfsng_ops_init(conf);
+	cgroup_ops = cgroup_ops_init(conf);
 	if (!cgroup_ops)
 		return log_error_errno(NULL, errno, "Failed to initialize cgroup driver");
 
@@ -47,13 +47,13 @@ struct cgroup_ops *cgroup_init(struct lxc_conf *conf)
 	TRACE("Initialized cgroup driver %s", cgroup_ops->driver);
 
 	if (cgroup_ops->cgroup_layout == CGROUP_LAYOUT_LEGACY)
-		TRACE("Running with legacy cgroup layout");
+		TRACE("Legacy cgroup layout");
 	else if (cgroup_ops->cgroup_layout == CGROUP_LAYOUT_HYBRID)
-		TRACE("Running with hybrid cgroup layout");
+		TRACE("Hybrid cgroup layout");
 	else if (cgroup_ops->cgroup_layout == CGROUP_LAYOUT_UNIFIED)
-		TRACE("Running with unified cgroup layout");
+		TRACE("Unified cgroup layout");
 	else
-		WARN("Running with unknown cgroup layout");
+		WARN("Unsupported cgroup layout");
 
 	return cgroup_ops;
 }
@@ -73,28 +73,28 @@ void cgroup_exit(struct cgroup_ops *ops)
 
 	bpf_device_program_free(ops);
 
-	if (ops->dfd_mnt_cgroupfs_host >= 0)
-		close(ops->dfd_mnt_cgroupfs_host);
+	if (ops->dfd_mnt >= 0)
+		close(ops->dfd_mnt);
 
 	for (struct hierarchy **it = ops->hierarchies; it && *it; it++) {
 		for (char **p = (*it)->controllers; p && *p; p++)
 			free(*p);
 		free((*it)->controllers);
 
-		for (char **p = (*it)->cgroup2_chown; p && *p; p++)
+		for (char **p = (*it)->delegate; p && *p; p++)
 			free(*p);
-		free((*it)->cgroup2_chown);
+		free((*it)->delegate);
 
-		free((*it)->mountpoint);
-		free((*it)->container_base_path);
+		free((*it)->at_mnt);
+		free((*it)->at_base);
 
-		free_equal((*it)->container_full_path,
-			   (*it)->container_limit_path);
+		free_equal((*it)->path_con,
+			   (*it)->path_lim);
 
-		close_equal((*it)->cgfd_con, (*it)->cgfd_limit);
+		close_equal((*it)->dfd_con, (*it)->dfd_lim);
 
-		if ((*it)->cgfd_mon >= 0)
-			close((*it)->cgfd_mon);
+		if ((*it)->dfd_mon >= 0)
+			close((*it)->dfd_mon);
 
 		close_equal((*it)->dfd_base, (*it)->dfd_mnt);
 
@@ -106,15 +106,3 @@ void cgroup_exit(struct cgroup_ops *ops)
 
 	return;
 }
-
-#define INIT_SCOPE "/init.scope"
-char *prune_init_scope(char *cg)
-{
-	if (is_empty_string(cg))
-		return NULL;
-
-	if (strnequal(cg, INIT_SCOPE, STRLITERALLEN(INIT_SCOPE)))
-		return cg + STRLITERALLEN(INIT_SCOPE);
-
-	return cg;
-}

src/lxc/cgroups/cgroup.h

@@ -6,6 +6,7 @@
 #include <stdbool.h>
 #include <stddef.h>
 #include <sys/types.h>
+#include <linux/magic.h>
 
 #include "compiler.h"
 #include "macro.h"
@@ -32,6 +33,14 @@ typedef enum {
         CGROUP_LAYOUT_UNIFIED =  2,
 } cgroup_layout_t;
 
+typedef enum {
+	LEGACY_HIERARCHY = CGROUP_SUPER_MAGIC,
+	UNIFIED_HIERARCHY = CGROUP2_SUPER_MAGIC,
+} cgroupfs_type_magic_t;
+
+#define DEVICES_CONTROLLER (1U << 0)
+#define FREEZER_CONTROLLER (1U << 1)
+
 /* A descriptor for a mounted hierarchy
  *
  * @controllers
@@ -40,8 +49,8 @@ typedef enum {
  * - unified hierarchy
  *   Either NULL, or a null-terminated list of all enabled controllers.
  *
- * @mountpoint
- * - The mountpoint we will use.
+ * @at_mnt
+ * - The at_mnt we will use.
  * - legacy hierarchy
  *   It will be either /sys/fs/cgroup/controller or
  *   /sys/fs/cgroup/controllerlist.
@@ -50,17 +59,17 @@ typedef enum {
  *   depending on whether this is a hybrid cgroup layout (mix of legacy and
  *   unified hierarchies) or a pure unified cgroup layout.
  *
- * @container_base_path
+ * @at_base
  * - The cgroup under which the container cgroup path
  *   is created. This will be either the caller's cgroup (if not root), or
  *   init's cgroup (if root).
  *
- * @container_full_path
+ * @path_con
  * - The full path to the container's cgroup.
  *
- * @container_limit_path
+ * @path_lim
  * - The full path to the container's limiting cgroup. May simply point to
- *   container_full_path.
+ *   path_con.
  *
  * @version
  * - legacy hierarchy
@@ -71,42 +80,53 @@ typedef enum {
  *   CGROUP2_SUPER_MAGIC.
  */
 struct hierarchy {
-	/*
-	 * cgroup2 only: what files need to be chowned to delegate a cgroup to
-	 * an unprivileged user.
-	 */
-	char **cgroup2_chown;
-	char **controllers;
-	char *mountpoint;
-	char *container_base_path;
-	char *container_full_path;
-	char *container_limit_path;
-	int version;
-
-	/* cgroup2 only */
-	unsigned int bpf_device_controller:1;
-	unsigned int freezer_controller:1;
+	cgroupfs_type_magic_t fs_type;
 
-	/* File descriptor for the container's cgroup @container_full_path. */
-	int cgfd_con;
+	/* File descriptor for the container's cgroup @path_con. */
+	int dfd_con;
+	char *path_con;
 
 	/*
 	 * File descriptor for the container's limiting cgroup
-	 * @container_limit_path.
-	 * Will be equal to @cgfd_con if no limiting cgroup has been requested.
+	 * @path_lim.
+	 * Will be equal to @dfd_con if no limiting cgroup has been requested.
 	 */
-	int cgfd_limit;
+	int dfd_lim;
+	char *path_lim;
 
 	/* File descriptor for the monitor's cgroup. */
-	int cgfd_mon;
+	int dfd_mon;
 
-	/* File descriptor for the controller's mountpoint @mountpoint. */
+	/* File descriptor for the controller's mountpoint @at_mnt. */
 	int dfd_mnt;
+	char *at_mnt;
 
-	/* File descriptor for the controller's base cgroup path @container_base_path. */
+	/* File descriptor for the controller's base cgroup path @at_base. */
 	int dfd_base;
+	char *at_base;
+
+	struct /* unified hierarchy specific */ {
+		char **delegate;
+		unsigned int utilities;
+	};
+
+	char **controllers;
 };
 
+static inline bool device_utility_controller(const struct hierarchy *h)
+{
+	if (h->fs_type == UNIFIED_HIERARCHY && (h->utilities & DEVICES_CONTROLLER))
+		return true;
+	return false;
+}
+
+static inline bool freezer_utility_controller(const struct hierarchy *h)
+{
+	if (h->fs_type == UNIFIED_HIERARCHY && (h->utilities & FREEZER_CONTROLLER))
+		return true;
+	return false;
+}
+
 struct cgroup_ops {
 	/* string constant */
 	const char *driver;
@@ -124,7 +144,7 @@ struct cgroup_ops {
 	 * So for CGROUP_LAYOUT_LEGACY or CGROUP_LAYOUT_HYBRID we allow
 	 * mountpoint crossing iff we cross from a tmpfs into a cgroupfs mount.
 	 * */
-	int dfd_mnt_cgroupfs_host;
+	int dfd_mnt;
 
 	/* What controllers is the container supposed to use. */
 	char **cgroup_use;
@@ -207,8 +227,6 @@ __hidden extern struct cgroup_ops *cgroup_init(struct lxc_conf *conf);
 __hidden extern void cgroup_exit(struct cgroup_ops *ops);
 define_cleanup_function(struct cgroup_ops *, cgroup_exit);
 
-__hidden extern char *prune_init_scope(char *cg);
-
 __hidden extern int cgroup_attach(const struct lxc_conf *conf, const char *name,
 				  const char *lxcpath, pid_t pid);
 __hidden extern int cgroup_get(const char *name, const char *lxcpath,
@@ -229,7 +247,14 @@ static inline int cgroup_unified_fd(const struct cgroup_ops *ops)
 	if (!ops->unified)
 		return -EBADF;
 
-	return ops->unified->cgfd_con;
+	return ops->unified->dfd_con;
 }
 
+#define make_cgroup_path(__hierarchy, __first, ...)                    \
+	({                                                             \
+		const struct hierarchy *__h = __hierarchy;             \
+		must_make_path(DEFAULT_CGROUP_MOUNTPOINT, __h->at_mnt, \
+			       __first, __VA_ARGS__);                  \
+	})
+
 #endif /* __LXC_CGROUP_H */

src/lxc/cgroups/cgroup2_devices.c

@@ -609,7 +609,7 @@ bool bpf_cgroup_devices_attach(struct cgroup_ops *ops,
 		return syserrno(false, "Failed to create bpf program");
 
 	ret = bpf_program_cgroup_attach(prog, BPF_CGROUP_DEVICE,
-					ops->unified->cgfd_limit,
+					ops->unified->dfd_lim,
 					BPF_F_ALLOW_MULTI);
 	if (ret)
 		return syserrno(false, "Failed to attach bpf program");
@@ -635,7 +635,7 @@ bool bpf_cgroup_devices_update(struct cgroup_ops *ops,
 	if (!pure_unified_layout(ops))
 		return ret_set_errno(false, EINVAL);
 
-	if (ops->unified->cgfd_limit < 0)
+	if (ops->unified->dfd_lim < 0)
 		return ret_set_errno(false, EBADF);
 
 	/*

src/lxc/cgroups/cgroup_utils.c

@@ -20,73 +20,7 @@
 
 lxc_log_define(cgroup_utils, lxc);
 
-int get_cgroup_version(char *line)
-{
-	if (is_cgroupfs_v1(line))
-		return CGROUP_SUPER_MAGIC;
-
-	if (is_cgroupfs_v2(line))
-		return CGROUP2_SUPER_MAGIC;
-
-	return 0;
-}
-
-bool is_cgroupfs_v1(char *line)
-{
-	char *p = strstr(line, " - ");
-	if (!p)
-		return false;
-	return strnequal(p, " - cgroup ", 10);
-}
-
-bool is_cgroupfs_v2(char *line)
-{
-	char *p = strstr(line, " - ");
-	if (!p)
-		return false;
-
-	return strnequal(p, " - cgroup2 ", 11);
-}
-
-bool test_writeable_v1(char *mountpoint, char *path)
-{
-	__do_free char *fullpath = must_make_path(mountpoint, path, NULL);
-	return (access(fullpath, W_OK) == 0);
-}
-
-bool test_writeable_v2(char *mountpoint, char *path)
-{
-	/* In order to move ourselves into an appropriate sub-cgroup we need to
-	 * have write access to the parent cgroup's "cgroup.procs" file, i.e. we
-	 * need to have write access to the our current cgroups's "cgroup.procs"
-	 * file.
-	 */
-	int ret;
-	__do_free char *cgroup_path = NULL, *cgroup_procs_file = NULL,
-		       *cgroup_threads_file = NULL;
-
-	cgroup_path = must_make_path(mountpoint, path, NULL);
-	cgroup_procs_file = must_make_path(cgroup_path, "cgroup.procs", NULL);
-
-	ret = access(cgroup_path, W_OK);
-	if (ret < 0)
-		return false;
-
-	ret = access(cgroup_procs_file, W_OK);
-	if (ret < 0)
-		return false;
-
-	/* Newer versions of cgroup2 now also require write access to the
-	 * "cgroup.threads" file.
-	 */
-	cgroup_threads_file = must_make_path(cgroup_path, "cgroup.threads", NULL);
-	if (!file_exists(cgroup_threads_file))
-		return true;
-
-	return (access(cgroup_threads_file, W_OK) == 0);
-}
-
-int unified_cgroup_fd(int fd)
+bool unified_cgroup_fd(int fd)
 {
 
 	int ret;
@@ -159,3 +93,32 @@ int cgroup_tree_prune(int dfd, const char *path)
 
 	return 0;
 }
+
+#define INIT_SCOPE "/init.scope"
+char *prune_init_scope(char *path)
+{
+	char *slash = path;
+	size_t len;
+
+	/*
+	 * This function can only be called on information parsed from
+	 * /proc/<pid>/cgroup. The file displays the current cgroup of the
+	 * process as absolute paths. So if we are passed a non-absolute path
+	 * things are way wrong.
+	 */
+	if (!abspath(path))
+		return ret_set_errno(NULL, EINVAL);
+
+	len = strlen(path);
+	if (len < STRLITERALLEN(INIT_SCOPE))
+		return path;
+
+	slash += (len - STRLITERALLEN(INIT_SCOPE));
+	if (strequal(slash, INIT_SCOPE)) {
+		if (slash == path)
+			slash++;
+		*slash = '\0';
+	}
+
+	return path;
+}

src/lxc/cgroups/cgroup_utils.h

@@ -9,27 +9,7 @@
 #include "compiler.h"
 #include "file_utils.h"
 
-/* Retrieve the cgroup version of a given entry from /proc/<pid>/mountinfo. */
-__hidden extern int get_cgroup_version(char *line);
-
-/* Check if given entry from /proc/<pid>/mountinfo is a cgroupfs v1 mount. */
-__hidden extern bool is_cgroupfs_v1(char *line);
-
-/* Check if given entry from /proc/<pid>/mountinfo is a cgroupfs v2 mount. */
-__hidden extern bool is_cgroupfs_v2(char *line);
-
-/* Given a v1 hierarchy @mountpoint and base @path, verify that we can create
- * directories underneath it.
- */
-__hidden extern bool test_writeable_v1(char *mountpoint, char *path);
-
-/* Given a v2 hierarchy @mountpoint and base @path, verify that we can create
- * directories underneath it and that we have write access to the cgroup's
- * "cgroup.procs" file.
- */
-__hidden extern bool test_writeable_v2(char *mountpoint, char *path);
-
-__hidden extern int unified_cgroup_fd(int fd);
+__hidden extern bool unified_cgroup_fd(int fd);
 
 static inline bool cgns_supported(void)
 {
@@ -43,4 +23,11 @@ static inline bool cgns_supported(void)
 
 __hidden extern int cgroup_tree_prune(int dfd, const char *path);
 
+/*
+ * This function can only be called on information parsed from
+ * /proc/<pid>/cgroup or on absolute paths and it will verify the latter and
+ * return NULL if a relative path is passed.
+ */
+__hidden extern char *prune_init_scope(char *path);
+
 #endif /* __LXC_CGROUP_UTILS_H */

src/lxc/commands.c

@@ -898,7 +898,7 @@ static int lxc_cmd_stop_callback(int fd, struct lxc_cmd_req *req,
 			TRACE("Sent signal %d to pidfd %d", stopsignal, handler->pid);
 
 		if (pure_unified_layout(cgroup_ops))
-			ret = __cgroup_unfreeze(cgroup_ops->unified->cgfd_limit, -1);
+			ret = __cgroup_unfreeze(cgroup_ops->unified->dfd_lim, -1);
 		else
 			ret = cgroup_ops->unfreeze(cgroup_ops, -1);
 		if (ret)
@@ -1518,8 +1518,8 @@ static int lxc_cmd_get_cgroup2_fd_callback_do(int fd, struct lxc_cmd_req *req,
 	if (!pure_unified_layout(ops) || !ops->unified)
 		return lxc_cmd_rsp_send(fd, &rsp);
 
-	send_fd = limiting_cgroup ? ops->unified->cgfd_limit
-				  : ops->unified->cgfd_con;
+	send_fd = limiting_cgroup ? ops->unified->dfd_lim
+				  : ops->unified->dfd_con;
 
 	rsp.ret = 0;
 	ret = lxc_abstract_unix_send_fds(fd, &send_fd, 1, &rsp, sizeof(rsp));

src/lxc/compiler.h

@@ -85,4 +85,7 @@
 #define __public __attribute__((visibility("default")))
 #endif
 
+#define likely(x) __builtin_expect(!!(x), 1)
+#define unlikely(x) __builtin_expect(!!(x), 0)
+
 #endif /* __LXC_COMPILER_H */

src/lxc/file_utils.c

@@ -63,9 +63,9 @@ int lxc_write_openat(const char *dir, const char *filename, const void *buf,
 {
 	__do_close int dirfd = -EBADF;
 
-	dirfd = open(dir, O_DIRECTORY | O_RDONLY | O_CLOEXEC | O_NOCTTY | O_NOFOLLOW);
+	dirfd = open(dir, PROTECT_OPEN);
 	if (dirfd < 0)
-		return -1;
+		return -errno;
 
 	return lxc_writeat(dirfd, filename, buf, count);
 }

src/lxc/log.h

@@ -515,6 +515,13 @@ __lxc_unused static inline void LXC_##LEVEL(struct lxc_log_locinfo* locinfo,	\
 		__internal_ret__;                             \
 	})
 
+#define sysinfo(__ret__, format, ...)                         \
+	({                                                    \
+		typeof(__ret__) __internal_ret__ = (__ret__); \
+		SYSINFO(format, ##__VA_ARGS__);               \
+		__internal_ret__;                             \
+	})
+
 #define syserrno_set(__ret__, format, ...)                    \
 	({                                                    \
 		typeof(__ret__) __internal_ret__ = (__ret__); \

src/lxc/macro.h

@@ -21,6 +21,8 @@
 #include <sys/un.h>
 #include <unistd.h>
 
+#include "compiler.h"
+
 #ifndef PATH_MAX
 #define PATH_MAX 4096
 #endif
@@ -406,11 +408,6 @@ extern int __build_bug_on_failed;
 	} while (0)
 #endif
 
-#define lxc_iterate_parts(__iterator, __splitme, __separators)                  \
-	for (char *__p = NULL, *__it = strtok_r(__splitme, __separators, &__p); \
-	     (__iterator = __it);                                               \
-	     __iterator = __it = strtok_r(NULL, __separators, &__p))
-
 #define prctl_arg(x) ((unsigned long)x)
 
 /* networking */
@@ -703,4 +700,41 @@ enum {
 		(b) = __tmp;           \
 	} while (0)
 
+#define MAX_ERRNO 4095
+
+#define IS_ERR_VALUE(x) unlikely((x) >= (unsigned long)-MAX_ERRNO)
+
+static inline void *ERR_PTR(long error)
+{
+	return (void *)error;
+}
+
+static inline long PTR_ERR(const void *ptr)
+{
+	return (long)ptr;
+}
+
+static inline long IS_ERR(const void *ptr)
+{
+	return IS_ERR_VALUE((unsigned long)ptr);
+}
+
+static inline long IS_ERR_OR_NULL(const void *ptr)
+{
+	return !ptr || IS_ERR_VALUE((unsigned long)ptr);
+}
+
+static inline void *ERR_CAST(const void *ptr)
+{
+	return (void *)ptr;
+}
+
+static inline int PTR_RET(const void *ptr)
+{
+	if (IS_ERR(ptr))
+		return PTR_ERR(ptr);
+	else
+		return 0;
+}
+
 #endif /* __LXC_MACRO_H */

src/lxc/memory_utils.h

@@ -52,8 +52,10 @@ define_cleanup_function(DIR *, closedir);
 
 #define free_disarm(ptr)                    \
 	({                                  \
+		if (!IS_ERR_OR_NULL(ptr)) { \
 			free(ptr);          \
 			ptr = NULL;         \
+		}                           \
 	})
 
 static inline void free_disarm_function(void *ptr)
@@ -64,7 +66,7 @@ static inline void free_disarm_function(void *ptr)
 
 static inline void free_string_list(char **list)
 {
-	if (list) {
+	if (list && !IS_ERR(list)) {
 		for (int i = 0; list[i]; i++)
 			free(list[i]);
 		free_disarm(list);

src/lxc/string_utils.h

@@ -187,4 +187,9 @@ static inline const char *fdstr(int fd)
 	return buf;
 }
 
+#define lxc_iterate_parts(__iterator, __splitme, __separators)                  \
+	for (char *__p = NULL, *__it = strtok_r(__splitme, __separators, &__p); \
+	     (__iterator = __it);                                               \
+	     __iterator = __it = strtok_r(NULL, __separators, &__p))
+
 #endif /* __LXC_STRING_UTILS_H */