[PATCH v2] 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.)

To avoid dispatching such tasks through the iterator, recheck the
threadgroup's size (follows same logic we have at other places) and skip
such tasks on the dying_tasks list.

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
    release css_set_lock
                                     T exits 					[A]
                                     --signal->live == 0  // we can check this
				     cgroup_task_dead(T) // css_set_lock	[C]
                                     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)
  css_task_iter_next() // another iteration					[B]
    take css_set_lock
    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@xxxxxxxxxxxxxxx # v6.19+
Link: https://lists.debian.org/debian-kernel/2026/08/msg00220.html
Suggested-by: Tejun Heo <tj@xxxxxxxxxx>
Reported-by: Noah Elias Feldt <N.Feldt@xxxxxxxxxxx>
Reported-by: Salvatore Bonaccorso <carnil@xxxxxxxxxx>
Tested-by: Noah Elias Feldt <N.Feldt@xxxxxxxxxxx> # if-variant
Signed-off-by: Michal Koutný <mkoutny@xxxxxxxx>
---
 kernel/cgroup/cgroup.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

Changes from v1 (https://lore.kernel.org/r/20260902161653.1051794-1-mkoutny@xxxxxxxx)
- check task->signal->live instead of t->usage
- add css_set_lock release to sequence diagram

My comments to the signal-live variant.

First, I replaced the if() with a while() loop because when one such
dying leader could remain on dying_tasks, there can be more of them
(matter of effort) and single css_task_iter_advance() won't be enough
(we need to skip all such tasks before get_task_struct()).

<del>Second, if [B] races at moment of [A] (signal->live still positive)
and before [C], usage can still drop to zero.</del> Nothing, given the
ordering, positive signal->live implies positive L->usage so the
proposed check should be OK.

Thanks,
Michal

diff --git a/kernel/cgroup/cgroup.c b/kernel/cgroup/cgroup.c
index c3a12fee7528f..d3069a5e9185d 100644
--- a/kernel/cgroup/cgroup.c
+++ b/kernel/cgroup/cgroup.c
@@ -5290,6 +5290,7 @@ void css_task_iter_start(struct cgroup_subsys_state *css, unsigned int flags,
  */
 struct task_struct *css_task_iter_next(struct css_task_iter *it)
 {
+	struct task_struct *task;
 	unsigned long irqflags;
 
 	if (it->cur_task) {
@@ -5303,6 +5304,21 @@ struct task_struct *css_task_iter_next(struct css_task_iter *it)
 	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.
+	 */
+	while (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);
-- 
2.55.0