All of lore.kernel.org
 help / color / mirror / Atom feed
* [patch] pm_qos_params: cleanup: terminate a string
@ 2010-09-03 12:41 ` Dan Carpenter
  0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2010-09-03 12:41 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: mark gross, James Bottomley, Frederic Weisbecker,
	Jonathan Corbet, linux-kernel, kernel-janitors

This is just a picky thing, but we pass an possibly unterminated string
to printk if debugging is turned on.  Also printk level is set to
"debug" by pr_debug() so the "KERN_ERR" isn't used.

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
index b7e4c36..310a51e 100644
--- a/kernel/pm_qos_params.c
+++ b/kernel/pm_qos_params.c
@@ -389,10 +389,11 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
 	} else if (count == 11) { /* len('0x12345678/0') */
 		if (copy_from_user(ascii_value, buf, 11))
 			return -EFAULT;
+		ascii_value[10] = '\0';
 		x = sscanf(ascii_value, "%x", &value);
 		if (x != 1)
 			return -EINVAL;
-		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
+		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
 	} else
 		return -EINVAL;
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* [patch] pm_qos_params: cleanup: terminate a string
@ 2010-09-03 12:41 ` Dan Carpenter
  0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2010-09-03 12:41 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: mark gross, James Bottomley, Frederic Weisbecker,
	Jonathan Corbet, linux-kernel, kernel-janitors

This is just a picky thing, but we pass an possibly unterminated string
to printk if debugging is turned on.  Also printk level is set to
"debug" by pr_debug() so the "KERN_ERR" isn't used.

Signed-off-by: Dan Carpenter <error27@gmail.com>

diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
index b7e4c36..310a51e 100644
--- a/kernel/pm_qos_params.c
+++ b/kernel/pm_qos_params.c
@@ -389,10 +389,11 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
 	} else if (count = 11) { /* len('0x12345678/0') */
 		if (copy_from_user(ascii_value, buf, 11))
 			return -EFAULT;
+		ascii_value[10] = '\0';
 		x = sscanf(ascii_value, "%x", &value);
 		if (x != 1)
 			return -EINVAL;
-		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
+		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
 	} else
 		return -EINVAL;
 

^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
  2010-09-03 12:41 ` Dan Carpenter
@ 2010-09-07  6:22   ` mark gross
  -1 siblings, 0 replies; 14+ messages in thread
From: mark gross @ 2010-09-07  6:22 UTC (permalink / raw)
  To: Dan Carpenter, Rafael J. Wysocki, mark gross, James Bottomley,
	Frederic Weisbecker, Jonathan Corbet, linux-kernel,
	kernel-janitors

On Fri, Sep 03, 2010 at 02:41:06PM +0200, Dan Carpenter wrote:
> This is just a picky thing, but we pass an possibly unterminated string
> to printk if debugging is turned on.  Also printk level is set to
> "debug" by pr_debug() so the "KERN_ERR" isn't used.

Picky is good.  But we should probably get the other pr_debug fixed and
return -EINVAL if the strlen of the ascii_value is not bigger than 10.

thanks for finding my screw up!


> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index b7e4c36..310a51e 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -389,10 +389,11 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
>  	} else if (count == 11) { /* len('0x12345678/0') */
>  		if (copy_from_user(ascii_value, buf, 11))
>  			return -EFAULT;
> +		ascii_value[10] = '\0';
>  		x = sscanf(ascii_value, "%x", &value);
>  		if (x != 1)
>  			return -EINVAL;
> -		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
> +		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
>  	} else
>  		return -EINVAL;
>  

Updated version of this patch:

--mark

Signed-off-by: mark gross <markgross@thegnar.org>

Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
 pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
 pointing out the problem!

---
 kernel/pm_qos_params.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
index f42d3f7..db4295a 100644
--- a/kernel/pm_qos_params.c
+++ b/kernel/pm_qos_params.c
@@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
 		call_notifier = 1;
 		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
 				extreme_value);
-		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
+		pr_debug("new target for qos %d is %d\n", pm_qos_class,
 			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
 	}
 	spin_unlock_irqrestore(&pm_qos_lock, flags);
@@ -374,10 +374,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
 	} else if (count == 11) { /* len('0x12345678/0') */
 		if (copy_from_user(ascii_value, buf, 11))
 			return -EFAULT;
+		if (strlen(ascii_value) > 10)
+			return -EINVAL;
 		x = sscanf(ascii_value, "%x", &value);
 		if (x != 1)
 			return -EINVAL;
-		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
+		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
 	} else
 		return -EINVAL;
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
@ 2010-09-07  6:22   ` mark gross
  0 siblings, 0 replies; 14+ messages in thread
From: mark gross @ 2010-09-07  6:22 UTC (permalink / raw)
  To: Dan Carpenter, Rafael J. Wysocki, mark gross, James Bottomley,
	Frederic Weisbecker, Jonathan Corbet, linux-kernel,
	kernel-janitors

On Fri, Sep 03, 2010 at 02:41:06PM +0200, Dan Carpenter wrote:
> This is just a picky thing, but we pass an possibly unterminated string
> to printk if debugging is turned on.  Also printk level is set to
> "debug" by pr_debug() so the "KERN_ERR" isn't used.

Picky is good.  But we should probably get the other pr_debug fixed and
return -EINVAL if the strlen of the ascii_value is not bigger than 10.

thanks for finding my screw up!


> 
> Signed-off-by: Dan Carpenter <error27@gmail.com>
> 
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index b7e4c36..310a51e 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -389,10 +389,11 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
>  	} else if (count = 11) { /* len('0x12345678/0') */
>  		if (copy_from_user(ascii_value, buf, 11))
>  			return -EFAULT;
> +		ascii_value[10] = '\0';
>  		x = sscanf(ascii_value, "%x", &value);
>  		if (x != 1)
>  			return -EINVAL;
> -		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
> +		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
>  	} else
>  		return -EINVAL;
>  

Updated version of this patch:

--mark

Signed-off-by: mark gross <markgross@thegnar.org>

Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
 pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
 pointing out the problem!

---
 kernel/pm_qos_params.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
index f42d3f7..db4295a 100644
--- a/kernel/pm_qos_params.c
+++ b/kernel/pm_qos_params.c
@@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
 		call_notifier = 1;
 		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
 				extreme_value);
-		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
+		pr_debug("new target for qos %d is %d\n", pm_qos_class,
 			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
 	}
 	spin_unlock_irqrestore(&pm_qos_lock, flags);
@@ -374,10 +374,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
 	} else if (count = 11) { /* len('0x12345678/0') */
 		if (copy_from_user(ascii_value, buf, 11))
 			return -EFAULT;
+		if (strlen(ascii_value) > 10)
+			return -EINVAL;
 		x = sscanf(ascii_value, "%x", &value);
 		if (x != 1)
 			return -EINVAL;
-		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
+		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
 	} else
 		return -EINVAL;
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
  2010-09-07  6:22   ` mark gross
@ 2010-09-07 13:38     ` mark gross
  -1 siblings, 0 replies; 14+ messages in thread
From: mark gross @ 2010-09-07 13:38 UTC (permalink / raw)
  To: mark gross
  Cc: Dan Carpenter, Rafael J. Wysocki, James Bottomley,
	Frederic Weisbecker, Jonathan Corbet, linux-kernel,
	kernel-janitors

On Mon, Sep 06, 2010 at 11:22:27PM -0700, mark gross wrote:
> On Fri, Sep 03, 2010 at 02:41:06PM +0200, Dan Carpenter wrote:
> > This is just a picky thing, but we pass an possibly unterminated string
> > to printk if debugging is turned on.  Also printk level is set to
> > "debug" by pr_debug() so the "KERN_ERR" isn't used.
> 
> Picky is good.  But we should probably get the other pr_debug fixed and
> return -EINVAL if the strlen of the ascii_value is not bigger than 10.
> 
> thanks for finding my screw up!
> 
> 
> > 
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > 
> > diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> > index b7e4c36..310a51e 100644
> > --- a/kernel/pm_qos_params.c
> > +++ b/kernel/pm_qos_params.c
> > @@ -389,10 +389,11 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
> >  	} else if (count == 11) { /* len('0x12345678/0') */
> >  		if (copy_from_user(ascii_value, buf, 11))
> >  			return -EFAULT;
> > +		ascii_value[10] = '\0';
> >  		x = sscanf(ascii_value, "%x", &value);
> >  		if (x != 1)
> >  			return -EINVAL;
> > -		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
> > +		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
> >  	} else
> >  		return -EINVAL;
> >  
> 
> Updated version of this patch:
> 
> --mark
> 
> Signed-off-by: mark gross <markgross@thegnar.org>
> 
> Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
>  pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
>  pointing out the problem!
> 
> ---
>  kernel/pm_qos_params.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index f42d3f7..db4295a 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
>  		call_notifier = 1;
>  		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
>  				extreme_value);
> -		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
> +		pr_debug("new target for qos %d is %d\n", pm_qos_class,
>  			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
>  	}
>  	spin_unlock_irqrestore(&pm_qos_lock, flags);
> @@ -374,10 +374,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
>  	} else if (count == 11) { /* len('0x12345678/0') */
>  		if (copy_from_user(ascii_value, buf, 11))
>  			return -EFAULT;
> +		if (strlen(ascii_value) > 10)
                         should be !=

> +			return -EINVAL;
>  		x = sscanf(ascii_value, "%x", &value);
>  		if (x != 1)
>  			return -EINVAL;
> -		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
> +		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
>  	} else
>  		return -EINVAL;
>  
> -- 
> 1.7.0.4

updated patch 

Signed-off-by: mark gross <markgross@thegnar.org>

--mgross 

Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
 pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
 pointing out the problem!

---
 kernel/pm_qos_params.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
index f42d3f7..aae58d2 100644
--- a/kernel/pm_qos_params.c
+++ b/kernel/pm_qos_params.c
@@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
 		call_notifier = 1;
 		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
 				extreme_value);
-		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
+		pr_debug("new target for qos %d is %d\n", pm_qos_class,
 			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
 	}
 	spin_unlock_irqrestore(&pm_qos_lock, flags);
@@ -374,10 +374,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
 	} else if (count == 11) { /* len('0x12345678/0') */
 		if (copy_from_user(ascii_value, buf, 11))
 			return -EFAULT;
+		if (strlen(ascii_value) != 10)
+			return -EINVAL;
 		x = sscanf(ascii_value, "%x", &value);
 		if (x != 1)
 			return -EINVAL;
-		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
+		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
 	} else
 		return -EINVAL;
 
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
@ 2010-09-07 13:38     ` mark gross
  0 siblings, 0 replies; 14+ messages in thread
From: mark gross @ 2010-09-07 13:38 UTC (permalink / raw)
  To: mark gross
  Cc: Dan Carpenter, Rafael J. Wysocki, James Bottomley,
	Frederic Weisbecker, Jonathan Corbet, linux-kernel,
	kernel-janitors

On Mon, Sep 06, 2010 at 11:22:27PM -0700, mark gross wrote:
> On Fri, Sep 03, 2010 at 02:41:06PM +0200, Dan Carpenter wrote:
> > This is just a picky thing, but we pass an possibly unterminated string
> > to printk if debugging is turned on.  Also printk level is set to
> > "debug" by pr_debug() so the "KERN_ERR" isn't used.
> 
> Picky is good.  But we should probably get the other pr_debug fixed and
> return -EINVAL if the strlen of the ascii_value is not bigger than 10.
> 
> thanks for finding my screw up!
> 
> 
> > 
> > Signed-off-by: Dan Carpenter <error27@gmail.com>
> > 
> > diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> > index b7e4c36..310a51e 100644
> > --- a/kernel/pm_qos_params.c
> > +++ b/kernel/pm_qos_params.c
> > @@ -389,10 +389,11 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
> >  	} else if (count = 11) { /* len('0x12345678/0') */
> >  		if (copy_from_user(ascii_value, buf, 11))
> >  			return -EFAULT;
> > +		ascii_value[10] = '\0';
> >  		x = sscanf(ascii_value, "%x", &value);
> >  		if (x != 1)
> >  			return -EINVAL;
> > -		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
> > +		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
> >  	} else
> >  		return -EINVAL;
> >  
> 
> Updated version of this patch:
> 
> --mark
> 
> Signed-off-by: mark gross <markgross@thegnar.org>
> 
> Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
>  pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
>  pointing out the problem!
> 
> ---
>  kernel/pm_qos_params.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index f42d3f7..db4295a 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
>  		call_notifier = 1;
>  		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
>  				extreme_value);
> -		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
> +		pr_debug("new target for qos %d is %d\n", pm_qos_class,
>  			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
>  	}
>  	spin_unlock_irqrestore(&pm_qos_lock, flags);
> @@ -374,10 +374,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
>  	} else if (count = 11) { /* len('0x12345678/0') */
>  		if (copy_from_user(ascii_value, buf, 11))
>  			return -EFAULT;
> +		if (strlen(ascii_value) > 10)
                         should be !
> +			return -EINVAL;
>  		x = sscanf(ascii_value, "%x", &value);
>  		if (x != 1)
>  			return -EINVAL;
> -		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
> +		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
>  	} else
>  		return -EINVAL;
>  
> -- 
> 1.7.0.4

updated patch 

Signed-off-by: mark gross <markgross@thegnar.org>

--mgross 

Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
 pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
 pointing out the problem!

---
 kernel/pm_qos_params.c |    6 ++++--
 1 files changed, 4 insertions(+), 2 deletions(-)

diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
index f42d3f7..aae58d2 100644
--- a/kernel/pm_qos_params.c
+++ b/kernel/pm_qos_params.c
@@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
 		call_notifier = 1;
 		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
 				extreme_value);
-		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
+		pr_debug("new target for qos %d is %d\n", pm_qos_class,
 			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
 	}
 	spin_unlock_irqrestore(&pm_qos_lock, flags);
@@ -374,10 +374,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
 	} else if (count = 11) { /* len('0x12345678/0') */
 		if (copy_from_user(ascii_value, buf, 11))
 			return -EFAULT;
+		if (strlen(ascii_value) != 10)
+			return -EINVAL;
 		x = sscanf(ascii_value, "%x", &value);
 		if (x != 1)
 			return -EINVAL;
-		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
+		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
 	} else
 		return -EINVAL;
 
-- 
1.7.0.4



^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
  2010-09-07 13:38     ` mark gross
@ 2010-09-07 21:38       ` Dan Carpenter
  -1 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2010-09-07 21:38 UTC (permalink / raw)
  To: mark gross
  Cc: Rafael J. Wysocki, James Bottomley, Frederic Weisbecker,
	Jonathan Corbet, linux-kernel, kernel-janitors

On Tue, Sep 07, 2010 at 06:38:05AM -0700, mark gross wrote:
> >  	spin_unlock_irqrestore(&pm_qos_lock, flags);
> > @@ -374,10 +374,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
> >  	} else if (count == 11) { /* len('0x12345678/0') */
> >  		if (copy_from_user(ascii_value, buf, 11))
> >  			return -EFAULT;
> > +		if (strlen(ascii_value) > 10)
>                          should be !=
> 
> > +			return -EINVAL;
> >  		x = sscanf(ascii_value, "%x", &value);
> >  		if (x != 1)
> >  			return -EINVAL;

With the original code you could do:
	char buf[11];  /* must be 11 chars */

	snprintf(buf, sizeof(buf), "0x%x", 42);
	write(fd, buf, sizeof(buf));

But the new code is stricter so the number would have to be zero padded.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
@ 2010-09-07 21:38       ` Dan Carpenter
  0 siblings, 0 replies; 14+ messages in thread
From: Dan Carpenter @ 2010-09-07 21:38 UTC (permalink / raw)
  To: mark gross
  Cc: Rafael J. Wysocki, James Bottomley, Frederic Weisbecker,
	Jonathan Corbet, linux-kernel, kernel-janitors

On Tue, Sep 07, 2010 at 06:38:05AM -0700, mark gross wrote:
> >  	spin_unlock_irqrestore(&pm_qos_lock, flags);
> > @@ -374,10 +374,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
> >  	} else if (count = 11) { /* len('0x12345678/0') */
> >  		if (copy_from_user(ascii_value, buf, 11))
> >  			return -EFAULT;
> > +		if (strlen(ascii_value) > 10)
>                          should be !> 
> > +			return -EINVAL;
> >  		x = sscanf(ascii_value, "%x", &value);
> >  		if (x != 1)
> >  			return -EINVAL;

With the original code you could do:
	char buf[11];  /* must be 11 chars */

	snprintf(buf, sizeof(buf), "0x%x", 42);
	write(fd, buf, sizeof(buf));

But the new code is stricter so the number would have to be zero padded.

regards,
dan carpenter

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
  2010-09-07 13:38     ` mark gross
@ 2010-09-08 22:13       ` Rafael J. Wysocki
  -1 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2010-09-08 22:13 UTC (permalink / raw)
  To: markgross
  Cc: Dan Carpenter, James Bottomley, Frederic Weisbecker,
	Jonathan Corbet, linux-kernel, kernel-janitors

On Tuesday, September 07, 2010, mark gross wrote:
> On Mon, Sep 06, 2010 at 11:22:27PM -0700, mark gross wrote:
> > On Fri, Sep 03, 2010 at 02:41:06PM +0200, Dan Carpenter wrote:
...
> 
> updated patch 
> 
> Signed-off-by: mark gross <markgross@thegnar.org>
> 
> --mgross 
> 
> Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
>  pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
>  pointing out the problem!
> 
> ---
>  kernel/pm_qos_params.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index f42d3f7..aae58d2 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
>  		call_notifier = 1;
>  		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
>  				extreme_value);
> -		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
> +		pr_debug("new target for qos %d is %d\n", pm_qos_class,
>  			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
>  	}
>  	spin_unlock_irqrestore(&pm_qos_lock, flags);

The above doesn't apply to the Linus' tree any more.  Care to fix?

Rafael

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
@ 2010-09-08 22:13       ` Rafael J. Wysocki
  0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2010-09-08 22:13 UTC (permalink / raw)
  To: markgross
  Cc: Dan Carpenter, James Bottomley, Frederic Weisbecker,
	Jonathan Corbet, linux-kernel, kernel-janitors

On Tuesday, September 07, 2010, mark gross wrote:
> On Mon, Sep 06, 2010 at 11:22:27PM -0700, mark gross wrote:
> > On Fri, Sep 03, 2010 at 02:41:06PM +0200, Dan Carpenter wrote:
...
> 
> updated patch 
> 
> Signed-off-by: mark gross <markgross@thegnar.org>
> 
> --mgross 
> 
> Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
>  pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
>  pointing out the problem!
> 
> ---
>  kernel/pm_qos_params.c |    6 ++++--
>  1 files changed, 4 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index f42d3f7..aae58d2 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
>  		call_notifier = 1;
>  		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
>  				extreme_value);
> -		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
> +		pr_debug("new target for qos %d is %d\n", pm_qos_class,
>  			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
>  	}
>  	spin_unlock_irqrestore(&pm_qos_lock, flags);

The above doesn't apply to the Linus' tree any more.  Care to fix?

Rafael

^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
  2010-09-08 22:13       ` Rafael J. Wysocki
@ 2010-09-09  2:56         ` mark gross
  -1 siblings, 0 replies; 14+ messages in thread
From: mark gross @ 2010-09-09  2:56 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: markgross, Dan Carpenter, James Bottomley, Frederic Weisbecker,
	Jonathan Corbet, linux-kernel, kernel-janitors

On Thu, Sep 09, 2010 at 12:13:59AM +0200, Rafael J. Wysocki wrote:
> On Tuesday, September 07, 2010, mark gross wrote:
> > On Mon, Sep 06, 2010 at 11:22:27PM -0700, mark gross wrote:
> > > On Fri, Sep 03, 2010 at 02:41:06PM +0200, Dan Carpenter wrote:
> ...
> > 
> > updated patch 
> > 
> > Signed-off-by: mark gross <markgross@thegnar.org>
> > 
> > --mgross 
> > 
> > Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
> >  pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
> >  pointing out the problem!
> > 
> > ---
> >  kernel/pm_qos_params.c |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> > index f42d3f7..aae58d2 100644
> > --- a/kernel/pm_qos_params.c
> > +++ b/kernel/pm_qos_params.c
> > @@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
> >  		call_notifier = 1;
> >  		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
> >  				extreme_value);
> > -		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
> > +		pr_debug("new target for qos %d is %d\n", pm_qos_class,
> >  			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
> >  	}
> >  	spin_unlock_irqrestore(&pm_qos_lock, flags);
> 
> The above doesn't apply to the Linus' tree any more.  Care to fix?
> 
> Rafael

no problem.  (I screwed up on my fetch and forgot to merge the linus
remote....) This bit doesn't apply post 2.6.35.
--mgross

Signed-off-by: mark gross <markgross@thegnar.org)


>From 64753f68e2baaef21783192566469abc54aca90d Mon Sep 17 00:00:00 2001
From: mgross <mgross@mgross-laptop.(none)>
Date: Wed, 8 Sep 2010 19:52:39 -0700
Subject: [PATCH] make it harder to misuse the pm_qos user mode api.

---
 kernel/pm_qos_params.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
index b7e4c36..645e541 100644
--- a/kernel/pm_qos_params.c
+++ b/kernel/pm_qos_params.c
@@ -389,10 +389,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
 	} else if (count == 11) { /* len('0x12345678/0') */
 		if (copy_from_user(ascii_value, buf, 11))
 			return -EFAULT;
+		if (strlen(ascii_value) != 10)
+			return -EINVAL;
 		x = sscanf(ascii_value, "%x", &value);
 		if (x != 1)
 			return -EINVAL;
-		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
+		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
 	} else
 		return -EINVAL;
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
@ 2010-09-09  2:56         ` mark gross
  0 siblings, 0 replies; 14+ messages in thread
From: mark gross @ 2010-09-09  2:56 UTC (permalink / raw)
  To: Rafael J. Wysocki
  Cc: markgross, Dan Carpenter, James Bottomley, Frederic Weisbecker,
	Jonathan Corbet, linux-kernel, kernel-janitors

On Thu, Sep 09, 2010 at 12:13:59AM +0200, Rafael J. Wysocki wrote:
> On Tuesday, September 07, 2010, mark gross wrote:
> > On Mon, Sep 06, 2010 at 11:22:27PM -0700, mark gross wrote:
> > > On Fri, Sep 03, 2010 at 02:41:06PM +0200, Dan Carpenter wrote:
> ...
> > 
> > updated patch 
> > 
> > Signed-off-by: mark gross <markgross@thegnar.org>
> > 
> > --mgross 
> > 
> > Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
> >  pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
> >  pointing out the problem!
> > 
> > ---
> >  kernel/pm_qos_params.c |    6 ++++--
> >  1 files changed, 4 insertions(+), 2 deletions(-)
> > 
> > diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> > index f42d3f7..aae58d2 100644
> > --- a/kernel/pm_qos_params.c
> > +++ b/kernel/pm_qos_params.c
> > @@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
> >  		call_notifier = 1;
> >  		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
> >  				extreme_value);
> > -		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
> > +		pr_debug("new target for qos %d is %d\n", pm_qos_class,
> >  			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
> >  	}
> >  	spin_unlock_irqrestore(&pm_qos_lock, flags);
> 
> The above doesn't apply to the Linus' tree any more.  Care to fix?
> 
> Rafael

no problem.  (I screwed up on my fetch and forgot to merge the linus
remote....) This bit doesn't apply post 2.6.35.
--mgross

Signed-off-by: mark gross <markgross@thegnar.org)


From 64753f68e2baaef21783192566469abc54aca90d Mon Sep 17 00:00:00 2001
From: mgross <mgross@mgross-laptop.(none)>
Date: Wed, 8 Sep 2010 19:52:39 -0700
Subject: [PATCH] make it harder to misuse the pm_qos user mode api.

---
 kernel/pm_qos_params.c |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
index b7e4c36..645e541 100644
--- a/kernel/pm_qos_params.c
+++ b/kernel/pm_qos_params.c
@@ -389,10 +389,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
 	} else if (count = 11) { /* len('0x12345678/0') */
 		if (copy_from_user(ascii_value, buf, 11))
 			return -EFAULT;
+		if (strlen(ascii_value) != 10)
+			return -EINVAL;
 		x = sscanf(ascii_value, "%x", &value);
 		if (x != 1)
 			return -EINVAL;
-		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
+		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
 	} else
 		return -EINVAL;
 
-- 
1.7.0.4


^ permalink raw reply related	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
  2010-09-09  2:56         ` mark gross
@ 2010-09-09 21:21           ` Rafael J. Wysocki
  -1 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2010-09-09 21:21 UTC (permalink / raw)
  To: markgross
  Cc: Dan Carpenter, James Bottomley, Frederic Weisbecker,
	Jonathan Corbet, linux-kernel, kernel-janitors

On Thursday, September 09, 2010, mark gross wrote:
> On Thu, Sep 09, 2010 at 12:13:59AM +0200, Rafael J. Wysocki wrote:
> > On Tuesday, September 07, 2010, mark gross wrote:
> > > On Mon, Sep 06, 2010 at 11:22:27PM -0700, mark gross wrote:
> > > > On Fri, Sep 03, 2010 at 02:41:06PM +0200, Dan Carpenter wrote:
> > ...
> > > 
> > > updated patch 
> > > 
> > > Signed-off-by: mark gross <markgross@thegnar.org>
> > > 
> > > --mgross 
> > > 
> > > Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
> > >  pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
> > >  pointing out the problem!
> > > 
> > > ---
> > >  kernel/pm_qos_params.c |    6 ++++--
> > >  1 files changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> > > index f42d3f7..aae58d2 100644
> > > --- a/kernel/pm_qos_params.c
> > > +++ b/kernel/pm_qos_params.c
> > > @@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
> > >  		call_notifier = 1;
> > >  		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
> > >  				extreme_value);
> > > -		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
> > > +		pr_debug("new target for qos %d is %d\n", pm_qos_class,
> > >  			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
> > >  	}
> > >  	spin_unlock_irqrestore(&pm_qos_lock, flags);
> > 
> > The above doesn't apply to the Linus' tree any more.  Care to fix?
> > 
> > Rafael
> 
> no problem.  (I screwed up on my fetch and forgot to merge the linus
> remote....) This bit doesn't apply post 2.6.35.
> --mgross

Thanks, applied to suspend-2.6/linux-next .

Rafael


> Signed-off-by: mark gross <markgross@thegnar.org)
> 
> 
> From 64753f68e2baaef21783192566469abc54aca90d Mon Sep 17 00:00:00 2001
> From: mgross <mgross@mgross-laptop.(none)>
> Date: Wed, 8 Sep 2010 19:52:39 -0700
> Subject: [PATCH] make it harder to misuse the pm_qos user mode api.
> 
> ---
>  kernel/pm_qos_params.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index b7e4c36..645e541 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -389,10 +389,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
>  	} else if (count == 11) { /* len('0x12345678/0') */
>  		if (copy_from_user(ascii_value, buf, 11))
>  			return -EFAULT;
> +		if (strlen(ascii_value) != 10)
> +			return -EINVAL;
>  		x = sscanf(ascii_value, "%x", &value);
>  		if (x != 1)
>  			return -EINVAL;
> -		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
> +		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
>  	} else
>  		return -EINVAL;
>  
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread
* Re: [patch] pm_qos_params: cleanup: terminate a string
@ 2010-09-09 21:21           ` Rafael J. Wysocki
  0 siblings, 0 replies; 14+ messages in thread
From: Rafael J. Wysocki @ 2010-09-09 21:21 UTC (permalink / raw)
  To: markgross
  Cc: Dan Carpenter, James Bottomley, Frederic Weisbecker,
	Jonathan Corbet, linux-kernel, kernel-janitors

On Thursday, September 09, 2010, mark gross wrote:
> On Thu, Sep 09, 2010 at 12:13:59AM +0200, Rafael J. Wysocki wrote:
> > On Tuesday, September 07, 2010, mark gross wrote:
> > > On Mon, Sep 06, 2010 at 11:22:27PM -0700, mark gross wrote:
> > > > On Fri, Sep 03, 2010 at 02:41:06PM +0200, Dan Carpenter wrote:
> > ...
> > > 
> > > updated patch 
> > > 
> > > Signed-off-by: mark gross <markgross@thegnar.org>
> > > 
> > > --mgross 
> > > 
> > > Subject: [PATCH] correct some pr_debug misuse and add a stronger parrameter check to
> > >  pm_qos_write for the ascii hex value case.  Thanks to Dan Carpenter for
> > >  pointing out the problem!
> > > 
> > > ---
> > >  kernel/pm_qos_params.c |    6 ++++--
> > >  1 files changed, 4 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> > > index f42d3f7..aae58d2 100644
> > > --- a/kernel/pm_qos_params.c
> > > +++ b/kernel/pm_qos_params.c
> > > @@ -155,7 +155,7 @@ static void update_target(int pm_qos_class)
> > >  		call_notifier = 1;
> > >  		atomic_set(&pm_qos_array[pm_qos_class]->target_value,
> > >  				extreme_value);
> > > -		pr_debug(KERN_ERR "new target for qos %d is %d\n", pm_qos_class,
> > > +		pr_debug("new target for qos %d is %d\n", pm_qos_class,
> > >  			atomic_read(&pm_qos_array[pm_qos_class]->target_value));
> > >  	}
> > >  	spin_unlock_irqrestore(&pm_qos_lock, flags);
> > 
> > The above doesn't apply to the Linus' tree any more.  Care to fix?
> > 
> > Rafael
> 
> no problem.  (I screwed up on my fetch and forgot to merge the linus
> remote....) This bit doesn't apply post 2.6.35.
> --mgross

Thanks, applied to suspend-2.6/linux-next .

Rafael


> Signed-off-by: mark gross <markgross@thegnar.org)
> 
> 
> From 64753f68e2baaef21783192566469abc54aca90d Mon Sep 17 00:00:00 2001
> From: mgross <mgross@mgross-laptop.(none)>
> Date: Wed, 8 Sep 2010 19:52:39 -0700
> Subject: [PATCH] make it harder to misuse the pm_qos user mode api.
> 
> ---
>  kernel/pm_qos_params.c |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
> 
> diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c
> index b7e4c36..645e541 100644
> --- a/kernel/pm_qos_params.c
> +++ b/kernel/pm_qos_params.c
> @@ -389,10 +389,12 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf,
>  	} else if (count = 11) { /* len('0x12345678/0') */
>  		if (copy_from_user(ascii_value, buf, 11))
>  			return -EFAULT;
> +		if (strlen(ascii_value) != 10)
> +			return -EINVAL;
>  		x = sscanf(ascii_value, "%x", &value);
>  		if (x != 1)
>  			return -EINVAL;
> -		pr_debug(KERN_ERR "%s, %d, 0x%x\n", ascii_value, x, value);
> +		pr_debug("%s, %d, 0x%x\n", ascii_value, x, value);
>  	} else
>  		return -EINVAL;
>  
> 


^ permalink raw reply	[flat|nested] 14+ messages in thread
end of thread, other threads:[~2010-09-09 21:22 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2010-09-03 12:41 [patch] pm_qos_params: cleanup: terminate a string Dan Carpenter
2010-09-03 12:41 ` Dan Carpenter
2010-09-07  6:22 ` mark gross
2010-09-07  6:22   ` mark gross
2010-09-07 13:38   ` mark gross
2010-09-07 13:38     ` mark gross
2010-09-07 21:38     ` Dan Carpenter
2010-09-07 21:38       ` Dan Carpenter
2010-09-08 22:13     ` Rafael J. Wysocki
2010-09-08 22:13       ` Rafael J. Wysocki
2010-09-09  2:56       ` mark gross
2010-09-09  2:56         ` mark gross
2010-09-09 21:21         ` Rafael J. Wysocki
2010-09-09 21:21           ` Rafael J. Wysocki

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.