* [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 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
[parent not found: <CGME20170807130735epcas5p4386af2ade3bd98b67fc5a9c956152c7d@epcms1p5>]
* 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
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).