[PATCH] PM / devfreq: Rewrite devfreq_update_status() to fix multiple bugs

[PATCH] PM / devfreq: Rewrite devfreq_update_status() to fix multiple bugs

[Saravana Kannan]

The current devfreq_update_status() has the following bugs:
- If previous frequency doesn't have a valid level, it does an out of bounds
  access into the trans_table and causes memory corruption.
- When the new frequency doesn't have a valid level, the time spent in the
  new frequency is counted towards the next valid frequency switch instead of
  being ignored.
- The time spent on the previous frequency is added to the new frequency's
  stats instead of the previous frequency's stats.

This patch fixes all of this.

Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
---
 drivers/devfreq/devfreq.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index a0b2f7e..2042ec3 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -91,26 +91,35 @@ static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq)
  */
 static int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
 {
-	int lev, prev_lev;
+	int lev, prev_lev, ret = 0;
 	unsigned long cur_time;
 
-	lev = devfreq_get_freq_level(devfreq, freq);
-	if (lev < 0)
-		return lev;
-
 	cur_time = jiffies;
-	devfreq->time_in_state[lev] +=
+
+	prev_lev = devfreq_get_freq_level(devfreq, devfreq->previous_freq);
+	if (prev_lev < 0) {
+		ret = prev_lev;
+		goto out;
+	}
+
+	devfreq->time_in_state[prev_lev] +=
 			 cur_time - devfreq->last_stat_updated;
-	if (freq != devfreq->previous_freq) {
-		prev_lev = devfreq_get_freq_level(devfreq,
-						devfreq->previous_freq);
+
+	lev = devfreq_get_freq_level(devfreq, freq);
+	if (lev < 0) {
+		ret = lev;
+		goto out;
+	}
+
+	if (lev != prev_lev) {
 		devfreq->trans_table[(prev_lev *
 				devfreq->profile->max_state) + lev]++;
 		devfreq->total_trans++;
 	}
-	devfreq->last_stat_updated = cur_time;
 
-	return 0;
+out:
+	devfreq->last_stat_updated = cur_time;
+	return ret;
 }
 
 /**
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH] PM / devfreq: Rewrite devfreq_update_status() to fix multiple bugs

[Saravana Kannan]

The current devfreq_update_status() has the following bugs:
- If previous frequency doesn't have a valid level, it does an out of bounds
  access into the trans_table and causes memory corruption.
- When the new frequency doesn't have a valid level, the time spent in the
  new frequency is counted towards the next valid frequency switch instead of
  being ignored.
- The time spent on the previous frequency is added to the new frequency's
  stats instead of the previous frequency's stats.

This patch fixes all of this.

Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
---
 drivers/devfreq/devfreq.c | 31 ++++++++++++++++++++-----------
 1 file changed, 20 insertions(+), 11 deletions(-)

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index a0b2f7e..2042ec3 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -91,26 +91,35 @@ static int devfreq_get_freq_level(struct devfreq *devfreq, unsigned long freq)
  */
 static int devfreq_update_status(struct devfreq *devfreq, unsigned long freq)
 {
-	int lev, prev_lev;
+	int lev, prev_lev, ret = 0;
 	unsigned long cur_time;
 
-	lev = devfreq_get_freq_level(devfreq, freq);
-	if (lev < 0)
-		return lev;
-
 	cur_time = jiffies;
-	devfreq->time_in_state[lev] +=
+
+	prev_lev = devfreq_get_freq_level(devfreq, devfreq->previous_freq);
+	if (prev_lev < 0) {
+		ret = prev_lev;
+		goto out;
+	}
+
+	devfreq->time_in_state[prev_lev] +=
 			 cur_time - devfreq->last_stat_updated;
-	if (freq != devfreq->previous_freq) {
-		prev_lev = devfreq_get_freq_level(devfreq,
-						devfreq->previous_freq);
+
+	lev = devfreq_get_freq_level(devfreq, freq);
+	if (lev < 0) {
+		ret = lev;
+		goto out;
+	}
+
+	if (lev != prev_lev) {
 		devfreq->trans_table[(prev_lev *
 				devfreq->profile->max_state) + lev]++;
 		devfreq->total_trans++;
 	}
-	devfreq->last_stat_updated = cur_time;
 
-	return 0;
+out:
+	devfreq->last_stat_updated = cur_time;
+	return ret;
 }
 
 /**
-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] PM / devfreq: Rewrite devfreq_update_status() to fix multiple bugs

[Saravana Kannan]

On 02/27/2014 07:38 PM, Saravana Kannan wrote:
> The current devfreq_update_status() has the following bugs:
> - If previous frequency doesn't have a valid level, it does an out of bounds
>    access into the trans_table and causes memory corruption.
> - When the new frequency doesn't have a valid level, the time spent in the
>    new frequency is counted towards the next valid frequency switch instead of
>    being ignored.
> - The time spent on the previous frequency is added to the new frequency's
>    stats instead of the previous frequency's stats.
>
> This patch fixes all of this.
>
> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
> ---

MyungJoo/Kyungmin,

Can you please take a look and let me know if this looks ok? Any 
possibility of getting this into 3.14?

Thanks,
Saravana

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH] PM / devfreq: Rewrite devfreq_update_status() to fix multiple bugs

[Saravana Kannan]

On 02/27/2014 07:38 PM, Saravana Kannan wrote:
> The current devfreq_update_status() has the following bugs:
> - If previous frequency doesn't have a valid level, it does an out of bounds
>    access into the trans_table and causes memory corruption.
> - When the new frequency doesn't have a valid level, the time spent in the
>    new frequency is counted towards the next valid frequency switch instead of
>    being ignored.
> - The time spent on the previous frequency is added to the new frequency's
>    stats instead of the previous frequency's stats.
>
> This patch fixes all of this.
>
> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
> ---

MyungJoo/Kyungmin,

Can you please take a look and let me know if this looks ok? Any 
possibility of getting this into 3.14?

Thanks,
Saravana

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] PM / devfreq: Rewrite devfreq_update_status() to fix multiple bugs

[Saravana Kannan]

On 03/04/2014 02:06 AM, MyungJoo Ham wrote:
>> The current devfreq_update_status() has the following bugs:
>> - If previous frequency doesn't have a valid level, it does an out of bounds
>>    access into the trans_table and causes memory corruption.
>> - When the new frequency doesn't have a valid level, the time spent in the
>>    new frequency is counted towards the next valid frequency switch instead of
>>    being ignored.
>> - The time spent on the previous frequency is added to the new frequency's
>>    stats instead of the previous frequency's stats.
>>
>> This patch fixes all of this.
>>
>> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
> 
> Merged.
> 
> It appears to be Greg's LTS material as well.
> 

Thanks!

-Saravana

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

[PATCH] PM / devfreq: Rewrite devfreq_update_status() to fix multiple bugs

[Saravana Kannan]

On 03/04/2014 02:06 AM, MyungJoo Ham wrote:
>> The current devfreq_update_status() has the following bugs:
>> - If previous frequency doesn't have a valid level, it does an out of bounds
>>    access into the trans_table and causes memory corruption.
>> - When the new frequency doesn't have a valid level, the time spent in the
>>    new frequency is counted towards the next valid frequency switch instead of
>>    being ignored.
>> - The time spent on the previous frequency is added to the new frequency's
>>    stats instead of the previous frequency's stats.
>>
>> This patch fixes all of this.
>>
>> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>
> 
> Merged.
> 
> It appears to be Greg's LTS material as well.
> 

Thanks!

-Saravana

-- 
The Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
hosted by The Linux Foundation

Re: [PATCH] PM / devfreq: Rewrite devfreq_update_status() to fix multiple bugs

[MyungJoo Ham]

> The current devfreq_update_status() has the following bugs:
> - If previous frequency doesn't have a valid level, it does an out of bounds
>   access into the trans_table and causes memory corruption.
> - When the new frequency doesn't have a valid level, the time spent in the
>   new frequency is counted towards the next valid frequency switch instead of
>   being ignored.
> - The time spent on the previous frequency is added to the new frequency's
>   stats instead of the previous frequency's stats.
> 
> This patch fixes all of this.
> 
> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>

Merged.

It appears to be Greg's LTS material as well.


Cheers,
MyungJoo

Re: [PATCH] PM / devfreq: Rewrite devfreq_update_status() to fix multiple bugs

[MyungJoo Ham]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #1: Type: text/plain; charset=euc-kr, Size: 841 bytes --]

> The current devfreq_update_status() has the following bugs:
> - If previous frequency doesn't have a valid level, it does an out of bounds
>   access into the trans_table and causes memory corruption.
> - When the new frequency doesn't have a valid level, the time spent in the
>   new frequency is counted towards the next valid frequency switch instead of
>   being ignored.
> - The time spent on the previous frequency is added to the new frequency's
>   stats instead of the previous frequency's stats.
> 
> This patch fixes all of this.
> 
> Signed-off-by: Saravana Kannan <skannan@codeaurora.org>

Merged.

It appears to be Greg's LTS material as well.


Cheers,
MyungJoo

ÿôèº{.nÇ+‰·Ÿ®‰­†+%ŠËÿ±éݶ\x17¥Šwÿº{.nÇ+‰·¥Š{±þG«éÿŠ{ayº\x1dʇڙë,j\a­¢f£¢·hšïêÿ‘êçz_è®\x03(­éšŽŠÝ¢j"ú\x1a¶^[m§ÿÿ¾\a«þG«éÿ¢¸?™¨è­Ú&£ø§~á¶iO•æ¬z·švØ^\x14\x04\x1a¶^[m§ÿÿÃ\fÿ¶ìÿ¢¸?–I¥