[PATCH] cgroup: Avoid iteration of dying tasks with zero refcount

[PATCH] cgroup: Avoid iteration of dying tasks with zero refcount

[Michal Koutný]

The commit 260fbcb92bbea ("cgroup: Move dying_tasks cleanup from
cgroup_task_release() to cgroup_task_free()") extended the lifetime of
tasks on the dying_tasks list.
The iterators have provision to go through dying_tasks because of
dying threadgroup leaders or explicit CSS_TASK_ITER_WITH_DEAD, however,
it was expected that such tasks can obtain a new reference (that is
possible before cgroup_task_release()/put_task_struct_rcu_user()).
The tasks after cgroup_task_release() and before cgroup_task_free()
are subject to race when they may or may not have usage count > 0.
The iterator should not attempt to resurrect tasks whose usage count
dropped to zero. (When that happens, __put_task_struct_rcu_cb() is
already imminent and the returned task_struct would could be used
after free.)

As a band-aid, filter tasks returned from css_task_iter_next() only to
those that have positive usage count (whose reference's lifetime can be
extended).

Rough illustration of the possible race

  R (reader of cgroup.procs)         T (thread)                       L (group leader)
  ---------------------------------  -------------------------------- --------------------------------
                                                                      L exits, signal->live > 0
                                                                      cgroup_task_dead(L)
                                                                        css_set_skip_task_iters() // skips only cset->tasks
                                                                        list_add_tail(&L->cg_list, &cset->dying_tasks)
  css_task_iter_next()
    take css_set_lock
    css_task_iter_advance()
      leader && signal->live != 0
      => it->task_pos = &L->cg_list
                                     T exits
                                     --signal->live == 0
                                     release_task(T)
                                       cgroup_task_release(T)
                                       release_task(L) // zap_leader
                                         cgroup_task_release(L)
                                         put_task_struct_rcu_user(L)
                                         ...RCU...
                                         put_task_struct(L)
                                           L->usage = 0
                                           /* L still on dying_tasks */
                                           ...RCU...
                                           __put_task_struct(L)
    it->task_pos = &L->cg_list
    get_task_struct(L)
      => addition on 0
    drop css_set_lock
                                           cgroup_task_free(L)
                                             css_set_skip_task_iters() // dying skip comes too late
                                           free_task(L)
  cgroup_procs_show()
    task_pid_vnr(L)

cgroup_task_release() is not synced via css_set_lock hence the race
possibility. (I'm not 100% convinced about this LLM-assisted
interleaving, multiple css_task_iter_next() calls may be involved with
css_set_lock released.)

Fixes: 260fbcb92bbea ("cgroup: Move dying_tasks cleanup from cgroup_task_release() to cgroup_task_free()")
Cc: stable@vger.kernel.org # v6.19+
Link: https://lists.debian.org/debian-kernel/2026/08/msg00220.html
Reported-by: Noah Elias Feldt <N.Feldt@mittwald.de>
Reported-by: Salvatore Bonaccorso <carnil@debian.org>
Signed-off-by: Michal Koutný <mkoutny@suse.com>
---
 kernel/cgroup/cgroup.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

Salvatore,
I was able to trigger the issue with your reproducer on 7.1.8
(occassionally, not always). With the patch on top of v7.3-rc1, I cannot
reproduce it (different base though). If you can confirm that in your
env, it'd be good.

Thanks,
Michal

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index c3a12fee7528f..112d68fe28778 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5303,10 +5303,13 @@ struct task_struct *css_task_iter_next(struct css_task_iter *it)
 	if (it->flags & CSS_TASK_ITER_SKIPPED)
 		css_task_iter_advance(it);
 
-	if (it->task_pos) {
+	while (it->task_pos && !it->cur_task) {
 		it->cur_task = list_entry(it->task_pos, struct task_struct,
 					  cg_list);
-		get_task_struct(it->cur_task);
+		/* a task on dying_tasks with zero refcount is only valid for
+		 * RCU readers, not even interesting for
+		 * CSS_TASK_ITER_WITH_DEAD, find another one */
+		it->cur_task = tryget_task_struct(it->cur_task);
 		css_task_iter_advance(it);
 	}
 
-- 
2.55.0

Re: [PATCH] cgroup: Avoid iteration of dying tasks with zero refcount

[Tejun Heo]

How about something like the following? It's more in line with other skips
and the resulting behavior should remain the same as before:

--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5215,6 +5215,7 @@
  */
 struct task_struct *css_task_iter_next(struct css_task_iter *it)
 {
+	struct task_struct *task;
 	unsigned long irqflags;
 
 	if (it->cur_task) {
@@ -5228,6 +5229,21 @@
 	if (it->flags & CSS_TASK_ITER_SKIPPED)
 		css_task_iter_advance(it);
 
+	/*
+	 * @it->task_pos was picked on an earlier call. A dying leader stays on
+	 * dying_tasks until cgroup_task_free(), past its last usage ref drop,
+	 * so it may have been reaped since and get_task_struct() on it would
+	 * resurrect a task about to be freed. That last ref is dropped by an
+	 * RCU callback queued from release_task(), after signal->live hit zero,
+	 * so a leader still showing live threads in this irq-disabled section
+	 * can't lose its ref before the section ends.
+	 */
+	if (it->task_pos && it->cur_tasks_head == &it->cur_cset->dying_tasks) {
+		task = list_entry(it->task_pos, struct task_struct, cg_list);
+		if (!atomic_read(&task->signal->live))
+			css_task_iter_advance(it);
+	}
+
 	if (it->task_pos) {
 		it->cur_task = list_entry(it->task_pos, struct task_struct,
 					  cg_list);

Re: [PATCH] cgroup: Avoid iteration of dying tasks with zero refcount

[Salvatore Bonaccorso]

Hi Michal,

On Wed, Sep 02, 2026 at 06:16:52PM +0200, Michal Koutný wrote:
> The commit 260fbcb92bbea ("cgroup: Move dying_tasks cleanup from
> cgroup_task_release() to cgroup_task_free()") extended the lifetime of
> tasks on the dying_tasks list.
> The iterators have provision to go through dying_tasks because of
> dying threadgroup leaders or explicit CSS_TASK_ITER_WITH_DEAD, however,
> it was expected that such tasks can obtain a new reference (that is
> possible before cgroup_task_release()/put_task_struct_rcu_user()).
> The tasks after cgroup_task_release() and before cgroup_task_free()
> are subject to race when they may or may not have usage count > 0.
> The iterator should not attempt to resurrect tasks whose usage count
> dropped to zero. (When that happens, __put_task_struct_rcu_cb() is
> already imminent and the returned task_struct would could be used
> after free.)
> 
> As a band-aid, filter tasks returned from css_task_iter_next() only to
> those that have positive usage count (whose reference's lifetime can be
> extended).
> 
> Rough illustration of the possible race
> 
>   R (reader of cgroup.procs)         T (thread)                       L (group leader)
>   ---------------------------------  -------------------------------- --------------------------------
>                                                                       L exits, signal->live > 0
>                                                                       cgroup_task_dead(L)
>                                                                         css_set_skip_task_iters() // skips only cset->tasks
>                                                                         list_add_tail(&L->cg_list, &cset->dying_tasks)
>   css_task_iter_next()
>     take css_set_lock
>     css_task_iter_advance()
>       leader && signal->live != 0
>       => it->task_pos = &L->cg_list
>                                      T exits
>                                      --signal->live == 0
>                                      release_task(T)
>                                        cgroup_task_release(T)
>                                        release_task(L) // zap_leader
>                                          cgroup_task_release(L)
>                                          put_task_struct_rcu_user(L)
>                                          ...RCU...
>                                          put_task_struct(L)
>                                            L->usage = 0
>                                            /* L still on dying_tasks */
>                                            ...RCU...
>                                            __put_task_struct(L)
>     it->task_pos = &L->cg_list
>     get_task_struct(L)
>       => addition on 0
>     drop css_set_lock
>                                            cgroup_task_free(L)
>                                              css_set_skip_task_iters() // dying skip comes too late
>                                            free_task(L)
>   cgroup_procs_show()
>     task_pid_vnr(L)
> 
> cgroup_task_release() is not synced via css_set_lock hence the race
> possibility. (I'm not 100% convinced about this LLM-assisted
> interleaving, multiple css_task_iter_next() calls may be involved with
> css_set_lock released.)
> 
> Fixes: 260fbcb92bbea ("cgroup: Move dying_tasks cleanup from cgroup_task_release() to cgroup_task_free()")
> Cc: stable@vger.kernel.org # v6.19+
> Link: https://lists.debian.org/debian-kernel/2026/08/msg00220.html
> Reported-by: Noah Elias Feldt <N.Feldt@mittwald.de>
> Reported-by: Salvatore Bonaccorso <carnil@debian.org>
> Signed-off-by: Michal Koutný <mkoutny@suse.com>
> ---
>  kernel/cgroup/cgroup.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> Salvatore,
> I was able to trigger the issue with your reproducer on 7.1.8
> (occassionally, not always). With the patch on top of v7.3-rc1, I cannot
> reproduce it (different base though). If you can confirm that in your
> env, it'd be good.

I tested it with the additional poc which Noah did sent to us (not
public) and with your patch it survives. 

Tested-by: Salvatore Bonaccorso <carnil@debian.org>

Regards,
Salvatore