From: Russell King - ARM Linux <linux@arm.linux.org.uk>
To: "pi-cheng.chen" <pi-cheng.chen@linaro.org>
Cc: Viresh Kumar <viresh.kumar@linaro.org>,
Matthias Brugger <matthias.bgg@gmail.com>,
Rob Herring <robh+dt@kernel.org>,
"Rafael J. Wysocki" <rjw@rjwysocki.net>,
Thomas Petazzoni <thomas.petazzoni@free-electrons.com>,
Mark Rutland <mark.rutland@arm.com>,
devicetree@vger.kernel.org, linaro-kernel@lists.linaro.org,
Mike Turquette <mturquette@linaro.org>,
Pawel Moll <pawel.moll@arm.com>,
Ian Campbell <ijc+devicetree@hellion.org.uk>,
Catalin Marinas <catalin.marinas@arm.com>,
linux-pm@vger.kernel.org, Will Deacon <will.deacon@arm.com>,
linux-kernel@vger.kernel.org, fan.chen@mediatek.com,
Howard Chen <ibanezchen@gmail.com>,
Ashwin Chaugule <ashwin.chaugule@linaro.org>,
linux-mediatek@lists.infradead.org,
Kumar Gala <galak@codeaurora.org>,
"Joe.C" <yingjoe.chen@mediatek.com>,
Eddie Huang <eddie.huang@mediatek.com>,
linux-arm-kernel@lists.infradead.org
Subject: Re: [PATCH v2 3/4] cpufreq: mediatek: add Mediatek cpufreq driver
Date: Mon, 9 Mar 2015 16:28:09 +0000 [thread overview]
Message-ID: <20150309162809.GY8656@n2100.arm.linux.org.uk> (raw)
In-Reply-To: <1425458956-20665-4-git-send-email-pi-cheng.chen@linaro.org>
On Wed, Mar 04, 2015 at 04:49:15PM +0800, pi-cheng.chen wrote:
> +static int cpu_opp_table_get_freq_index(unsigned int freq)
> +{
> + struct cpu_opp_table *opp_tbl = dvfs_info->opp_tbl;
> + int i;
> +
> + for (i = 0; opp_tbl[i].freq != 0; i++) {
> + if (opp_tbl[i].freq >= freq)
> + return i;
> + }
> +
> + return -1;
My "return -1" detector fired on this...
> +static int cpu_opp_table_get_volt_index(unsigned int volt)
> +{
> + struct cpu_opp_table *opp_tbl = dvfs_info->opp_tbl;
> + int i;
> +
> + for (i = 0; opp_tbl[i].vproc != -1; i++)
> + if (opp_tbl[i].vproc >= volt)
> + return i;
> +
> + return -1;
And this.
> +static int mtk_cpufreq_notify(struct notifier_block *nb,
> + unsigned long action, void *data)
> +{
> + struct cpufreq_freqs *freqs = data;
> + struct cpu_opp_table *opp_tbl = dvfs_info->opp_tbl;
> + int old_vproc, new_vproc, old_index, new_index;
> +
> + if (!cpumask_test_cpu(freqs->cpu, &dvfs_info->cpus))
> + return NOTIFY_DONE;
> +
> + old_vproc = regulator_get_voltage(dvfs_info->proc_reg);
> + old_index = cpu_opp_table_get_volt_index(old_vproc);
> + new_index = cpu_opp_table_get_freq_index(freqs->new * 1000);
> + new_vproc = opp_tbl[new_index].vproc;
Let's say that cpu_opp_table_get_freq_index() returns -1. We then
do no error checking on this, and access the memory immediately
preceding opp_tbl[0].
Since we'll be loading garbage from opp_tbl[-1], this probably means
that mtk_cpufreq_voltage_trace() will go wrong. Your method of using
the vproc values to work out which direction we should walk between
old_index...new_index means that we could end up walking through
almost the whole UINT_MAX range to wrap around to the new index.
Yet again, "return -1" proves to be a sure sign of a bug.
--
FTTC broadband for 0.8mile line: currently at 10.5Mbps down 400kbps up
according to speedtest.net.
next prev parent reply other threads:[~2015-03-09 16:28 UTC|newest]
Thread overview: 27+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-03-04 8:49 [PATCH v2 0/4] cpufreq: add cpufreq driver for Mediatek MT8173 SoC pi-cheng.chen
2015-03-04 8:49 ` [PATCH v2 1/4] cpufreq-dt: add clock domain and intermediate frequency support pi-cheng.chen
2015-03-04 10:15 ` Viresh Kumar
2015-03-04 10:17 ` Viresh Kumar
2015-03-05 3:32 ` Pi-Cheng Chen
2015-03-05 3:58 ` Viresh Kumar
2015-03-05 7:28 ` Pi-Cheng Chen
2015-03-04 8:49 ` [PATCH v2 2/4] cpufreq: dt-bindings: add bindings for mtk-cpufreq driver pi-cheng.chen
2015-03-04 10:29 ` Viresh Kumar
2015-03-04 8:49 ` [PATCH v2 3/4] cpufreq: mediatek: add Mediatek cpufreq driver pi-cheng.chen
2015-03-04 11:09 ` Viresh Kumar
2015-03-05 7:27 ` Pi-Cheng Chen
2015-03-05 9:55 ` Viresh Kumar
2015-03-06 5:49 ` Pi-Cheng Chen
2015-03-10 2:50 ` Viresh Kumar
2015-03-11 10:53 ` Mark Brown
2015-03-11 11:03 ` Viresh Kumar
2015-03-11 11:42 ` Lucas Stach
2015-03-11 11:46 ` Viresh Kumar
2015-03-11 12:46 ` Mark Brown
2015-03-11 12:45 ` Mark Brown
2015-03-12 9:28 ` Viresh Kumar
2015-03-12 11:15 ` Pi-Cheng Chen
2015-03-18 6:59 ` Viresh Kumar
2015-03-09 16:28 ` Russell King - ARM Linux [this message]
2015-03-10 1:57 ` Pi-Cheng Chen
2015-03-04 8:49 ` [PATCH v2 4/4] ARM64: dts: mediatek: add cpufreq dts for MT8173 SoC pi-cheng.chen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20150309162809.GY8656@n2100.arm.linux.org.uk \
--to=linux@arm.linux.org.uk \
--cc=ashwin.chaugule@linaro.org \
--cc=catalin.marinas@arm.com \
--cc=devicetree@vger.kernel.org \
--cc=eddie.huang@mediatek.com \
--cc=fan.chen@mediatek.com \
--cc=galak@codeaurora.org \
--cc=ibanezchen@gmail.com \
--cc=ijc+devicetree@hellion.org.uk \
--cc=linaro-kernel@lists.linaro.org \
--cc=linux-arm-kernel@lists.infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-mediatek@lists.infradead.org \
--cc=linux-pm@vger.kernel.org \
--cc=mark.rutland@arm.com \
--cc=matthias.bgg@gmail.com \
--cc=mturquette@linaro.org \
--cc=pawel.moll@arm.com \
--cc=pi-cheng.chen@linaro.org \
--cc=rjw@rjwysocki.net \
--cc=robh+dt@kernel.org \
--cc=thomas.petazzoni@free-electrons.com \
--cc=viresh.kumar@linaro.org \
--cc=will.deacon@arm.com \
--cc=yingjoe.chen@mediatek.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).