linux-kernel - Re: [PATCH] cpufreq: Avoid creating excessively large stack frames

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: <20200123053251.3j7muofewhfidplw@vireshk-i7>
Date:   Thu, 23 Jan 2020 11:02:51 +0530
From:   Viresh Kumar <viresh.kumar@...aro.org>
To:     "Rafael J. Wysocki" <rjw@...ysocki.net>
Cc:     Linux PM <linux-pm@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        kbuild test robot <lkp@...el.com>
Subject: Re: [PATCH] cpufreq: Avoid creating excessively large stack frames

On 23-01-20, 00:16, Rafael J. Wysocki wrote:
> From: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> 
> In the process of modifying a cpufreq policy, the cpufreq core makes
> a copy of it including all of the internals which is stored on the
> CPU stack.  Because struct cpufreq_policy is relatively large, this
> may cause the size of the stack frame to exceed the 2 KB limit and
> so the GCC complains when -Wframe-larger-than= is used.
> 
> In fact, it is not necessary to copy the entire policy structure
> in order to modify it, however.
> 
> First, because cpufreq_set_policy() obtains the min and max policy
> limits from frequency QoS now, it is not necessary to pass the limits
> to it from the callers.  The only things that need to be passed to it
> from there are the new governor pointer or (if there is a built-in
> governor in the driver) the "policy" value representing the governor
> choice.  They both can be passed as individual arguments, though, so
> make cpufreq_set_policy() take them this way and rework its callers
> accordingly.  This avoids making copies of cpufreq policies in the
> callers of cpufreq_set_policy().
> 
> Second, cpufreq_set_policy() still needs to pass the new policy
> data to the ->verify() callback of the cpufreq driver whose task
> is to sanitize the min and max policy limits.  It still does not
> need to make a full copy of struct cpufreq_policy for this purpose,
> but it needs to pass a few items from it to the driver in case they
> are needed (different drivers have different needs in that respect
> and all of them have to be covered).  For this reason, introduce
> struct cpufreq_policy_data to hold copies of the members of
> struct cpufreq_policy used by the existing ->verify() driver
> callbacks and pass a pointer to a temporary structure of that
> type to ->verify() (instead of passing a pointer to full struct
> cpufreq_policy to it).
> 
> While at it, notice that intel_pstate doesn't really need to verify
> the "policy" value in struct cpufreq_policy, so drop that check from
> it to avoid copying "policy" into struct cpufreq_policy_data (which
> allows it to be slightly smaller).
> 
> Also while at it fix up white space in a couple of places and make
> cpufreq_set_policy() static (as it can be so).
> 
> Fixes: 3000ce3c52f8 ("cpufreq: Use per-policy frequency QoS")
> Link: https://lore.kernel.org/linux-pm/CAMuHMdX6-jb1W8uC2_237m8ctCpsnGp=JCxqt8pCWVqNXHmkVg@mail.gmail.com
> Reported-by: kbuild test robot <lkp@...el.com>
> Reported-by: Geert Uytterhoeven <geert@...ux-m68k.org>
> Cc: 5.4+ <stable@...r.kernel.org> # 5.4+
> Signed-off-by: Rafael J. Wysocki <rafael.j.wysocki@...el.com>
> ---
>  drivers/cpufreq/cppc_cpufreq.c     |    2 
>  drivers/cpufreq/cpufreq-nforce2.c  |    2 
>  drivers/cpufreq/cpufreq.c          |  147 +++++++++++++++++--------------------
>  drivers/cpufreq/freq_table.c       |    4 -
>  drivers/cpufreq/gx-suspmod.c       |    2 
>  drivers/cpufreq/intel_pstate.c     |   38 ++++-----
>  drivers/cpufreq/longrun.c          |    2 
>  drivers/cpufreq/pcc-cpufreq.c      |    2 
>  drivers/cpufreq/sh-cpufreq.c       |    2 
>  drivers/cpufreq/unicore2-cpufreq.c |    2 
>  include/linux/cpufreq.h            |   32 +++++---
>  11 files changed, 119 insertions(+), 116 deletions(-)

Acked-by: Viresh Kumar <viresh.kumar@...aro.org>

-- 
viresh