linux-kernel - Re: [PATCH v2 -next 1/2] sched,freezer: Remove unnecessary warning in __thaw

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: <CAJZ5v0jnUQJSBbYWfmGb6wr_CY3j61TSq_cR9KBMqtvxWrmsaA@mail.gmail.com>
Date: Thu, 17 Jul 2025 20:26:26 +0200
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Chen Ridong <chenridong@...weicloud.com>
Cc: tj@...nel.org, hannes@...xchg.org, mkoutny@...e.com, rafael@...nel.org, 
	pavel@...nel.org, timvp@...gle.com, cgroups@...r.kernel.org, 
	linux-kernel@...r.kernel.org, linux-pm@...r.kernel.org, lujialin4@...wei.com, 
	chenridong@...wei.com
Subject: Re: [PATCH v2 -next 1/2] sched,freezer: Remove unnecessary warning in __thaw_task

On Thu, Jul 17, 2025 at 11:09 AM Chen Ridong <chenridong@...weicloud.com> wrote:
>
> From: Chen Ridong <chenridong@...wei.com>
>
> Commit cff5f49d433f ("cgroup_freezer: cgroup_freezing: Check if not
> frozen") modified the cgroup_freezing() logic to verify that the FROZEN
> flag is not set, affecting the return value of the freezing() function,
> in order to address a warning in __thaw_task.
>
> A race condition exists that may allow tasks to escape being frozen. The
> following scenario demonstrates this issue:
>
> CPU 0 (get_signal path)         CPU 1 (freezer.state reader)
> try_to_freeze                   read freezer.state
> __refrigerator                  freezer_read
>                                 update_if_frozen
> WRITE_ONCE(current->__state, TASK_FROZEN);
>                                 ...
>                                 /* Task is now marked frozen */
>                                 /* frozen(task) == true */
>                                 /* Assuming other tasks are frozen */
>                                 freezer->state |= CGROUP_FROZEN;
> /* freezing(current) returns false */
> /* because cgroup is frozen (not freezing) */
> break out
> __set_current_state(TASK_RUNNING);
> /* Bug: Task resumes running when it should remain frozen */
>
> The existing !frozen(p) check in __thaw_task makes the
> WARN_ON_ONCE(freezing(p)) warning redundant. Removing this warning enables
> reverting commit cff5f49d433f ("cgroup_freezer: cgroup_freezing: Check if
> not frozen") to resolve the issue.
>
> This patch removes the warning from __thaw_task. A subsequent patch will
> revert commit cff5f49d433f ("cgroup_freezer: cgroup_freezing: Check if
> not frozen") to complete the fix.
>
> Reported-by: Zhong Jiawei<zhongjiawei1@...wei.com>
> Signed-off-by: Chen Ridong <chenridong@...wei.com>
> ---
>  kernel/freezer.c | 15 +++------------
>  1 file changed, 3 insertions(+), 12 deletions(-)
>
> diff --git a/kernel/freezer.c b/kernel/freezer.c
> index 8d530d0949ff..6a96149aede9 100644
> --- a/kernel/freezer.c
> +++ b/kernel/freezer.c
> @@ -201,18 +201,9 @@ static int __restore_freezer_state(struct task_struct *p, void *arg)
>
>  void __thaw_task(struct task_struct *p)
>  {
> -       unsigned long flags;
> -
> -       spin_lock_irqsave(&freezer_lock, flags);
> -       if (WARN_ON_ONCE(freezing(p)))
> -               goto unlock;
> -
> -       if (!frozen(p) || task_call_func(p, __restore_freezer_state, NULL))
> -               goto unlock;
> -
> -       wake_up_state(p, TASK_FROZEN);
> -unlock:
> -       spin_unlock_irqrestore(&freezer_lock, flags);
> +       guard(spinlock_irqsave)(&freezer_lock);
> +       if (frozen(p) && !task_call_func(p, __restore_freezer_state, NULL))
> +               wake_up_state(p, TASK_FROZEN);
>  }
>
>  /**
> --

I can apply this one, but I'll need an ACK for the [2/2].

Thanks!