linux-kernel - Re: [PATCH] cpuidle: warn and fixup on sanity check instead of rejecting the driver

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: <CAJZ5v0h2u_Y8H-mCzeX+gxT4Aipa_MK6egMaA2fGuGkJBGXq7Q@mail.gmail.com>
Date: Fri, 21 Nov 2025 14:23:32 +0100
From: "Rafael J. Wysocki" <rafael@...nel.org>
To: Artem Bityutskiy <artem.bityutskiy@...ux.intel.com>
Cc: Val Packett <val@...kett.cool>, "Rafael J. Wysocki" <rafael@...nel.org>, 
	Daniel Lezcano <daniel.lezcano@...aro.org>, Christian Loehle <christian.loehle@....com>, 
	linux-arm-kernel@...ts.infradead.org, linux-arm-msm@...r.kernel.org, 
	"Rafael J. Wysocki" <rafael.j.wysocki@...el.com>, linux-pm@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] cpuidle: warn and fixup on sanity check instead of
 rejecting the driver

On Fri, Nov 21, 2025 at 2:16 PM Artem Bityutskiy
<artem.bityutskiy@...ux.intel.com> wrote:
>
> On Thu, 2025-11-20 at 22:06 -0300, Val Packett wrote:
> > On Device Tree platforms, the latency and target residency values come
> > directly from device trees, which are numerous and weren't all written
> > with cpuidle invariants in mind. For example, qcom/hamoa.dtsi currently
> > trips this check: exit latency 680000 > residency 600000.
> >
> > Instead of harshly rejecting the entire cpuidle driver with a mysterious
> > error message, print a warning and set the target residency value to be
> > equal to the exit latency.
> >
> > Fixes: 76934e495cdc ("cpuidle: Add sanity check for exit latency and target residency")
> > Signed-off-by: Val Packett <val@...kett.cool>
> > ---
> >  drivers/cpuidle/driver.c | 7 +++++--
> >  1 file changed, 5 insertions(+), 2 deletions(-)
> >
> > diff --git a/drivers/cpuidle/driver.c b/drivers/cpuidle/driver.c
> > index 1c295a93d582..06aeb59c1017 100644
> > --- a/drivers/cpuidle/driver.c
> > +++ b/drivers/cpuidle/driver.c
> > @@ -199,8 +199,11 @@ static int __cpuidle_driver_init(struct cpuidle_driver *drv)
> >                * exceed its target residency which is assumed in cpuidle in
> >                * multiple places.
> >                */
> > -             if (s->exit_latency_ns > s->target_residency_ns)
> > -                     return -EINVAL;
> > +             if (s->exit_latency_ns > s->target_residency_ns) {
> > +                     pr_warn("cpuidle: state %d: exit latency %lld > residency %lld (fixing)\n",
> > +                             i, s->exit_latency_ns, s->target_residency_ns);
> > +                     s->target_residency_ns = s->exit_latency_ns;
> > +             }
> >       }
>
> Ideally, in a perfect world, driver.c should verify input data and
> reject bad input, rather than correct bad input.
>
> So ideally, if there is an idle driver between DT and driver.c (like
> intel_idle.c in case of Intel), that would be its job to correct DT
> data.
>
> But I'm not familiar with DT platforms, so I don't know if there is a
> driver/piece of SW between DT parsing and driver.c that could handle
> this correction at an earlier stage.
>
> The reason I think this patch is not ideal is because it changes the
> input data at the core framework level, and in theory the change may be
> surprising to users. In general, sometimes rejecting bluntly is better
> than correcting in a possibly unexpected way.

Unless rejecting it causes the functionality to be missing entirely
and users have no straightforward way to fix it up.

As I said in my reply, what can be done in this situation is to print
a warning when assumptions are not met.