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.
WARNING: multiple messages have this Message-ID (diff)
From: linux@arm.linux.org.uk (Russell King - ARM Linux) To: linux-arm-kernel@lists.infradead.org Subject: [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: 76+ 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 ` 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 8:49 ` pi-cheng.chen 2015-03-04 8:49 ` pi-cheng.chen 2015-03-04 10:15 ` Viresh Kumar 2015-03-04 10:15 ` Viresh Kumar 2015-03-04 10:15 ` Viresh Kumar 2015-03-04 10:17 ` Viresh Kumar 2015-03-04 10:17 ` Viresh Kumar 2015-03-04 10:17 ` Viresh Kumar 2015-03-05 3:32 ` Pi-Cheng Chen 2015-03-05 3:32 ` Pi-Cheng Chen 2015-03-05 3:32 ` Pi-Cheng Chen 2015-03-05 3:58 ` Viresh Kumar 2015-03-05 3:58 ` Viresh Kumar 2015-03-05 3:58 ` Viresh Kumar 2015-03-05 7:28 ` Pi-Cheng Chen 2015-03-05 7:28 ` Pi-Cheng Chen 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 8:49 ` pi-cheng.chen 2015-03-04 10:29 ` Viresh Kumar 2015-03-04 10:29 ` Viresh Kumar 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 8:49 ` pi-cheng.chen 2015-03-04 11:09 ` Viresh Kumar 2015-03-04 11:09 ` Viresh Kumar 2015-03-04 11:09 ` Viresh Kumar 2015-03-05 7:27 ` Pi-Cheng Chen 2015-03-05 7:27 ` Pi-Cheng Chen 2015-03-05 7:27 ` Pi-Cheng Chen 2015-03-05 9:55 ` Viresh Kumar 2015-03-05 9:55 ` Viresh Kumar 2015-03-05 9:55 ` Viresh Kumar 2015-03-06 5:49 ` Pi-Cheng Chen 2015-03-06 5:49 ` Pi-Cheng Chen 2015-03-06 5:49 ` Pi-Cheng Chen 2015-03-10 2:50 ` Viresh Kumar 2015-03-10 2:50 ` Viresh Kumar 2015-03-10 2:50 ` Viresh Kumar 2015-03-11 10:53 ` Mark Brown 2015-03-11 10:53 ` Mark Brown 2015-03-11 10:53 ` Mark Brown 2015-03-11 11:03 ` Viresh Kumar 2015-03-11 11:03 ` Viresh Kumar 2015-03-11 11:03 ` Viresh Kumar 2015-03-11 11:42 ` Lucas Stach 2015-03-11 11:42 ` Lucas Stach 2015-03-11 11:42 ` Lucas Stach 2015-03-11 11:46 ` Viresh Kumar 2015-03-11 11:46 ` Viresh Kumar 2015-03-11 11:46 ` Viresh Kumar 2015-03-11 12:46 ` Mark Brown 2015-03-11 12:46 ` Mark Brown 2015-03-11 12:46 ` Mark Brown 2015-03-11 12:45 ` Mark Brown 2015-03-11 12:45 ` Mark Brown 2015-03-11 12:45 ` Mark Brown 2015-03-12 9:28 ` Viresh Kumar 2015-03-12 9:28 ` Viresh Kumar 2015-03-12 9:28 ` Viresh Kumar 2015-03-12 11:15 ` Pi-Cheng Chen 2015-03-12 11:15 ` Pi-Cheng Chen 2015-03-12 11:15 ` Pi-Cheng Chen 2015-03-18 6:59 ` Viresh Kumar 2015-03-18 6:59 ` Viresh Kumar 2015-03-18 6:59 ` Viresh Kumar 2015-03-09 16:28 ` Russell King - ARM Linux [this message] 2015-03-09 16:28 ` Russell King - ARM Linux 2015-03-10 1:57 ` Pi-Cheng Chen 2015-03-10 1:57 ` Pi-Cheng Chen 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 2015-03-04 8:49 ` 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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.