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

[Salvatore Bonaccorso <carnil@debian.org>]

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