[PATCH] cgroup: serialize migrations of newly forked processes

[PATCH] cgroup: serialize migrations of newly forked processes

[Daehyeon Ko]

With favordynmods enabled, a cgroup.procs migration takes the target
threadgroup's cgroup_threadgroup_rwsem for writing. A process fork holds
the forking threadgroup's semaphore for reading. These are different
semaphores when fork creates a new process.

copy_process() publishes the child's PID before cgroup_post_fork() links
the child to its css_set. A writer can find the child in that interval,
take its uncontended semaphore, and migrate it while child->cgroups still
points to init_css_set and child->cg_list is empty. Migration links the
node into init_css_set.mg_tasks. cgroup_post_fork() then adds the same
node to the intended css_set as though it were unlinked, corrupting both
task lists and their css_set ownership.

This regresses the fork/migration race fixed by commit b12b533fa523
("cgroups: add lock for child->cgroups in cgroup_post_fork()") and
restored again in 2012 after the lock had been removed. The old task_lock
protection was later superseded by global css_set and cgroup threadgroup
locking. The change identified by the Fixes tag narrowed only the migration
side back to a per-threadgroup lock, recreating the different-process
exclusion failure.

Take the new threadgroup's semaphore for reading before the child becomes
visible and hold it through cgroup_post_fork(). Use a nested lock subclass
because fork already holds the parent's semaphore. CLONE_THREAD children
share the parent's signal_struct and remain covered by the existing lock.
Migrations of unrelated, fully initialized threadgroups remain concurrent.

An AI coding assistant was used to identify and reproduce the regression
and to develop and test this fix.

Fixes: 0568f89d4fb8 ("cgroup: replace global percpu_rwsem with per threadgroup resem when writing to cgroup.procs")
Cc: stable@vger.kernel.org # 6.18+
Signed-off-by: Daehyeon Ko <4ncienth@gmail.com>
---
 kernel/cgroup/cgroup.c | 28 ++++++++++++++++++++++++++++
 1 file changed, 28 insertions(+)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index f534c0473a469..faccc793325e5 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6880,6 +6880,31 @@ static struct cgroup *cgroup_get_from_file(struct file *f)
 	return cgrp;
 }
 
+/*
+ * A new threadgroup gets its own cgroup_threadgroup_rwsem.  Once its PID is
+ * visible, a cgroup.procs writer can find the child and lock that rwsem even
+ * though cgroup_post_fork() has not linked the child to its css_set yet.
+ *
+ * cgroup_can_fork() already holds the global read lock, which keeps
+ * cgroup_enable_per_threadgroup_rwsem stable.  Lock the child's rwsem as a
+ * nested instance until cgroup_post_fork() finishes.  Threads share their
+ * parent's signal_struct and are covered by the existing read lock.
+ */
+static void cgroup_fork_child_lock(struct task_struct *child)
+{
+	if (cgroup_enable_per_threadgroup_rwsem &&
+	    child->signal != current->signal)
+		down_read_nested(&child->signal->cgroup_threadgroup_rwsem,
+				 SINGLE_DEPTH_NESTING);
+}
+
+static void cgroup_fork_child_unlock(struct task_struct *child)
+{
+	if (cgroup_enable_per_threadgroup_rwsem &&
+	    child->signal != current->signal)
+		up_read(&child->signal->cgroup_threadgroup_rwsem);
+}
+
 /**
  * cgroup_css_set_fork - find or create a css_set for a child process
  * @kargs: the arguments passed to create the child process
@@ -7045,6 +7070,7 @@ int cgroup_can_fork(struct task_struct *child, struct kernel_clone_args *kargs)
 			goto out_revert;
 	} while_each_subsys_mask();
 
+	cgroup_fork_child_lock(child);
 	return 0;
 
 out_revert:
@@ -7079,6 +7105,7 @@ void cgroup_cancel_fork(struct task_struct *child,
 		if (ss->cancel_fork)
 			ss->cancel_fork(child, kargs->cset);
 
+	cgroup_fork_child_unlock(child);
 	cgroup_css_set_put_fork(kargs);
 }
 
@@ -7176,6 +7203,7 @@ void cgroup_post_fork(struct task_struct *child,
 	if (unlikely(kill))
 		do_send_sig_info(SIGKILL, SEND_SIG_NOINFO, child, PIDTYPE_TGID);
 
+	cgroup_fork_child_unlock(child);
 	cgroup_css_set_put_fork(kargs);
 }
 

Re: [PATCH] cgroup: serialize migrations of newly forked processes

[Tejun Heo]

Applied to cgroup/for-7.4-fixes.

Thanks.

--
tejun