linux-kernel - Re: [linux-next PATCH 6/7] PM / devfreq: allow sysfs governor node to switch governor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [day] [month] [year] [list]

Message-ID: <20121106212825.GA21925@kahuna>
Date:	Tue, 6 Nov 2012 15:28:25 -0600
From:	Nishanth Menon <nm@...com>
To:	MyungJoo Ham <myungjoo.ham@...il.com>
CC:	<myungjoo.ham@...sung.com>, LKML <linux-kernel@...r.kernel.org>,
	Kevin Hilman <khilman@...com>,
	linux-pm <linux-pm@...r.kernel.org>,
	Rajagopal Venkat <rajagopal.venkat@...aro.org>,
	박경민 <kyungmin.park@...sung.com>,
	"Rafael J. Wysocki" <rjw@...k.pl>
Subject: Re: [linux-next PATCH 6/7] PM / devfreq: allow sysfs governor node
 to switch governor

On 18:18-20121106, MyungJoo Ham wrote:
> 2012. 11. 6. 오후 6:02에 "MyungJoo Ham" <myungjoo.ham@...sung.com>님이 작성:
> >
> > > --- a/drivers/devfreq/devfreq.c
> > > +++ b/drivers/devfreq/devfreq.c
> > > @@ -629,6 +629,44 @@ static ssize_t show_governor(struct device *dev,
> > >       return sprintf(buf, "%s\n", to_devfreq(dev)->governor->name);
> > >  }
> > >
> > > +static ssize_t store_governor(struct device *dev, struct
> device_attribute *attr,
> > > +                           const char *buf, size_t count)
> > > +{
> > > +     struct devfreq *df = to_devfreq(dev);
> > > +     int ret = 0;
> > > +     struct devfreq_governor *governor;
> > > +
> > > +     mutex_lock(&devfreq_list_lock);
> >
> > Please remove the trailing \n from buf here.
> > When user enters "userspace", buf gets "userspace\n".
> >
> > With some printks in find_devfreq_governor (printing the buf and governor
> names):
> >
> > # echo userspace > /sys/class/devfreq/exynos4210-busfreq.0/governor
> > [   65.975000] [userspace
> > [   65.975000] ].[userspace]
> > [   65.980000] [userspace
> > [   65.980000] ].[powersave]
> > [   65.985000] [userspace
> > [   65.985000] ].[performance]
> > [   65.990000] [userspace
> > [   65.990000] ].[simple_ondemand]
> > [   65.995000] err no = -19
> > #
> >
> 
> Anyway this seems quite wierd. I will look more into this one in my target
> system.
Thanks for catching this.
:( I missed the \n (been using echo -n toooo often now grr..)
What do you think of the following change to the patch?

diff --git a/drivers/devfreq/devfreq.c b/drivers/devfreq/devfreq.c
index 586b6fd..9a4e898 100644
--- a/drivers/devfreq/devfreq.c
+++ b/drivers/devfreq/devfreq.c
@@ -614,11 +614,16 @@ static ssize_t store_governor(struct device *dev, struct device_attribute *attr,
 			      const char *buf, size_t count)
 {
 	struct devfreq *df = to_devfreq(dev);
-	int ret = 0;
+	int ret;
+	char str_governor[DEVFREQ_NAME_LEN + 1];
 	struct devfreq_governor *governor;
 
+	ret = sscanf(buf, "%" __stringify(DEVFREQ_NAME_LEN) "s", str_governor);
+	if (ret != 1)
+	        return -EINVAL;
+
 	mutex_lock(&devfreq_list_lock);
-	governor = find_devfreq_governor(buf);
+	governor = find_devfreq_governor(str_governor);
 	if (IS_ERR(governor)) {
 		ret = PTR_ERR(governor);
 		goto out;
-- 
Regards,
Nishanth Menon

output:
/sys/devices/platform/iva.0/devfreq/iva.0 # cat governor 
simple_ondemand
/sys/devices/platform/iva.0/devfreq/iva.0 # echo -n "userspace">governor 
/sys/devices/platform/iva.0/devfreq/iva.0 # echo -n "simple_ondeman">governor 
sh: write error: No such device
/sys/devices/platform/iva.0/devfreq/iva.0 # cat governor 
userspace
/sys/devices/platform/iva.0/devfreq/iva.0 # echo -n "simple_ondemand">governor 
/sys/devices/platform/iva.0/devfreq/iva.0 # cat governor 
simple_ondemand
/sys/devices/platform/iva.0/devfreq/iva.0 # echo "userspace">governor 
/sys/devices/platform/iva.0/devfreq/iva.0 # cat governor 
userspace
/sys/devices/platform/iva.0/devfreq/iva.0 # echo  "simple_ondemand">governor 
/sys/devices/platform/iva.0/devfreq/iva.0 # echo "userspace123123213123213123212
3123123131231">governor 
sh: write error: No such device
/sys/devices/platform/iva.0/devfreq/iva.0 # cat governor 
simple_ondemand

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/