cgroups.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH v2 -next 0/2] sched,freezer: prevent tasks from escaping being frozen
@ 2025-07-17  8:55 Chen Ridong
  2025-07-17  8:55 ` [PATCH v2 -next 1/2] sched,freezer: Remove unnecessary warning in __thaw_task Chen Ridong
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Chen Ridong @ 2025-07-17  8:55 UTC (permalink / raw)
  To: tj, hannes, mkoutny, rafael, pavel, timvp
  Cc: cgroups, linux-kernel, linux-pm, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

This series fixes an issue where a task may escape freezing.

---

v2: remove the warning in the __thaw_task and revert cff5f49d433f

v1: https://lore.kernel.org/lkml/20250703133427.3301899-1-chenridong@huaweicloud.com/

Chen Ridong (2):
  sched,freezer: Remove unnecessary warning in __thaw_task
  Revert "cgroup_freezer: cgroup_freezing: Check if not frozen"

 kernel/cgroup/legacy_freezer.c |  8 +-------
 kernel/freezer.c               | 15 +++------------
 2 files changed, 4 insertions(+), 19 deletions(-)

-- 
2.34.1


^ permalink raw reply	[flat|nested] 7+ messages in thread

* [PATCH v2 -next 1/2] sched,freezer: Remove unnecessary warning in __thaw_task
  2025-07-17  8:55 [PATCH v2 -next 0/2] sched,freezer: prevent tasks from escaping being frozen Chen Ridong
@ 2025-07-17  8:55 ` Chen Ridong
  2025-07-17 18:26   ` Rafael J. Wysocki
  2025-07-17  8:55 ` [PATCH v2 -next 2/2] Revert "cgroup_freezer: cgroup_freezing: Check if not frozen" Chen Ridong
  2025-07-17 17:59 ` [PATCH v2 -next 0/2] sched,freezer: prevent tasks from escaping being frozen Tejun Heo
  2 siblings, 1 reply; 7+ messages in thread
From: Chen Ridong @ 2025-07-17  8:55 UTC (permalink / raw)
  To: tj, hannes, mkoutny, rafael, pavel, timvp
  Cc: cgroups, linux-kernel, linux-pm, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.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@huawei.com>
Signed-off-by: Chen Ridong <chenridong@huawei.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);
 }
 
 /**
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* [PATCH v2 -next 2/2] Revert "cgroup_freezer: cgroup_freezing: Check if not frozen"
  2025-07-17  8:55 [PATCH v2 -next 0/2] sched,freezer: prevent tasks from escaping being frozen Chen Ridong
  2025-07-17  8:55 ` [PATCH v2 -next 1/2] sched,freezer: Remove unnecessary warning in __thaw_task Chen Ridong
@ 2025-07-17  8:55 ` Chen Ridong
  2025-07-17 17:59 ` [PATCH v2 -next 0/2] sched,freezer: prevent tasks from escaping being frozen Tejun Heo
  2 siblings, 0 replies; 7+ messages in thread
From: Chen Ridong @ 2025-07-17  8:55 UTC (permalink / raw)
  To: tj, hannes, mkoutny, rafael, pavel, timvp
  Cc: cgroups, linux-kernel, linux-pm, lujialin4, chenridong

From: Chen Ridong <chenridong@huawei.com>

This reverts commit cff5f49d433fcd0063c8be7dd08fa5bf190c6c37.

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 the commit cff5f49d433f ("cgroup_freezer: cgroup_freezing: Check
if not frozen") to resolve the issue.

The warning has been removed in the previous patch. This patch revert the
commit cff5f49d433f ("cgroup_freezer: cgroup_freezing: Check if not
frozen") to complete the fix.

Fixes: cff5f49d433f ("cgroup_freezer: cgroup_freezing: Check if not frozen")
Reported-by: Zhong Jiawei<zhongjiawei1@huawei.com>
Signed-off-by: Chen Ridong <chenridong@huawei.com>
---
 kernel/cgroup/legacy_freezer.c | 8 +-------
 1 file changed, 1 insertion(+), 7 deletions(-)

diff --git a/kernel/cgroup/legacy_freezer.c b/kernel/cgroup/legacy_freezer.c
index 507b8f19a262..dd9417425d92 100644
--- a/kernel/cgroup/legacy_freezer.c
+++ b/kernel/cgroup/legacy_freezer.c
@@ -66,15 +66,9 @@ static struct freezer *parent_freezer(struct freezer *freezer)
 bool cgroup_freezing(struct task_struct *task)
 {
 	bool ret;
-	unsigned int state;
 
 	rcu_read_lock();
-	/* Check if the cgroup is still FREEZING, but not FROZEN. The extra
-	 * !FROZEN check is required, because the FREEZING bit is not cleared
-	 * when the state FROZEN is reached.
-	 */
-	state = task_freezer(task)->state;
-	ret = (state & CGROUP_FREEZING) && !(state & CGROUP_FROZEN);
+	ret = task_freezer(task)->state & CGROUP_FREEZING;
 	rcu_read_unlock();
 
 	return ret;
-- 
2.34.1


^ permalink raw reply related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 -next 0/2] sched,freezer: prevent tasks from escaping being frozen
  2025-07-17  8:55 [PATCH v2 -next 0/2] sched,freezer: prevent tasks from escaping being frozen Chen Ridong
  2025-07-17  8:55 ` [PATCH v2 -next 1/2] sched,freezer: Remove unnecessary warning in __thaw_task Chen Ridong
  2025-07-17  8:55 ` [PATCH v2 -next 2/2] Revert "cgroup_freezer: cgroup_freezing: Check if not frozen" Chen Ridong
@ 2025-07-17 17:59 ` Tejun Heo
  2 siblings, 0 replies; 7+ messages in thread
From: Tejun Heo @ 2025-07-17 17:59 UTC (permalink / raw)
  To: Chen Ridong
  Cc: hannes, mkoutny, rafael, pavel, timvp, cgroups, linux-kernel,
	linux-pm, lujialin4, chenridong

On Thu, Jul 17, 2025 at 08:55:48AM +0000, Chen Ridong wrote:
> From: Chen Ridong <chenridong@huawei.com>
> 
> This series fixes an issue where a task may escape freezing.

Applied 1-2 to cgroup/for-6.16-fixes.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 -next 1/2] sched,freezer: Remove unnecessary warning in __thaw_task
  2025-07-17  8:55 ` [PATCH v2 -next 1/2] sched,freezer: Remove unnecessary warning in __thaw_task Chen Ridong
@ 2025-07-17 18:26   ` Rafael J. Wysocki
  2025-07-17 19:09     ` Tejun Heo
  0 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2025-07-17 18:26 UTC (permalink / raw)
  To: Chen Ridong
  Cc: tj, hannes, mkoutny, rafael, pavel, timvp, cgroups, linux-kernel,
	linux-pm, lujialin4, chenridong

On Thu, Jul 17, 2025 at 11:09 AM Chen Ridong <chenridong@huaweicloud.com> wrote:
>
> From: Chen Ridong <chenridong@huawei.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@huawei.com>
> Signed-off-by: Chen Ridong <chenridong@huawei.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!

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 -next 1/2] sched,freezer: Remove unnecessary warning in __thaw_task
  2025-07-17 18:26   ` Rafael J. Wysocki
@ 2025-07-17 19:09     ` Tejun Heo
  2025-07-18 16:34       ` Rafael J. Wysocki
  0 siblings, 1 reply; 7+ messages in thread
From: Tejun Heo @ 2025-07-17 19:09 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: Chen Ridong, hannes, mkoutny, pavel, timvp, cgroups, linux-kernel,
	linux-pm, lujialin4, chenridong

On Thu, Jul 17, 2025 at 08:26:26PM +0200, Rafael J. Wysocki wrote:
> I can apply this one, but I'll need an ACK for the [2/2].

Ooh, I just applied the two patches as the original patch referenced went
through the cgroup tree. If you'd like to route it, I'd be happy to revert
them.

Thanks.

-- 
tejun

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 -next 1/2] sched,freezer: Remove unnecessary warning in __thaw_task
  2025-07-17 19:09     ` Tejun Heo
@ 2025-07-18 16:34       ` Rafael J. Wysocki
  0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2025-07-18 16:34 UTC (permalink / raw)
  To: Tejun Heo
  Cc: Rafael J. Wysocki, Chen Ridong, hannes, mkoutny, pavel, timvp,
	cgroups, linux-kernel, linux-pm, lujialin4, chenridong

On Thu, Jul 17, 2025 at 9:09 PM Tejun Heo <tj@kernel.org> wrote:
>
> On Thu, Jul 17, 2025 at 08:26:26PM +0200, Rafael J. Wysocki wrote:
> > I can apply this one, but I'll need an ACK for the [2/2].
>
> Ooh, I just applied the two patches as the original patch referenced went
> through the cgroup tree.

No worries then, I'll just leave them to you.

> If you'd like to route it, I'd be happy to revert them.

More work?  Nah.

^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2025-07-18 16:34 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-07-17  8:55 [PATCH v2 -next 0/2] sched,freezer: prevent tasks from escaping being frozen Chen Ridong
2025-07-17  8:55 ` [PATCH v2 -next 1/2] sched,freezer: Remove unnecessary warning in __thaw_task Chen Ridong
2025-07-17 18:26   ` Rafael J. Wysocki
2025-07-17 19:09     ` Tejun Heo
2025-07-18 16:34       ` Rafael J. Wysocki
2025-07-17  8:55 ` [PATCH v2 -next 2/2] Revert "cgroup_freezer: cgroup_freezing: Check if not frozen" Chen Ridong
2025-07-17 17:59 ` [PATCH v2 -next 0/2] sched,freezer: prevent tasks from escaping being frozen Tejun Heo

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).