backlight: catch invalid input

backlight: catch invalid input

[Pavel Machek]

Currently, echo > brightness turns brightness to zero due to
insufficient checking. Add a test to catch that.

Signed-off-by: Pavel Machek <pavel@suse.cz>

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index fab0bc8..5c2e23d 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -86,6 +86,8 @@ static ssize_t backlight_store_power(str
 	int power = simple_strtoul(buf, &endp, 0);
 	size_t size = endp - buf;
 
+	if (!size)
+		return -EINVAL;
 	if (*endp && isspace(*endp))
 		size++;
 	if (size != count)


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: backlight: catch invalid input

[Andrew Morton]

On Fri, 21 Nov 2008 12:14:14 +0100
Pavel Machek <pavel@suse.cz> wrote:

> 
> Currently, echo > brightness turns brightness to zero due to
> insufficient checking. Add a test to catch that.
> 

OK.  And what happens if you do

	echo wibble > brightness

?

> 
> diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> index fab0bc8..5c2e23d 100644
> --- a/drivers/video/backlight/backlight.c
> +++ b/drivers/video/backlight/backlight.c
> @@ -86,6 +86,8 @@ static ssize_t backlight_store_power(str
>  	int power = simple_strtoul(buf, &endp, 0);
>  	size_t size = endp - buf;
>  
> +	if (!size)
> +		return -EINVAL;
>  	if (*endp && isspace(*endp))
>  		size++;
>  	if (size != count)

A better fix would be to use strict_strtoul() and check its return value.

Re: backlight: catch invalid input

[Pavel Machek]

On Fri 2008-11-21 15:54:21, Andrew Morton wrote:
> On Fri, 21 Nov 2008 12:14:14 +0100
> Pavel Machek <pavel@suse.cz> wrote:
> 
> > 
> > Currently, echo > brightness turns brightness to zero due to
> > insufficient checking. Add a test to catch that.
> > 
> 
> OK.  And what happens if you do
> 
> 	echo wibble > brightness
> 
> ?

That's okay, that is catched by tests already existing in there. size
is the size we decoded, count is how many bytes user wrote. If (size
!= count), we return failure.

But there's an exception for whitespace, and missing check for nothing
at all being written.

> > diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
> > index fab0bc8..5c2e23d 100644
> > --- a/drivers/video/backlight/backlight.c
> > +++ b/drivers/video/backlight/backlight.c
> > @@ -86,6 +86,8 @@ static ssize_t backlight_store_power(str
> >  	int power = simple_strtoul(buf, &endp, 0);
> >  	size_t size = endp - buf;
> >  
> > +	if (!size)
> > +		return -EINVAL;
> >  	if (*endp && isspace(*endp))
> >  		size++;
> >  	if (size != count)
> 
> A better fix would be to use strict_strtoul() and check its return
> value.

Aha, here's the hint :-). Ok, will do.
									Pavel

-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

Re: backlight: catch invalid input

[Pavel Machek]

Andrew hinted that strict_strtoul is right fix. Check input properly
in backlight, echo > brightness should not turn off the backlight.

Signed-off-by: Pavel Machek <pavel@suse.cz>

diff --git a/drivers/video/backlight/backlight.c b/drivers/video/backlight/backlight.c
index 5c2e23d..e4c93fc 100644
--- a/drivers/video/backlight/backlight.c
+++ b/drivers/video/backlight/backlight.c
@@ -80,22 +80,18 @@ static ssize_t backlight_show_power(stru
 static ssize_t backlight_store_power(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
-	int rc = -ENXIO;
-	char *endp;
+	int rc;
 	struct backlight_device *bd = to_backlight_device(dev);
-	int power = simple_strtoul(buf, &endp, 0);
-	size_t size = endp - buf;
+	unsigned long power;
 
-	if (!size)
-		return -EINVAL;
-	if (*endp && isspace(*endp))
-		size++;
-	if (size != count)
-		return -EINVAL;
+	rc = strict_strtoul(buf, 0, &power);
+	if (rc)
+		return rc;
 
+	rc = -ENXIO;
 	mutex_lock(&bd->ops_lock);
 	if (bd->ops) {
-		pr_debug("backlight: set power to %d\n", power);
+		pr_debug("backlight: set power to %lu\n", power);
 		if (bd->props.power != power) {
 			bd->props.power = power;
 			backlight_update_status(bd);
@@ -118,23 +114,22 @@ static ssize_t backlight_show_brightness
 static ssize_t backlight_store_brightness(struct device *dev,
 		struct device_attribute *attr, const char *buf, size_t count)
 {
-	int rc = -ENXIO;
-	char *endp;
+	int rc;
 	struct backlight_device *bd = to_backlight_device(dev);
-	int brightness = simple_strtoul(buf, &endp, 0);
-	size_t size = endp - buf;
+	unsigned long brightness;
+
+ 	rc = strict_strtoul(buf, 0, &brightness);
+	if (rc)
+		return rc;
 
-	if (*endp && isspace(*endp))
-		size++;
-	if (size != count)
-		return -EINVAL;
+	rc = -ENXIO;
 
 	mutex_lock(&bd->ops_lock);
 	if (bd->ops) {
 		if (brightness > bd->props.max_brightness)
 			rc = -EINVAL;
 		else {
-			pr_debug("backlight: set brightness to %d\n",
+			pr_debug("backlight: set brightness to %lu\n",
 				 brightness);
 			if (bd->props.brightness != brightness) {
 				bd->props.brightness = brightness;


-- 
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html