* [PATCH] devfreq: add error check for sscanf in userspace governor
@ 2017-08-07 13:06 Santosh Mardi
2017-08-07 13:06 ` Santosh Mardi
[not found] ` <CGME20170807130735epcas5p4386af2ade3bd98b67fc5a9c956152c7d@epcms1p5>
0 siblings, 2 replies; 6+ messages in thread
From: Santosh Mardi @ 2017-08-07 13:06 UTC (permalink / raw)
To: myungjoo.ham, kyungmin.park, rafael.j.wysocki, cw00.choi
Cc: linux-pm, linux-kernel, gsantosh, gsantosh, skannan, rgottimu
What this patch does:
Current implementation of store_freq function in devfreq
userspace governor executes further, even if error is returned
from sscanf. This will result in setting up wrong frequency value.
This patch adds proper error check to bail out if
any error is returned.
Santosh Mardi (1):
devfreq: add error check for sscanf in userspace governor
drivers/devfreq/governor_userspace.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
--
1.9.1
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] devfreq: add error check for sscanf in userspace governor
2017-08-07 13:06 [PATCH] devfreq: add error check for sscanf in userspace governor Santosh Mardi
@ 2017-08-07 13:06 ` Santosh Mardi
2017-08-08 0:51 ` Chanwoo Choi
2017-08-08 6:56 ` Pavan Kondeti
[not found] ` <CGME20170807130735epcas5p4386af2ade3bd98b67fc5a9c956152c7d@epcms1p5>
1 sibling, 2 replies; 6+ messages in thread
From: Santosh Mardi @ 2017-08-07 13:06 UTC (permalink / raw)
To: myungjoo.ham, kyungmin.park, rafael.j.wysocki, cw00.choi
Cc: linux-pm, linux-kernel, gsantosh, gsantosh, skannan, rgottimu
store_freq function of devfreq userspace governor
executes further, even if error is returned from sscanf,
this will result in setting up wrong frequency value.
Add proper error check to bail out if any error is returned.
Signed-off-by: Santosh Mardi <gsantosh@codeaurora.org>
---
drivers/devfreq/governor_userspace.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
index 77028c2..1d0c9cc 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -53,12 +53,15 @@ static ssize_t store_freq(struct device *dev, struct device_attribute *attr,
mutex_lock(&devfreq->lock);
data = devfreq->data;
- sscanf(buf, "%lu", &wanted);
+ err = sscanf(buf, "%lu", &wanted);
+ if (err != 1)
+ goto out;
data->user_frequency = wanted;
data->valid = true;
err = update_devfreq(devfreq);
if (err == 0)
err = count;
+out:
mutex_unlock(&devfreq->lock);
return err;
}
--
1.9.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* RE: [PATCH] devfreq: add error check for sscanf in userspace governor
[not found] ` <CGME20170807130735epcas5p4386af2ade3bd98b67fc5a9c956152c7d@epcms1p5>
@ 2017-08-08 0:14 ` MyungJoo Ham
0 siblings, 0 replies; 6+ messages in thread
From: MyungJoo Ham @ 2017-08-08 0:14 UTC (permalink / raw)
To: Kyungmin Park, rafael.j.wysocki, Chanwoo Choi
Cc: linux-pm, linux-kernel, gsantosh, gsantosh, skannan, rgottimu
> store_freq function of devfreq userspace governor
> executes further, even if error is returned from sscanf,
> this will result in setting up wrong frequency value.
>
> Add proper error check to bail out if any error is returned.
>
> Signed-off-by: Santosh Mardi <gsantosh@codeaurora.org>
Acked-by: MyungJoo Ham <myungjoo.ham@samsung.com>
> ---
> drivers/devfreq/governor_userspace.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] devfreq: add error check for sscanf in userspace governor
2017-08-07 13:06 ` Santosh Mardi
@ 2017-08-08 0:51 ` Chanwoo Choi
2017-08-08 6:56 ` Pavan Kondeti
1 sibling, 0 replies; 6+ messages in thread
From: Chanwoo Choi @ 2017-08-08 0:51 UTC (permalink / raw)
To: Santosh Mardi, myungjoo.ham, kyungmin.park, rafael.j.wysocki
Cc: linux-pm, linux-kernel, gsantosh, skannan, rgottimu
Hi,
On 2017년 08월 07일 22:06, Santosh Mardi wrote:
> store_freq function of devfreq userspace governor
> executes further, even if error is returned from sscanf,
> this will result in setting up wrong frequency value.
>
> Add proper error check to bail out if any error is returned.
>
> Signed-off-by: Santosh Mardi <gsantosh@codeaurora.org>
> ---
> drivers/devfreq/governor_userspace.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
> index 77028c2..1d0c9cc 100644
> --- a/drivers/devfreq/governor_userspace.c
> +++ b/drivers/devfreq/governor_userspace.c
> @@ -53,12 +53,15 @@ static ssize_t store_freq(struct device *dev, struct device_attribute *attr,
> mutex_lock(&devfreq->lock);
> data = devfreq->data;
>
> - sscanf(buf, "%lu", &wanted);
> + err = sscanf(buf, "%lu", &wanted);
> + if (err != 1)
> + goto out;
> data->user_frequency = wanted;
> data->valid = true;
> err = update_devfreq(devfreq);
> if (err == 0)
> err = count;
> +out:
> mutex_unlock(&devfreq->lock);
> return err;
> }
>
Looks good to me.
Reviewed-by: Chanwoo Choi <cw00.choi@samsung.com>
--
Best Regards,
Chanwoo Choi
Samsung Electronics
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] devfreq: add error check for sscanf in userspace governor
2017-08-07 13:06 ` Santosh Mardi
2017-08-08 0:51 ` Chanwoo Choi
@ 2017-08-08 6:56 ` Pavan Kondeti
2017-08-08 19:26 ` Saravana Kannan
1 sibling, 1 reply; 6+ messages in thread
From: Pavan Kondeti @ 2017-08-08 6:56 UTC (permalink / raw)
To: Santosh Mardi
Cc: myungjoo.ham, kyungmin.park, rafael.j.wysocki, cw00.choi,
linux-pm, LKML, gsantosh, skannan, rgottimu
Hi Santosh,
On Mon, Aug 7, 2017 at 6:36 PM, Santosh Mardi <gsantosh@codeaurora.org> wrote:
> store_freq function of devfreq userspace governor
> executes further, even if error is returned from sscanf,
> this will result in setting up wrong frequency value.
>
> Add proper error check to bail out if any error is returned.
>
> Signed-off-by: Santosh Mardi <gsantosh@codeaurora.org>
> ---
> drivers/devfreq/governor_userspace.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
> index 77028c2..1d0c9cc 100644
> --- a/drivers/devfreq/governor_userspace.c
> +++ b/drivers/devfreq/governor_userspace.c
> @@ -53,12 +53,15 @@ static ssize_t store_freq(struct device *dev, struct device_attribute *attr,
> mutex_lock(&devfreq->lock);
> data = devfreq->data;
>
> - sscanf(buf, "%lu", &wanted);
> + err = sscanf(buf, "%lu", &wanted);
> + if (err != 1)
> + goto out;
You can save this goto statement by moving this sscanf checking to
before taking the mutex.
> data->user_frequency = wanted;
> data->valid = true;
> err = update_devfreq(devfreq);
> if (err == 0)
> err = count;
> +out:
> mutex_unlock(&devfreq->lock);
> return err;
> }
> --
> 1.9.1
>
--
Qualcomm India Private Limited, on behalf of Qualcomm Innovation Center, Inc.
Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a
Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] devfreq: add error check for sscanf in userspace governor
2017-08-08 6:56 ` Pavan Kondeti
@ 2017-08-08 19:26 ` Saravana Kannan
0 siblings, 0 replies; 6+ messages in thread
From: Saravana Kannan @ 2017-08-08 19:26 UTC (permalink / raw)
To: Pavan Kondeti
Cc: Santosh Mardi, myungjoo.ham, kyungmin.park, rafael.j.wysocki,
cw00.choi, linux-pm, LKML, gsantosh, skannan, rgottimu
On 08/07/2017 11:56 PM, Pavan Kondeti wrote:
> Hi Santosh,
>
> On Mon, Aug 7, 2017 at 6:36 PM, Santosh Mardi <gsantosh@codeaurora.org> wrote:
>> store_freq function of devfreq userspace governor
>> executes further, even if error is returned from sscanf,
>> this will result in setting up wrong frequency value.
>>
>> Add proper error check to bail out if any error is returned.
>>
>> Signed-off-by: Santosh Mardi <gsantosh@codeaurora.org>
>> ---
>> drivers/devfreq/governor_userspace.c | 5 ++++-
>> 1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
>> index 77028c2..1d0c9cc 100644
>> --- a/drivers/devfreq/governor_userspace.c
>> +++ b/drivers/devfreq/governor_userspace.c
>> @@ -53,12 +53,15 @@ static ssize_t store_freq(struct device *dev, struct device_attribute *attr,
>> mutex_lock(&devfreq->lock);
>> data = devfreq->data;
>>
>> - sscanf(buf, "%lu", &wanted);
>> + err = sscanf(buf, "%lu", &wanted);
Also, we could just use kstroul().
>> + if (err != 1)
>> + goto out;
>
> You can save this goto statement by moving this sscanf checking to
> before taking the mutex.
>
>> data->user_frequency = wanted;
>> data->valid = true;
>> err = update_devfreq(devfreq);
>> if (err == 0)
>> err = count;
>> +out:
>> mutex_unlock(&devfreq->lock);
>> return err;
>> }
>> --
>> 1.9.1
>>
-Saravana
--
Qualcomm Innovation Center, Inc.
The Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum,
a Linux Foundation Collaborative Project
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2017-08-08 19:26 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-08-07 13:06 [PATCH] devfreq: add error check for sscanf in userspace governor Santosh Mardi
2017-08-07 13:06 ` Santosh Mardi
2017-08-08 0:51 ` Chanwoo Choi
2017-08-08 6:56 ` Pavan Kondeti
2017-08-08 19:26 ` Saravana Kannan
[not found] ` <CGME20170807130735epcas5p4386af2ade3bd98b67fc5a9c956152c7d@epcms1p5>
2017-08-08 0:14 ` MyungJoo Ham
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).