linux-kernel - Re: [PATCH v2 3/3] ARM: da850: fix da850_set

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: <130825f1-5c8f-f6bc-1f27-929ff591a9c3@ti.com>
Date:   Mon, 5 Dec 2016 14:15:39 +0530
From:   Sekhar Nori <nsekhar@...com>
To:     Bartosz Golaszewski <bgolaszewski@...libre.com>,
        Kevin Hilman <khilman@...libre.com>,
        Michael Turquette <mturquette@...libre.com>,
        Peter Ujfalusi <peter.ujfalusi@...com>,
        Russell King <linux@...linux.org.uk>,
        Viresh Kumar <viresh.kumar@...aro.org>,
        Boris Brezillon <boris.brezillon@...e-electrons.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Richard Weinberger <richard@....at>,
        David Woodhouse <dwmw2@...radead.org>,
        Brian Norris <computersforpeace@...il.com>,
        Marek Vasut <marek.vasut@...il.com>,
        Cyrille Pitchen <cyrille.pitchen@...el.com>
CC:     LKML <linux-kernel@...r.kernel.org>,
        arm-soc <linux-arm-kernel@...ts.infradead.org>,
        <linux-pm@...r.kernel.org>
Subject: Re: [PATCH v2 3/3] ARM: da850: fix da850_set_pll0rate()

On Friday 02 December 2016 09:08 PM, Bartosz Golaszewski wrote:
> This function is broken - its second argument is an index to the freq
> table, not the requested clock rate in Hz. It leads to an oops when
> called from clk_set_rate() since this argument isn't bounds checked
> either.
> 
> Fix it by iterating over the array of supported frequencies and
> selecting a one that matches or returning -EINVAL for unsupported
> rates.
> 
> Also: update the davinci cpufreq driver. It's the only user of this
> clock and currently it passes the cpufreq table index to
> clk_set_rate(), which is confusing. Make it pass the requested clock
> rate in Hz.
> 
> Signed-off-by: Bartosz Golaszewski <bgolaszewski@...libre.com>
> ---
>  arch/arm/mach-davinci/da850.c     | 22 ++++++++++++++++++----
>  drivers/cpufreq/davinci-cpufreq.c |  2 +-
>  2 files changed, 19 insertions(+), 5 deletions(-)
> 
> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
> index a55101c..92e3303 100644
> --- a/arch/arm/mach-davinci/da850.c
> +++ b/arch/arm/mach-davinci/da850.c
> @@ -1179,14 +1179,28 @@ static int da850_set_armrate(struct clk *clk, unsigned long index)
>  	return clk_set_rate(pllclk, index);
>  }
>  
> -static int da850_set_pll0rate(struct clk *clk, unsigned long index)
> +static int da850_set_pll0rate(struct clk *clk, unsigned long rate)
>  {
> -	unsigned int prediv, mult, postdiv;
> -	struct da850_opp *opp;
>  	struct pll_data *pll = clk->pll_data;
> +	struct cpufreq_frequency_table *freq;
> +	unsigned int prediv, mult, postdiv;
> +	struct da850_opp *opp = NULL;
>  	int ret;
>  
> -	opp = (struct da850_opp *) cpufreq_info.freq_table[index].driver_data;
> +	for (freq = da850_freq_table;
> +	     freq->frequency != CPUFREQ_TABLE_END; freq++) {
> +		/* requested_rate is in Hz, freq->frequency is in KHz */
> +		unsigned long freq_rate = freq->frequency * 1000;

A small optimization here. Instead of multiplying potentially every
frequency in the table by 1000, you could divide the incoming rate down
to KHz. This will also avoid the need for 'freq_rate'. Should have
noticed this earlier. Sorry about that.

Thanks,
Sekhar