[PATCH RFC v3 44/90] pidfd: let pidfd_configure() name a cgroup to start in

[Josef Bacik]

A new task of a cell starts in the budget of the cell. That stops working
once the init of the cell manages cgroups. It moves itself out of the
budget and enables controllers there, which systemd does right at the
start. A cgroup that distributes a controller to its children cannot
contain tasks, so from then on every start fails with EBUSY. Spawning is
the replacement for entering a container, so this means that nothing can
be added to a running cell.

Container runtimes have the same problem today and solve it themselves.
runc exec accepts a cgroup for the new process. If it is given none and
gets EBUSY, it looks up the cgroup of the init of the container and uses
that.

So let the spawner choose. PIDFD_SET_FD with the key "cgroup" names the
cgroup that the task starts in. Which cgroup to use is a policy decision,
and it stays with the runtime. For containers that share a cell, like
those of a pod, this is also how each of them gets its own cgroup.

The cgroup must be the budget or lie below it, because a kill finds the
tasks of the cell through the budget. The fork checks this with
cgroup_mutex held, along with whether the cgroup still exists and can
contain tasks. It has to be checked that late because the cell can change
what is below its budget at any time. The task is created in the cgroup
rather than moved there, so it is never anywhere else.

No permission on the cgroup is required. Whoever holds the cellfd can
already do anything to the cell, including killing it, and the permission
to put tasks into the budget was checked when the slot was filled.
Requiring write access to cgroup.procs would not work in practice: the
cgroups that the init of a cell creates are owned by ids that nobody
outside of the cell has, so only a builder with CAP_DAC_OVERRIDE would
pass.

Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
 include/linux/sched/task.h |  1 +
 kernel/cgroup/cgroup.c     |  8 ++++++++
 kernel/embryo.c            | 31 +++++++++++++++++++++++++++++++
 3 files changed, 40 insertions(+)

diff --git a/include/linux/sched/task.h b/include/linux/sched/task.h
index 97e3675101bd..83bc58598ea0 100644
--- a/include/linux/sched/task.h
+++ b/include/linux/sched/task.h
@@ -50,6 +50,7 @@ struct kernel_clone_args {
 	unsigned int kill_seq;
 	struct pid *pid;
 	struct cell *cell;
+	struct cgroup *cell_cgrp;
 	struct embryo *embryo;
 };
 
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 3869cfa6af50..d3a36b586cd3 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6917,6 +6917,14 @@ static int cgroup_css_set_fork_cell(struct kernel_clone_args *kargs,
 	struct cgroup *dst_cgrp = cell_budget(kargs->cell);
 	int ret;
 
+	/* Somewhere below, if whoever spawns says so. Nowhere else: a kill goes by the budget. */
+	if (kargs->cell_cgrp) {
+		ret = -EPERM;
+		if (!cgroup_is_descendant(kargs->cell_cgrp, dst_cgrp))
+			goto err;
+		dst_cgrp = kargs->cell_cgrp;
+	}
+
 	ret = -ENODEV;
 	if (cgroup_is_dead(dst_cgrp))
 		goto err;
diff --git a/kernel/embryo.c b/kernel/embryo.c
index 0ad85cd2b865..0a04f39fa37a 100644
--- a/kernel/embryo.c
+++ b/kernel/embryo.c
@@ -19,6 +19,7 @@
  */
 #include <linux/binfmts.h>
 #include <linux/cell.h>
+#include <linux/cgroup.h>
 #include <linux/completion.h>
 #include <linux/cred.h>
 #include <linux/fdtable.h>
@@ -73,6 +74,7 @@ struct embryo {
 	kuid_t			uid;
 	kgid_t			gid;
 	struct group_info	*groups;
+	struct cgroup		*cgrp;		/* where in the budget, or NULL for the budget */
 
 	struct pid		*pid;		/* the reference alloc_pid_struct() returned */
 	struct cell		*cell;		/* where the task is going to be */
@@ -133,6 +135,9 @@ static void embryo_drop_config(struct embryo *e)
 	if (e->groups)
 		put_group_info(e->groups);
 	e->groups = NULL;
+	if (e->cgrp)
+		cgroup_put(e->cgrp);
+	e->cgrp = NULL;
 }
 
 static void embryo_put(struct embryo *e)
@@ -540,6 +545,26 @@ static int embryo_set_fd(struct embryo *e, const char *key, struct file *file)
 	return 0;
 }
 
+/*
+ * Where in the budget of the cell the task starts. Whether it is in there, still
+ * there and able to hold a task is for the fork to say, with cgroup_mutex held:
+ * what is below the budget is the cell's to change at any time.
+ *
+ * It takes no permission. Whoever holds the cellfd can do anything to the cell, and
+ * what the cell made in there belongs to ids that nobody outside of it has.
+ */
+static int embryo_set_cgroup(struct embryo *e, int fd)
+{
+	struct cgroup *cgrp = cgroup_get_from_fd(fd);
+
+	if (IS_ERR(cgrp))
+		return PTR_ERR(cgrp);
+	if (e->cgrp)
+		cgroup_put(e->cgrp);
+	e->cgrp = cgrp;
+	return 0;
+}
+
 static int embryo_start(struct embryo *e)
 {
 	struct kernel_clone_args args = {
@@ -569,6 +594,8 @@ static int embryo_start(struct embryo *e)
 		ret = e->started ? -EBUSY : e->exec ? 0 : -EINVAL;
 		if (!ret)
 			e->started = true;
+		/* Nobody changes it from here on. */
+		args.cell_cgrp = e->cgrp;
 	}
 	if (ret) {
 		cell_fork_finish(e->cell);
@@ -698,6 +725,10 @@ SYSCALL_DEFINE5(pidfd_configure, int, pidfd, unsigned int, cmd,
 	case PIDFD_SET_FD: {
 		CLASS(fd, slot)(aux);
 
+		if (!strcmp(key, "cgroup")) {
+			ret = embryo_set_cgroup(e, aux);
+			break;
+		}
 		ret = -EBADF;
 		if (!fd_empty(slot))
 			ret = embryo_set_fd(e, key, fd_file(slot));

-- 
2.55.0