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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <85fc85e8-af92-4d58-8271-9bf4aeb0a63d@huaweicloud.com>
Date: Fri, 4 Jul 2025 18:25:12 +0800
From: Chen Ridong <chenridong@...weicloud.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Michal Koutn?? <mkoutny@...e.com>, rafael@...nel.org, pavel@...nel.org,
 timvp@...gle.com, tj@...nel.org, linux-pm@...r.kernel.org,
 linux-kernel@...r.kernel.org, lujialin4@...wei.com, chenridong@...wei.com
Subject: Re: [PATCH next] sched,freezer: prevent tasks from escaping being
 frozen



On 2025/7/4 15:57, Peter Zijlstra wrote:
> On Fri, Jul 04, 2025 at 11:11:52AM +0800, Chen Ridong wrote:
> 
> Your patches are mangled; please educate your MUA.
> 
Hi Peter,

Thank you for your review and feedback
Apologies for the formatting issues in the patch.

>> --- a/kernel/freezer.c
>> +++ b/kernel/freezer.c
>> @@ -71,19 +71,20 @@ bool __refrigerator(bool check_kthr_stop)
>>         for (;;) {
>>                 bool freeze;
>>
>> -               raw_spin_lock_irq(&current->pi_lock);
>> -               WRITE_ONCE(current->__state, TASK_FROZEN);
>> -               /* unstale saved_state so that __thaw_task() will wake
>> us up */
>> -               current->saved_state = TASK_RUNNING;
>> -               raw_spin_unlock_irq(&current->pi_lock);
>> -
>>                 spin_lock_irq(&freezer_lock);
>> -               freeze = freezing(current) && !(check_kthr_stop &&
>> kthread_should_stop());
>> +               freeze = (freezing(current) || !cgroup_thawed(current))
>> +                        && !(check_kthr_stop && kthread_should_stop());
> 
> This makes no sense to me; why can't this stay in cgroup_freezing()?
> 
> Also, can someone please fix that broken comment style there.
> 
The change relates to commit cff5f49d433f ("cgroup_freezer: cgroup_freezing: Check if not frozen"),
which modified cgroup_freezing() to verify the FROZEN flag isn't set. The freezing(p) will return
false if the cgroup is frozen.

>>                 spin_unlock_irq(&freezer_lock);
>>
>>                 if (!freeze)
>>                         break;
>>
>> +               raw_spin_lock_irq(&current->pi_lock);
>> +               WRITE_ONCE(current->__state, TASK_FROZEN);
>> +               /* unstale saved_state so that __thaw_task() will wake
>> us up */
>> +               current->saved_state = TASK_RUNNING;
>> +               raw_spin_unlock_irq(&current->pi_lock);
>> +
> 
> 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
      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.

Here's the corrected format patch:

diff --git a/kernel/freezer.c b/kernel/freezer.c
index 8d530d0949ff..16e98a6b497a 100644
--- a/kernel/freezer.c
+++ b/kernel/freezer.c
@@ -71,19 +71,20 @@ bool __refrigerator(bool check_kthr_stop)
        for (;;) {
                bool freeze;

-               raw_spin_lock_irq(&current->pi_lock);
-               WRITE_ONCE(current->__state, TASK_FROZEN);
-               /* unstale saved_state so that __thaw_task() will wake us up */
-               current->saved_state = TASK_RUNNING;
-               raw_spin_unlock_irq(&current->pi_lock);
-
                spin_lock_irq(&freezer_lock);
-               freeze = freezing(current) && !(check_kthr_stop && kthread_should_stop());
+               freeze = (freezing(current) || !cgroup_thawed(current))
+                        && !(check_kthr_stop && kthread_should_stop());
                spin_unlock_irq(&freezer_lock);

                if (!freeze)
                        break;

+               raw_spin_lock_irq(&current->pi_lock);
+               WRITE_ONCE(current->__state, TASK_FROZEN);
+               /* unstale saved_state so that __thaw_task() will wake us up */
+               current->saved_state = TASK_RUNNING;
+               raw_spin_unlock_irq(&current->pi_lock);
+
                was_frozen = true;
                schedule();
        }

Best regards,
Ridong