linux-kernel - Re: [PATCH] cpufreq: Disallow ->resolve_freq() for drivers providing ->target

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] [day] [month] [year] [list]

Message-ID: <CAJZ5v0jW=RHuOrJcw4+bvxLHP+v=FRksdgVE=qOnwSVcvQPEzw@mail.gmail.com>
Date:	Thu, 21 Jul 2016 23:37:12 +0200
From:	"Rafael J. Wysocki" <rafael@...nel.org>
To:	Viresh Kumar <viresh.kumar@...aro.org>
Cc:	Rafael Wysocki <rjw@...ysocki.net>,
	Lists linaro-kernel <linaro-kernel@...ts.linaro.org>,
	Linux PM <linux-pm@...r.kernel.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
	Steve Muckle <steve.muckle@...aro.org>,
	Peter Zijlstra <peterz@...radead.org>,
	Ingo Molnar <mingo@...hat.com>,
	Vincent Guittot <vincent.guittot@...aro.org>,
	Morten Rasmussen <morten.rasmussen@....com>,
	Dietmar Eggemann <dietmar.eggemann@....com>,
	Juri Lelli <Juri.Lelli@....com>,
	Patrick Bellasi <patrick.bellasi@....com>
Subject: Re: [PATCH] cpufreq: Disallow ->resolve_freq() for drivers providing ->target_index()

On Thu, Jul 21, 2016 at 11:23 PM, Viresh Kumar <viresh.kumar@...aro.org> wrote:
> The handlers provided by cpufreq core are sufficient for resolving the
> frequency for drivers providing ->target_index(), as the core already
> has the frequency table and so ->resolve_freq() isn't required for such
> platforms.
>
> This patch disallows drivers with ->target_index() callback to use the
> ->resolve_freq() callback.
>
> Also, it fixes a potential kernel crash for drivers providing ->target()
> but no ->resolve_freq().
>
> Fixes: e3c062360870 ("cpufreq: add cpufreq_driver_resolve_freq()")
> Signed-off-by: Viresh Kumar <viresh.kumar@...aro.org>
> ---
>  drivers/cpufreq/cpufreq.c | 16 ++++++++++++----
>  1 file changed, 12 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index b696baeb249d..342bbee8d0e1 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -507,12 +507,20 @@ unsigned int cpufreq_driver_resolve_freq(struct cpufreq_policy *policy,
>  {
>         target_freq = clamp_val(target_freq, policy->min, policy->max);
>         policy->cached_target_freq = target_freq;
> +
> +       if (cpufreq_driver->target_index) {
> +               int idx;
> +
> +               idx = cpufreq_frequency_table_target(policy, target_freq,
> +                                                    CPUFREQ_RELATION_L);
> +               policy->cached_resolved_idx = idx;
> +               return policy->freq_table[idx].frequency;
> +       }
> +
>         if (cpufreq_driver->resolve_freq)
>                 return cpufreq_driver->resolve_freq(policy, target_freq);
> -       policy->cached_resolved_idx =
> -               cpufreq_frequency_table_target(policy, target_freq,
> -                                              CPUFREQ_RELATION_L);
> -       return policy->freq_table[policy->cached_resolved_idx].frequency;
> +
> +       return UINT_MAX;

I think it should just return target_freq in this case or the UINT_MAX
will confuse things in schedutil.

Thanks,
Rafael