In cgroup1 every task is an implicit member of the root cgroup of every
registered hierarchy, so a starting container's root_cset references the
root cgroups of all cgroup1 hierarchies present on the host - including
ones the container manager never created a per-container subdirectory in.
ve_check_root_cgroups() rejects a start if any virtualized cgroup in the
cset is a host root (no parent), to ensure the container has its own
isolated subdirectory in each virtualized hierarchy. But a cgroup1
hierarchy can outlive its umount: cgroup_kill_sb() only kills the root
reference when the root has no children at umount time, and a child
removed shortly before the umount lingers on ->self.children until its
css release finishes (several RCU grace periods later). Nothing retries
the kill, so
mount -t cgroup -o devices xxx /mnt
mkdir /mnt/child; rmdir /mnt/child
umount /mnt
leaves the devices hierarchy registered indefinitely. Its root then sits
in every task's css_set and ve_check_root_cgroups() rejects EVERY
subsequent container start with -EINVAL (returned by cgroup_mark_ve_roots()
-> ve_start_container() out of the ve.state write) until the host is
rebooted. A transient cgroup1 mount+child+umount thus denies container
start host-wide.
A cgroup1 hierarchy with no cgroups besides its root is not used by
anyone: the container is only an involuntary member of its root and there
is nothing to virtualize in it. Skip such roots in ve_check_root_cgroups()
and, symmetrically, in the mark/unmark loops so a host root is never
marked as a VE root. A hierarchy that is actually in use (its root has
children - a populated named/systemd hierarchy, or any cgroup1 someone
created a subdirectory in) is still rejected, so the isolation guarantee
is preserved.
Fixes: 68dd8db57792 ("ve/cgroup: Skip non-virtualized roots in
cgroup_{,un}mark_ve_roots()")
Feature: ve: ve generic structures
https://virtuozzo.atlassian.net/browse/VSTOR-137234
Signed-off-by: Konstantin Khorenko <[email protected]>
---
kernel/cgroup/cgroup.c | 50 ++++++++++++++++++++++++++++++++++++++++++
1 file changed, 50 insertions(+)
diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index 4db77090f1be..cb761954f15a 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -2150,6 +2150,39 @@ static inline bool is_virtualized_cgroup(struct cgroup
*cgrp)
return is_virtualized_cgroot(cgrp->root);
}
+/*
+ * In cgroup1 every task belongs to the root cgroup of every active
+ * hierarchy, so the root_cset of a starting container references root
+ * cgroups of all registered cgroup1 hierarchies, including hierarchies
+ * the container manager knows nothing about and thus could not create
+ * a per-container subdirectory in.
+ *
+ * Moreover, a cgroup1 hierarchy can outlive its umount: cgroup_kill_sb()
+ * does not kill the root if the root cgroup still has children, and a
+ * child removed shortly before the umount lingers on ->self.children
+ * until its css release is finished. Nothing ever retries killing such
+ * a root, so e.g. "mount -t cgroup -o devices xxx /mnt; mkdir /mnt/x;
+ * rmdir /mnt/x; umount /mnt" leaves the hierarchy registered until the
+ * next mount/umount cycle of it.
+ *
+ * Container start must not be refused because of such hierarchies: if a
+ * cgroup1 hierarchy has no cgroups except the root one, nobody uses it,
+ * the container is only an involuntary member of its root and there is
+ * nothing to virtualize in it. Skip it both in ve_check_root_cgroups()
+ * and in the mark/unmark loops (a host root cgroup must never be marked
+ * as a VE root).
+ *
+ * The children list is modified under cgroup_mutex, which the callers
+ * may not hold, but a racy empty <-> non-empty transition can only turn
+ * the skip into the -EINVAL check failure or vice versa, both acceptable
+ * outcomes for a hierarchy changing in the middle of container start.
+ */
+static inline bool ve_unused_cgroup1_root(struct cgroup *cgrp)
+{
+ return cgrp->root != &cgrp_dfl_root && !cgroup_parent(cgrp) &&
+ list_empty(&cgrp->self.children);
+}
+
/*
* Iterate all cgroups in a given css_set and for all obligatory Virtuozzo
* container cgroups check that container has its own cgroup subdirectory:
@@ -2165,6 +2198,10 @@ static inline bool ve_check_root_cgroups(struct css_set
*cset)
if (!is_virtualized_cgroup(link->cgrp))
continue;
+ /* Roots of unused (possibly stale) cgroup1 hierarchies */
+ if (ve_unused_cgroup1_root(link->cgrp))
+ continue;
+
/* Host cgroups not allowed */
if (!rcu_access_pointer(link->cgrp->kn->__parent))
return true;
@@ -2203,6 +2240,10 @@ int cgroup_mark_ve_roots(struct ve_struct *ve)
if (!is_virtualized_cgroup(cgrp))
continue;
+ /* Skipped by ve_check_root_cgroups(), must not be marked */
+ if (ve_unused_cgroup1_root(cgrp))
+ continue;
+
rcu_assign_pointer(cgrp->ve_owner, ve);
set_bit(CGRP_VE_ROOT, &cgrp->flags);
}
@@ -2236,6 +2277,15 @@ void cgroup_unmark_ve_roots(struct ve_struct *ve)
if (!is_virtualized_cgroup(cgrp))
continue;
+ /*
+ * Not marked at container start. Note: even if the root has
+ * gained children since then (the helper now returns false),
+ * clearing an unmarked root cgroup below is a harmless no-op:
+ * root cgroups are never marked as VE roots.
+ */
+ if (ve_unused_cgroup1_root(cgrp))
+ continue;
+
rcu_assign_pointer(cgrp->ve_owner, NULL);
clear_bit(CGRP_VE_ROOT, &cgrp->flags);
}
--
2.47.1
_______________________________________________
Devel mailing list
[email protected]
https://lists.openvz.org/mailman/listinfo/devel
