Re: [PATCH next] sched,freezer: prevent tasks from escaping being frozen

[Peter Zijlstra <peterz@infradead.org>]

On Mon, Jul 07, 2025 at 12:02:47PM +0800, Chen Ridong wrote:

> >> And I'm not quite sure I understand this hunk either. If we bail out,
> >> current->__state is reset to TASK_RUNNING, so what's the problem?
> > 
> > The issue occurs in this race scenario:
> > 
> > echo FROZEN > freezer.state
> >   freeze_cgroup()
> >     freeze_task()
> >       fake_signal_wake_up() // wakes task to freeze it
> > 
> > In task context:
> > get_signal
> >   try_to_freeze
> >     __refrigerator
> >       WRITE_ONCE(current->__state, TASK_FROZEN); // set TASK_FROZEN
> >       // race: cgroup state updates to frozen

I suppose this is me not quite knowing how this cgroup freezer works;
how does it race? what code marks the task frozen?

> >       freezing(current) now return false
> >       // We bail out, the task is not frozen but it should be frozen.
> > 
> > I hope this explanation clarifies the issue I encountered.
> > 
> 
> Hi, Peter, Tim
> 
> I was looking at the WARN_ON_ONCE(freezing(p)) check in __thaw_task
> and started wondering: since we already have !frozen(p) check, is this
> warning still needed? If we can remove it, maybe reverting commit
> cff5f49d433f ("cgroup_freezer: cgroup_freezing: Check if not frozen")
> would be a better approach.

I suppose that is possible; modern sensibilities require we write that
function something like so:

void __thaw_task(struct task_struct *p)
{
	guard(spinlock_irqsave)(&freezer_lock);
	if (frozen(p) && !task_call_func(p, __restore_freezer_state, NULL))
		wake_up_state(p, TASK_FROZEN);
}