Replace sscanf() with kstrtoul() in set_freq_store() and check the result
to avoid invalid input.
Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com>
---
drivers/devfreq/governor_userspace.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c
index d1aa6806b683..175de0c0b50e 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -9,6 +9,7 @@
#include <linux/slab.h>
#include <linux/device.h>
#include <linux/devfreq.h>
+#include <linux/kstrtox.h>
#include <linux/pm.h>
#include <linux/mutex.h>
#include <linux/module.h>
@@ -39,10 +40,13 @@ static ssize_t set_freq_store(struct device *dev, struct device_attribute *attr,
unsigned long wanted;
int err = 0;
+ err = kstrtoul(buf, 0, &wanted);
+ if (err)
+ return err;
+
mutex_lock(&devfreq->lock);
data = devfreq->governor_data;
- sscanf(buf, "%lu", &wanted);
data->user_frequency = wanted;
data->valid = true;
err = update_devfreq(devfreq);
--
2.33.0
Hi, Applied it. Thanks. On Mon, Apr 21, 2025 at 12:00 PM Lifeng Zheng <zhenglifeng1@huawei.com> wrote: > > Replace sscanf() with kstrtoul() in set_freq_store() and check the result > to avoid invalid input. > > Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> > --- > drivers/devfreq/governor_userspace.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c > index d1aa6806b683..175de0c0b50e 100644 > --- a/drivers/devfreq/governor_userspace.c > +++ b/drivers/devfreq/governor_userspace.c > @@ -9,6 +9,7 @@ > #include <linux/slab.h> > #include <linux/device.h> > #include <linux/devfreq.h> > +#include <linux/kstrtox.h> > #include <linux/pm.h> > #include <linux/mutex.h> > #include <linux/module.h> > @@ -39,10 +40,13 @@ static ssize_t set_freq_store(struct device *dev, struct device_attribute *attr, > unsigned long wanted; > int err = 0; > > + err = kstrtoul(buf, 0, &wanted); > + if (err) > + return err; > + > mutex_lock(&devfreq->lock); > data = devfreq->governor_data; > > - sscanf(buf, "%lu", &wanted); > data->user_frequency = wanted; > data->valid = true; > err = update_devfreq(devfreq); > -- > 2.33.0 > > -- Best Regards, Chanwoo Choi Samsung Electronics
On Mon, 21 Apr 2025 11:00:17 +0800 Lifeng Zheng <zhenglifeng1@huawei.com> wrote: > Replace sscanf() with kstrtoul() in set_freq_store() and check the result > to avoid invalid input. Isn't this a UAPI change? The sscanf() version will ignore trailing characters. In this case it is actually likely that value might have a trailing "Hz". David > > Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> > --- > drivers/devfreq/governor_userspace.c | 6 +++++- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c > index d1aa6806b683..175de0c0b50e 100644 > --- a/drivers/devfreq/governor_userspace.c > +++ b/drivers/devfreq/governor_userspace.c > @@ -9,6 +9,7 @@ > #include <linux/slab.h> > #include <linux/device.h> > #include <linux/devfreq.h> > +#include <linux/kstrtox.h> > #include <linux/pm.h> > #include <linux/mutex.h> > #include <linux/module.h> > @@ -39,10 +40,13 @@ static ssize_t set_freq_store(struct device *dev, struct device_attribute *attr, > unsigned long wanted; > int err = 0; > > + err = kstrtoul(buf, 0, &wanted); > + if (err) > + return err; > + > mutex_lock(&devfreq->lock); > data = devfreq->governor_data; > > - sscanf(buf, "%lu", &wanted); > data->user_frequency = wanted; > data->valid = true; > err = update_devfreq(devfreq);
On 2025/4/27 19:17, David Laight wrote: > On Mon, 21 Apr 2025 11:00:17 +0800 > Lifeng Zheng <zhenglifeng1@huawei.com> wrote: > >> Replace sscanf() with kstrtoul() in set_freq_store() and check the result >> to avoid invalid input. > > Isn't this a UAPI change? > > The sscanf() version will ignore trailing characters. > In this case it is actually likely that value might have a trailing "Hz". I tried to still use sscanf() at first, but checkpatch warned: "Prefer kstrto<type> to single variable sscanf". I'm not sure if we should ignore this warning. > > David > >> >> Signed-off-by: Lifeng Zheng <zhenglifeng1@huawei.com> >> --- >> drivers/devfreq/governor_userspace.c | 6 +++++- >> 1 file changed, 5 insertions(+), 1 deletion(-) >> >> diff --git a/drivers/devfreq/governor_userspace.c b/drivers/devfreq/governor_userspace.c >> index d1aa6806b683..175de0c0b50e 100644 >> --- a/drivers/devfreq/governor_userspace.c >> +++ b/drivers/devfreq/governor_userspace.c >> @@ -9,6 +9,7 @@ >> #include <linux/slab.h> >> #include <linux/device.h> >> #include <linux/devfreq.h> >> +#include <linux/kstrtox.h> >> #include <linux/pm.h> >> #include <linux/mutex.h> >> #include <linux/module.h> >> @@ -39,10 +40,13 @@ static ssize_t set_freq_store(struct device *dev, struct device_attribute *attr, >> unsigned long wanted; >> int err = 0; >> >> + err = kstrtoul(buf, 0, &wanted); >> + if (err) >> + return err; >> + >> mutex_lock(&devfreq->lock); >> data = devfreq->governor_data; >> >> - sscanf(buf, "%lu", &wanted); >> data->user_frequency = wanted; >> data->valid = true; >> err = update_devfreq(devfreq); > >
© 2016 - 2025 Red Hat, Inc.