[PATCH RFC v3 53/90] cgroup: let a cell change nothing outside its budget

[Josef Bacik]

Whatever a task of a cell creates is owned by ids that are only lent to
the cell, and must not outlive the cell. The filesystem helpers that
decide whether an id can be stored take care of that, but not everything
in cgroupfs goes through them. Enabling a controller in
cgroup.subtree_control adds files to every child of the cgroup.
cgroup_add_file() assigns those files to the writer, directly through
kernfs.

Below the budget that is fine, because cgroup_cell_budget_put() removes
everything there. Anywhere else the files remain, and would belong to the
next cell that is lent the block. For that to happen, a builder has to
expose more of cgroupfs than the budget, and there has to be a
cgroup.subtree_control out there that the cell can write to. Neither
should be the case. But the rule that a lent id is never left behind
should not depend on nobody making a mistake.

Modes and owners do not help with a cell that has a single id either. It
runs with the id of the user who created it, so it owns every cgroup that
was delegated to that user. It could rename cgroups of a legacy hierarchy,
change their modes, and replace or exhaust the extended attributes that a
service manager keeps on them.

Implement may_change() for cgroupfs. Refuse every change by a task of a
cell to a cgroup that is not within its budget, whatever the mode and the
owner are. The check looks at the task that makes the change, not at the
task that opened the file, because the file may have been passed to the
cell.

No cgroup of a legacy hierarchy is within a budget, so a task of a cell
cannot change anything there.

Signed-off-by: Josef Bacik <josef@xxxxxxxxxxxxxx>
---
 include/linux/cell.h            |  7 +++++++
 kernel/cell/boundary.c          | 26 ++++++++++++++++++++++++++
 kernel/cgroup/cgroup-internal.h |  1 +
 kernel/cgroup/cgroup-v1.c       |  1 +
 kernel/cgroup/cgroup.c          | 19 +++++++++++++++++++
 5 files changed, 54 insertions(+)

diff --git a/include/linux/cell.h b/include/linux/cell.h
index 0c3c65b93f2b..2107cd3c994b 100644
--- a/include/linux/cell.h
+++ b/include/linux/cell.h
@@ -242,4 +242,11 @@ bool cell_may_migrate(struct task_struct *task, struct cgroup *dst);
 static inline bool cell_may_migrate(struct task_struct *t, struct cgroup *d) { return true; }
 #endif
 
+/* A task of a cell changes nothing outside of the budget. */
+#ifdef CONFIG_CELLS
+bool cell_may_change_cgroup(struct cgroup *cgrp);
+#else
+static inline bool cell_may_change_cgroup(struct cgroup *cgrp) { return true; }
+#endif
+
 #endif /* _LINUX_CELL_H */
diff --git a/kernel/cell/boundary.c b/kernel/cell/boundary.c
index 251a8516e76a..3f5cc898dc48 100644
--- a/kernel/cell/boundary.c
+++ b/kernel/cell/boundary.c
@@ -34,3 +34,29 @@ bool cell_may_migrate(struct task_struct *task, struct cgroup *dst)
 	budget = READ_ONCE(cell->budget);
 	return budget && cgroup_is_descendant(dst, budget);
 }
+
+/**
+ * cell_may_change_cgroup - may the caller change a cgroup?
+ * @cgrp: the cgroup
+ *
+ * A task of a cell changes nothing outside of the budget of the cell, whatever
+ * the modes say and whoever owns the cgroup, which a cell of one id does if its
+ * maker does. It writes to no file, makes, removes and renames no cgroup, and
+ * sets no mode, owner, time or extended attribute. See cgroup_may_change() for
+ * where this is asked.
+ *
+ * What it makes is owned by ids that were only lent to the cell. Some of that is
+ * made without the filesystem being asked whether it can store the id, such as
+ * the files that enabling a controller adds to the children of a cgroup. So it
+ * has to be where it goes with the cell, see cgroup_cell_budget_put().
+ *
+ * It goes by who is asking and not by who opened the file, which may have been
+ * handed to the cell.
+ *
+ * Return: true if the caller is in no cell, or @cgrp is the budget of its cell or
+ * lies below it.
+ */
+bool cell_may_change_cgroup(struct cgroup *cgrp)
+{
+	return cell_may_migrate(current, cgrp);
+}
diff --git a/kernel/cgroup/cgroup-internal.h b/kernel/cgroup/cgroup-internal.h
index 233fb8b45d2c..71744b453df5 100644
--- a/kernel/cgroup/cgroup-internal.h
+++ b/kernel/cgroup/cgroup-internal.h
@@ -258,6 +258,7 @@ int cgroup_lock_and_drain_offline(struct cgroup *cgrp);
 
 int cgroup_mkdir(struct kernfs_node *parent_kn, const char *name, umode_t mode);
 int cgroup_rmdir(struct kernfs_node *kn);
+int cgroup_may_change(struct kernfs_node *kn, enum kernfs_change what);
 int cgroup_show_path(struct seq_file *sf, struct kernfs_node *kf_node,
 		     struct kernfs_root *kf_root);
 
diff --git a/kernel/cgroup/cgroup-v1.c b/kernel/cgroup/cgroup-v1.c
index 216a2dcdde86..4c27e63774e6 100644
--- a/kernel/cgroup/cgroup-v1.c
+++ b/kernel/cgroup/cgroup-v1.c
@@ -1145,6 +1145,7 @@ struct kernfs_syscall_ops cgroup1_kf_syscall_ops = {
 	.mkdir			= cgroup_mkdir,
 	.rmdir			= cgroup_rmdir,
 	.show_path		= cgroup_show_path,
+	.may_change		= cgroup_may_change,
 };
 
 /*
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 3fc1f9ac2268..accae30a901d 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6488,11 +6488,30 @@ int cgroup_rmdir(struct kernfs_node *kn)
 	return ret;
 }
 
+/* Whatever the modes say, and whoever owns it. */
+int cgroup_may_change(struct kernfs_node *kn, enum kernfs_change what)
+{
+	struct cgroup *cgrp;
+
+	if (likely(!current_cell()))
+		return 0;
+
+	/* @kn may have been removed, and the cgroup may be on its way out. */
+	guard(rcu)();
+	if (kernfs_type(kn) != KERNFS_DIR)
+		kn = rcu_dereference(kn->__parent);
+	cgrp = rcu_dereference(*(void __rcu __force **)&kn->priv);
+	if (!cgrp || !cell_may_change_cgroup(cgrp))
+		return -EPERM;
+	return 0;
+}
+
 static struct kernfs_syscall_ops cgroup_kf_syscall_ops = {
 	.show_options		= cgroup_show_options,
 	.mkdir			= cgroup_mkdir,
 	.rmdir			= cgroup_rmdir,
 	.show_path		= cgroup_show_path,
+	.may_change		= cgroup_may_change,
 };
 
 static void __init cgroup_init_subsys(struct cgroup_subsys *ss, bool early)

-- 
2.55.0