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: <CANkg5ezUe4--TyR5rTrr0LSeOcEAtnDaydGOmxQq5nvfeEpwwg@mail.gmail.com>
Date: Tue, 8 Jul 2025 09:35:58 -0600
From: Tim Van Patten <timvp@...gle.com>
To: Peter Zijlstra <peterz@...radead.org>
Cc: Chen Ridong <chenridong@...weicloud.com>, "Michal Koutn??" <mkoutny@...e.com>, rafael@...nel.org, 
	pavel@...nel.org, 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

I am (admittedly) more of a naive observer here than anything, and far
from an expert, but my
understanding matches what the code says:

* freezing(): Check if there is a request to freeze a process
* frozen(): Check if a process has been frozen

> Before this change, most callers of
> freezing(p) would receive true when the cgroup was frozen, whereas now they receive false.
>
> My concern is that the state where freezing(p) returns true (i.e., the cgroup is freezing but not
> yet frozen) should be transient. I'm not entirely sure whether all callers of freezing(p) (except
> __thaw_task) expect or handle this state correctly. Do the callers want the intermediate freezing
> state? I am not sure...

I agree that there's not a clear way to be certain what the caller's
intent was, so all we have is the code.
Based on that, my interpretation is that anyone calling freezing() is
interested in the transient state (there
was a request, but it hasn't completed yet) and any callers treating
freezing() as frozen() are incorrect.
They should be calling frozen() instead (or possibly also) based on
the documentation and naming.

> Given this, I'm considering whether we should revert commit cff5f49d433f, provided we can safely
> remove the WARN_ON_ONCE(freezing(p)) check in __thaw_task. I'd appreciate your thoughts on this
> approach.

At the end of the day my goal was to clean up the warning since it was
breaking tests, so if we remove that
as part of the revert then we're still in the same state. I disagree
that we should revert though, since the
code now matches the documentation and naming, in my opinion. However,
I'll have to defer to the experts
for the final decision, since they're ultimately responsible for any
changes here.