[PATCH] cgroup: reject controller re-enable while css teardown is pending

[Tao Yu]

Writing cgroup.subtree_control currently calls cgroup_kn_lock_live() with
drain_offline=true and can therefore block in cgroup_lock_and_drain_offline()
before taking cgroup_mutex.

After 1dffd95575eb ("cgroup: Defer kill_css_finish() in
cgroup_apply_control_disable()"), disabling a controller can leave CSS_DYING
csses behind until the subtree is fully drained. If userspace re-enables a
controller before that teardown completes, the write path may wait
indefinitely from kernfs context. syzbot reported this as hung tasks stuck in
cgroup_subtree_control_write().

The write side does not need to synchronously drain all pending teardowns. It
only needs to avoid reusing controller state whose css teardown is still in
flight. Blocking the user-visible write path in TASK_UNINTERRUPTIBLE is the
wrong tradeoff here.

Stop draining offline csses from subtree_control and cgroup.type writes.
Instead, reject the operation with -EBUSY if it would reuse a subsystem which
still has dying csses in the target subtree. Limit the subtree_control check
to controllers being newly enabled, and use the current subtree controller
mask for cgroup.type because threaded conversion re-applies that state.

This preserves the asynchronous teardown model, avoids hung writers in D
state, and keeps the rejection scoped to the controllers that are actually
racing with teardown.

Fixes: 1dffd95575eb ("cgroup: Defer kill_css_finish() in cgroup_apply_control_disable()")
Reported-by: syzbot+bb2e19a1190a556c01b1@xxxxxxxxxxxxxxxxxxxxxxxxx
Closes: https://syzkaller.appspot.com/bug?extid=bb2e19a1190a556c01b1
Signed-off-by: Tao Yu <tao1.yu@xxxxxxxxx>
---
 kernel/cgroup/cgroup.c | 40 +++++++++++++++++++++++++++++++++++++---
 1 file changed, 37 insertions(+), 3 deletions(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index c3a12fee7528f..129a9b0752416 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -3356,6 +3356,20 @@ void cgroup_lock_and_drain_offline(struct cgroup *cgrp)
 	}
 }
 
+static bool cgroup_has_dying_csses(struct cgroup *cgrp, u32 ss_mask)
+{
+	int ssid;
+
+	lockdep_assert_held(&cgroup_mutex);
+
+	for (ssid = 0; ssid < CGROUP_SUBSYS_COUNT; ssid++) {
+		if ((ss_mask & (1 << ssid)) && cgrp->nr_dying_subsys[ssid])
+			return true;
+	}
+
+	return false;
+}
+
 /**
  * cgroup_save_control - save control masks and dom_cgrp of a subtree
  * @cgrp: root of the target subtree
@@ -3649,7 +3663,7 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of,
 			return -EINVAL;
 	}
 
-	cgrp = cgroup_kn_lock_live(of->kn, true);
+	cgrp = cgroup_kn_lock_live(of->kn, false);
 	if (!cgrp)
 		return -ENODEV;
 
@@ -3689,6 +3703,17 @@ static ssize_t cgroup_subtree_control_write(struct kernfs_open_file *of,
 	if (ret)
 		goto out_unlock;
 
+	/*
+	 * Disabled controllers are offlined asynchronously. Don't sleep here
+	 * waiting for them to drain while holding the kernfs write context.
+	 * Report the in-flight teardown and let userspace retry once the dying
+	 * csses have gone away.
+	 */
+	if (enable && cgroup_has_dying_csses(cgrp, enable)) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
 	/* save and update control masks and prepare csses */
 	cgroup_save_control(cgrp);
 
@@ -3788,14 +3813,23 @@ static ssize_t cgroup_type_write(struct kernfs_open_file *of, char *buf,
 	if (strcmp(strstrip(buf), "threaded"))
 		return -EINVAL;
 
-	/* drain dying csses before we re-apply (threaded) subtree control */
-	cgrp = cgroup_kn_lock_live(of->kn, true);
+	/*
+	 * Threaded conversion re-applies subtree control, so reject it while a
+	 * prior controller disable is still offlining csses in the subtree.
+	 */
+	cgrp = cgroup_kn_lock_live(of->kn, false);
 	if (!cgrp)
 		return -ENOENT;
 
+	if (cgroup_has_dying_csses(cgrp, cgrp->subtree_ss_mask)) {
+		ret = -EBUSY;
+		goto out_unlock;
+	}
+
 	/* threaded can only be enabled */
 	ret = cgroup_enable_threaded(cgrp);
 
+out_unlock:
 	cgroup_kn_unlock(of->kn);
 	return ret ?: nbytes;
 }
-- 
2.34.1