[PATCH 1/2] cgroup: fix spurious SIGKILL of CLONE_INTO_CGROUP children

[PATCH 1/2] cgroup: fix spurious SIGKILL of CLONE_INTO_CGROUP children

[Etienne Perot]

Since commit b69bb476dee9 ("cgroup: fix race between fork and
cgroup.kill"), the fork path snapshots the kill_seq of the child's
future cgroup into kargs->kill_seq, and cgroup_post_fork() SIGKILLs
the child if that cgroup's kill_seq has changed in the meantime, to
catch forks racing with a cgroup.kill sweep.

For CLONE_INTO_CGROUP, however, the snapshot in cgroup_css_set_fork()
is taken before the target cgroup has been resolved: kargs->cgrp is
always NULL at this point (it is only set at the end of the function).
So the "if (kargs->cgrp)" branch is dead code and the snapshot always
records the kill_seq of the parent's cgroup. cgroup_post_fork() then
compares it with the kill_seq of the target cgroup, so the child gets
SIGKILLed whenever the two cgroups have been killed a different number
of times.

As a result, once cgroup.kill has been written to a cgroup, every
child subsequently cloned into it with clone3(CLONE_INTO_CGROUP) is
killed on the spot, for as long as the cgroup exists: kill_seq is not
exposed to userspace and never resets.

Re-snapshot kill_seq from the target cgroup once it has been resolved,
and drop the dead branch at the early snapshot site.

This does not reopen the race fixed by b69bb476dee9. For
CLONE_INTO_CGROUP, everything from the snapshot to the check in
cgroup_post_fork() runs with cgroup_mutex held, and kill_seq is
only ever incremented under cgroup_mutex.

Fixes: b69bb476dee9 ("cgroup: fix race between fork and cgroup.kill")
Cc: stable@vger.kernel.org
Cc: Shakeel Butt <shakeel.butt@linux.dev>
Assisted-by: LLM
Signed-off-by: Etienne Perot <eperot@google.com>
---
 kernel/cgroup/cgroup.c | 6 ++----
 1 file changed, 2 insertions(+), 4 deletions(-)

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index c3a12fee7528..2d532bf2c0c7 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -6873,10 +6873,7 @@ static int cgroup_css_set_fork(struct kernel_clone_args *kargs)
 	spin_lock_irq(&css_set_lock);
 	cset = task_css_set(current);
 	get_css_set(cset);
-	if (kargs->cgrp)
-		kargs->kill_seq = kargs->cgrp->kill_seq;
-	else
-		kargs->kill_seq = cset->dfl_cgrp->kill_seq;
+	kargs->kill_seq = cset->dfl_cgrp->kill_seq;
 	spin_unlock_irq(&css_set_lock);
 
 	if (!(kargs->flags & CLONE_INTO_CGROUP)) {
@@ -6940,6 +6937,7 @@ static int cgroup_css_set_fork(struct kernel_clone_args *kargs)
 
 	put_css_set(cset);
 	kargs->cgrp = dst_cgrp;
+	kargs->kill_seq = dst_cgrp->kill_seq;
 	return ret;
 
 err:
-- 
2.55.0.897.gb25b4bd76c-goog

[PATCH 2/2] selftests/cgroup: test clone3() into a previously killed cgroup

[Etienne Perot]

Once cgroup.kill had been written to a cgroup, a stale kill_seq
snapshot (taken in cgroup_css_set_fork() before the target cgroup was
resolved) caused every child subsequently cloned into that cgroup with
clone3(CLONE_INTO_CGROUP) to be SIGKILLed on the spot.

Add a regression test: create a cgroup, kill it while it is empty,
then clone a child into it and check that the child runs and exits
cleanly. On a kernel without the fix, the test fails:

  not ok 4 test_cgkill_clone_into_killed

The test is skipped on kernels without clone3() or without
CLONE_INTO_CGROUP.

Cc: Shakeel Butt <shakeel.butt@linux.dev>
Assisted-by: LLM
Signed-off-by: Etienne Perot <eperot@google.com>
---
 tools/testing/selftests/cgroup/test_kill.c | 55 ++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/tools/testing/selftests/cgroup/test_kill.c b/tools/testing/selftests/cgroup/test_kill.c
index f6cd23a8ecc7..5ba8e285b2f3 100644
--- a/tools/testing/selftests/cgroup/test_kill.c
+++ b/tools/testing/selftests/cgroup/test_kill.c
@@ -7,6 +7,7 @@
 #include <stdlib.h>
 #include <string.h>
 #include <sys/types.h>
+#include <sys/wait.h>
 #include <unistd.h>
 
 #include "kselftest.h"
@@ -261,6 +262,59 @@ static int test_cgkill_forkbomb(const char *root)
 	return ret;
 }
 
+/*
+ * Test that a cgroup that was killed in the past can still be the target
+ * of clone3(CLONE_INTO_CGROUP): writing cgroup.kill must only kill the
+ * tasks in the cgroup at the time of the write, not tasks cloned into
+ * it afterwards.
+ */
+static int test_cgkill_clone_into_killed(const char *root)
+{
+	pid_t pid;
+	int cgroup_fd = -EBADF;
+	int ret = KSFT_FAIL;
+	char *cgroup = NULL;
+
+	cgroup = cg_name(root, "cg_test_clone_into_killed");
+	if (!cgroup)
+		goto cleanup;
+
+	if (cg_create(cgroup))
+		goto cleanup;
+
+	/* Kill the cgroup while it is still empty. */
+	if (cg_write(cgroup, "cgroup.kill", "1"))
+		goto cleanup;
+
+	cgroup_fd = dirfd_open_opath(cgroup);
+	if (cgroup_fd < 0)
+		goto cleanup;
+
+	pid = clone_into_cgroup(cgroup_fd);
+	if (pid < 0) {
+		if (errno == ENOSYS)
+			ret = KSFT_SKIP;
+		goto cleanup;
+	}
+
+	if (pid == 0)
+		exit(EXIT_SUCCESS);
+
+	/* The child must not be SIGKILLed; it has to exit cleanly. */
+	if (clone_reap(pid, WEXITED) != EXIT_SUCCESS)
+		goto cleanup;
+
+	ret = KSFT_PASS;
+
+cleanup:
+	if (cgroup_fd >= 0)
+		close(cgroup_fd);
+	if (cgroup)
+		cg_destroy(cgroup);
+	free(cgroup);
+	return ret;
+}
+
 #define T(x) { x, #x }
 struct cgkill_test {
 	int (*fn)(const char *root);
@@ -269,6 +323,7 @@ struct cgkill_test {
 	T(test_cgkill_simple),
 	T(test_cgkill_tree),
 	T(test_cgkill_forkbomb),
+	T(test_cgkill_clone_into_killed),
 };
 #undef T
 
-- 
2.55.0.897.gb25b4bd76c-goog

Re: [PATCH 1/2] cgroup: fix spurious SIGKILL of CLONE_INTO_CGROUP children

[Shakeel Butt]

On Fri, Aug 28, 2026 at 09:52:51PM +0000, Etienne Perot wrote:
> Since commit b69bb476dee9 ("cgroup: fix race between fork and
> cgroup.kill"), the fork path snapshots the kill_seq of the child's
> future cgroup into kargs->kill_seq, and cgroup_post_fork() SIGKILLs
> the child if that cgroup's kill_seq has changed in the meantime, to
> catch forks racing with a cgroup.kill sweep.
> 
> For CLONE_INTO_CGROUP, however, the snapshot in cgroup_css_set_fork()
> is taken before the target cgroup has been resolved: kargs->cgrp is
> always NULL at this point (it is only set at the end of the function).
> So the "if (kargs->cgrp)" branch is dead code and the snapshot always
> records the kill_seq of the parent's cgroup. cgroup_post_fork() then
> compares it with the kill_seq of the target cgroup, so the child gets
> SIGKILLed whenever the two cgroups have been killed a different number
> of times.
> 
> As a result, once cgroup.kill has been written to a cgroup, every
> child subsequently cloned into it with clone3(CLONE_INTO_CGROUP) is
> killed on the spot, for as long as the cgroup exists: kill_seq is not
> exposed to userspace and never resets.
> 
> Re-snapshot kill_seq from the target cgroup once it has been resolved,
> and drop the dead branch at the early snapshot site.
> 
> This does not reopen the race fixed by b69bb476dee9. For
> CLONE_INTO_CGROUP, everything from the snapshot to the check in
> cgroup_post_fork() runs with cgroup_mutex held, and kill_seq is
> only ever incremented under cgroup_mutex.

Thanks for catching this. Overall looks good. Can you please fix the comment
where kill_seq is defined in the header. Currently it says kill_seq is
serialized by css_set_lock. After your change, it should for normal fork it is
serialized by css_set_lock but for clone3(CLONE_INTO_CGROUP), it is serialized
by cgroup_mutex.

Orthogonally, we have plans to remove cgroup_mutex dependency from cgroup.kill,
so we will need to reevaluate this at that time.

> 
> Fixes: b69bb476dee9 ("cgroup: fix race between fork and cgroup.kill")
> Cc: stable@vger.kernel.org
> Cc: Shakeel Butt <shakeel.butt@linux.dev>
> Assisted-by: LLM
> Signed-off-by: Etienne Perot <eperot@google.com>
> ---
>  kernel/cgroup/cgroup.c | 6 ++----
>  1 file changed, 2 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
> index c3a12fee7528..2d532bf2c0c7 100644
> --- a/kernel/cgroup/cgroup.c
> +++ b/kernel/cgroup/cgroup.c
> @@ -6873,10 +6873,7 @@ static int cgroup_css_set_fork(struct kernel_clone_args *kargs)
>  	spin_lock_irq(&css_set_lock);
>  	cset = task_css_set(current);
>  	get_css_set(cset);
> -	if (kargs->cgrp)
> -		kargs->kill_seq = kargs->cgrp->kill_seq;
> -	else
> -		kargs->kill_seq = cset->dfl_cgrp->kill_seq;
> +	kargs->kill_seq = cset->dfl_cgrp->kill_seq;
>  	spin_unlock_irq(&css_set_lock);
>  
>  	if (!(kargs->flags & CLONE_INTO_CGROUP)) {
> @@ -6940,6 +6937,7 @@ static int cgroup_css_set_fork(struct kernel_clone_args *kargs)
>  
>  	put_css_set(cset);
>  	kargs->cgrp = dst_cgrp;
> +	kargs->kill_seq = dst_cgrp->kill_seq;
>  	return ret;
>  
>  err:
> -- 
> 2.55.0.897.gb25b4bd76c-goog
>