Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]


Groups > linux.kernel > #1331874 > unrolled thread

[PATCH] thermal: cpu_cooling: fix out of bounds access in time_in_idle

Started byJavi Merino <javi.merino@arm.com>
First post2016-02-11 13:10 +0100
Last post2016-02-11 19:50 +0100
Articles 3 — 2 participants

Back to article view | Back to linux.kernel


Contents

  [PATCH] thermal: cpu_cooling: fix out of bounds access in time_in_idle Javi Merino <javi.merino@arm.com> - 2016-02-11 13:10 +0100
    Re: [PATCH] thermal: cpu_cooling: fix out of bounds access in  time_in_idle Eduardo Valentin <edubezval@gmail.com> - 2016-02-11 16:10 +0100
      Re: [PATCH] thermal: cpu_cooling: fix out of bounds access in  time_in_idle Javi Merino <javi.merino@arm.com> - 2016-02-11 19:50 +0100

#1331874 — [PATCH] thermal: cpu_cooling: fix out of bounds access in time_in_idle

FromJavi Merino <javi.merino@arm.com>
Date2016-02-11 13:10 +0100
Subject[PATCH] thermal: cpu_cooling: fix out of bounds access in time_in_idle
Message-ID<r0Yeo-8w1-51@gated-at.bofh.it>
In __cpufreq_cooling_register() we allocate the arrays for time_in_idle
and time_in_idle_timestamp to be as big as the number of cpus in this
cpufreq device.  However, in get_load() we access this array using the
cpu number as index, which can result in an out of bound access.

Index time_in_idle{,_timestamp} using the index in the cpufreq_device's
allowed_cpus mask, as we do for the load_cpu array in
cpufreq_get_requested_power()

Reported-by: Nicolas Boichat <drinkcat@chromium.org>
Cc: Amit Daniel Kachhap <amit.kachhap@gmail.com>
Cc: Zhang Rui <rui.zhang@intel.com>
Cc: Eduardo Valentin <edubezval@gmail.com>
Tested-by: Nicolas Boichat <drinkcat@chromium.org>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
Signed-off-by: Javi Merino <javi.merino@arm.com>
---
Hi Andrew,

This patch fixes an out of bounds access found by Nicolas Boichat
using KASAN.  It is acked by Viresh, comaintainer of the cpu cooling
device and tested by the reporter.  It's been in the list[0] for more
than a month, I've pinged the thermal maintainers three times but they
haven't replied.

Can you merge it via your tree?  Thanks,
Javi

 drivers/thermal/cpu_cooling.c | 14 ++++++++------
 1 file changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/thermal/cpu_cooling.c b/drivers/thermal/cpu_cooling.c
index e3fbc5a5d88f..bd1bab9eade0 100644
--- a/drivers/thermal/cpu_cooling.c
+++ b/drivers/thermal/cpu_cooling.c
@@ -377,26 +377,28 @@ static u32 cpu_power_to_freq(struct cpufreq_cooling_device *cpufreq_device,
  * get_load() - get load for a cpu since last updated
  * @cpufreq_device:	&struct cpufreq_cooling_device for this cpu
  * @cpu:	cpu number
+ * @cpu_idx:	index of the cpu in cpufreq_device->allowed_cpus
  *
  * Return: The average load of cpu @cpu in percentage since this
  * function was last called.
  */
-static u32 get_load(struct cpufreq_cooling_device *cpufreq_device, int cpu)
+static u32 get_load(struct cpufreq_cooling_device *cpufreq_device, int cpu,
+                    int cpu_idx)
 {
 	u32 load;
 	u64 now, now_idle, delta_time, delta_idle;
 
 	now_idle = get_cpu_idle_time(cpu, &now, 0);
-	delta_idle = now_idle - cpufreq_device->time_in_idle[cpu];
-	delta_time = now - cpufreq_device->time_in_idle_timestamp[cpu];
+	delta_idle = now_idle - cpufreq_device->time_in_idle[cpu_idx];
+	delta_time = now - cpufreq_device->time_in_idle_timestamp[cpu_idx];
 
 	if (delta_time <= delta_idle)
 		load = 0;
 	else
 		load = div64_u64(100 * (delta_time - delta_idle), delta_time);
 
-	cpufreq_device->time_in_idle[cpu] = now_idle;
-	cpufreq_device->time_in_idle_timestamp[cpu] = now;
+	cpufreq_device->time_in_idle[cpu_idx] = now_idle;
+	cpufreq_device->time_in_idle_timestamp[cpu_idx] = now;
 
 	return load;
 }
@@ -598,7 +600,7 @@ static int cpufreq_get_requested_power(struct thermal_cooling_device *cdev,
 		u32 load;
 
 		if (cpu_online(cpu))
-			load = get_load(cpufreq_device, cpu);
+			load = get_load(cpufreq_device, cpu, i);
 		else
 			load = 0;
 
-- 
1.9.1

[toc] | [next] | [standalone]


#1332134 — Re: [PATCH] thermal: cpu_cooling: fix out of bounds access in time_in_idle

FromEduardo Valentin <edubezval@gmail.com>
Date2016-02-11 16:10 +0100
SubjectRe: [PATCH] thermal: cpu_cooling: fix out of bounds access in time_in_idle
Message-ID<r112z-23q-25@gated-at.bofh.it>
In reply to#1331874
On Thu, Feb 11, 2016 at 12:00:51PM +0000, Javi Merino wrote:
> In __cpufreq_cooling_register() we allocate the arrays for time_in_idle
> and time_in_idle_timestamp to be as big as the number of cpus in this
> cpufreq device.  However, in get_load() we access this array using the
> cpu number as index, which can result in an out of bound access.
> 
> Index time_in_idle{,_timestamp} using the index in the cpufreq_device's
> allowed_cpus mask, as we do for the load_cpu array in
> cpufreq_get_requested_power()
> 
> Reported-by: Nicolas Boichat <drinkcat@chromium.org>
> Cc: Amit Daniel Kachhap <amit.kachhap@gmail.com>
> Cc: Zhang Rui <rui.zhang@intel.com>
> Cc: Eduardo Valentin <edubezval@gmail.com>
> Tested-by: Nicolas Boichat <drinkcat@chromium.org>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> Signed-off-by: Javi Merino <javi.merino@arm.com>


> ---
> Hi Andrew,
> 
> This patch fixes an out of bounds access found by Nicolas Boichat
> using KASAN.  It is acked by Viresh, comaintainer of the cpu cooling
> device and tested by the reporter.  It's been in the list[0] for more
> than a month, I've pinged the thermal maintainers three times but they
> haven't replied.
> 
> Can you merge it via your tree?  Thanks,
> Javi

Somehow this patch was marked as accepted in patchwork and I missed it,
apologize for this. I am adding it to thermal-soc.

BR,
Eduardo

[toc] | [prev] | [next] | [standalone]


#1332296 — Re: [PATCH] thermal: cpu_cooling: fix out of bounds access in time_in_idle

FromJavi Merino <javi.merino@arm.com>
Date2016-02-11 19:50 +0100
SubjectRe: [PATCH] thermal: cpu_cooling: fix out of bounds access in time_in_idle
Message-ID<r14ts-4eC-23@gated-at.bofh.it>
In reply to#1332134
On Thu, Feb 11, 2016 at 07:00:28AM -0800, Eduardo Valentin wrote:
> On Thu, Feb 11, 2016 at 12:00:51PM +0000, Javi Merino wrote:
> > In __cpufreq_cooling_register() we allocate the arrays for time_in_idle
> > and time_in_idle_timestamp to be as big as the number of cpus in this
> > cpufreq device.  However, in get_load() we access this array using the
> > cpu number as index, which can result in an out of bound access.
> > 
> > Index time_in_idle{,_timestamp} using the index in the cpufreq_device's
> > allowed_cpus mask, as we do for the load_cpu array in
> > cpufreq_get_requested_power()
> > 
> > Reported-by: Nicolas Boichat <drinkcat@chromium.org>
> > Cc: Amit Daniel Kachhap <amit.kachhap@gmail.com>
> > Cc: Zhang Rui <rui.zhang@intel.com>
> > Cc: Eduardo Valentin <edubezval@gmail.com>
> > Tested-by: Nicolas Boichat <drinkcat@chromium.org>
> > Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> > Signed-off-by: Javi Merino <javi.merino@arm.com>
> 
> 
> > ---
> > Hi Andrew,
> > 
> > This patch fixes an out of bounds access found by Nicolas Boichat
> > using KASAN.  It is acked by Viresh, comaintainer of the cpu cooling
> > device and tested by the reporter.  It's been in the list[0] for more
> > than a month, I've pinged the thermal maintainers three times but they
> > haven't replied.
> > 
> > Can you merge it via your tree?  Thanks,
> > Javi
> 
> Somehow this patch was marked as accepted in patchwork and I missed it,
> apologize for this. I am adding it to thermal-soc.

Great, thanks!
Javi

[toc] | [prev] | [standalone]


Back to top | Article view | linux.kernel


csiph-web