[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
> 

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

[Tejun Heo]

Hello,

Applied 1-2 to cgroup/for-7.3-fixes with the comment above kill_seq
updated as Shakeel suggested:

--- a/include/linux/cgroup-defs.h
+++ b/include/linux/cgroup-defs.h
@@ -527,7 +527,10 @@ struct cgroup {

 	int nr_threaded_children;	/* # of live threaded child cgroups */

-	/* sequence number for cgroup.kill, serialized by css_set_lock. */
+	/*
+	 * Sequence number for cgroup.kill. Incremented with both cgroup_mutex
+	 * and css_set_lock held. Readers hold either one.
+	 */
 	unsigned int kill_seq;

 	struct kernfs_node *kn;		/* cgroup kernfs entry */

Thanks.

-- 
tejun

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

[Etienne Perot]

Thanks Tejun, and thanks Shakeel for the review.

- Etienne

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

[Daniel Golle]

On Fri, Aug 28, 2026 at 09:52:51PM +0000, Etienne Perot wrote:
> 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).

We hit this in OpenWrt and reached the same conclusion independently
before finding your patch, so here is a second data point from a real
workload.

The symptom is that re-creating an OCI container fails. procd's service
supervisor writes cgroup.kill to /sys/fs/cgroup/services/<service>/<name>
when a jailed instance exits, and leaves the directory in place. The next
generation of the same instance is started into that same leaf and execs
ujail, which clone3()s the container init with CLONE_INTO_CGROUP into a
freshly created cgroup under /sys/fs/cgroup/containers/. The container
init is SIGKILLed before it executes a single instruction, and every
subsequent attempt fails identically for as long as the services leaf
lives.

Instrumenting the child confirmed it never reaches its first statement
after clone3(). What isolated it was that rmdir() of the services leaf
followed immediately by mkdir() of the same path at the same mode makes
the failure disappear, while an unrelated cgroup operation in the same
window does not, and while the leaf's attributes are byte for byte
identical to a freshly created one. That pointed at per-cgroup state
exposed in no file, and the snapshot site then explained it: on 6.18.52
the capture is at cgroup.c:6740 while kargs->cgrp is only assigned at
:6803, so the else branch is always taken and cgroup_post_fork() ends up
comparing two independent counters.

Backporting this patch to 6.18.52 fixes it. With no userspace change at
all, and with the killed cgroup still deliberately left in place, three
consecutive create attempts that previously failed now succeed:

  before: create rc=251, container spuriously left running  (3/3)
  after:  create rc=0,   container correctly left created   (3/3)

Your selftest in 2/2 reproduces it on the same machine, and behaves
exactly as your commit message says:

  6.18.52 without 1/2:   ok 1 test_cgkill_simple
                         ok 2 test_cgkill_tree
                         ok 3 test_cgkill_forkbomb
                         not ok 4 test_cgkill_clone_into_killed

  6.18.52 with 1/2:      ok 1 test_cgkill_simple
                         ok 2 test_cgkill_tree
                         ok 3 test_cgkill_forkbomb
                         ok 4 test_cgkill_clone_into_killed

Tested on x86_64, kernel 6.18.52, with procd/ujail as the OCI runtime;
the two kernels differ only by 1/2.

Reviewed-by: Daniel Golle <daniel@makrotopia.org>
Tested-by: Daniel Golle <daniel@makrotopia.org>

One request about stable, and apologies if this is simply a matter of
timing. The patch is in mainline from v7.3-rc2 and carries Cc: stable,
but as of 6.18.53 it is not in linux-6.18.y yet. Given that the failure
is silent from userspace, the child dies with no diagnostic and kill_seq
is visible nowhere, it would be worth queueing for 6.18.y and the other
branches carrying b69bb476dee9 ahead of the usual post-release sweep.
We are carrying it as a local backport in the meantime.

Thanks for tracking this down.