On Fri, Feb 18, 2011 at 10:54:56AM +0900, Simon Horman wrote: > In "PM QoS: Correct pr_debug() misuse and improve parameter checks" > the parsing of the ASCII hex value was tightened. Unfortunately > it was tightened to the point where no value is valid. How is it of no value? Can you not sent a 10 char string with a zero-byte end of string maker? Is expecting the c-string marker in the interface a bad idea that needs fixing? > Root of the problem seems to lie in wheather the ASCII hex is followed > by a '\n' or not. My reading of the documentation is that the '\n' should > not be present. However the code previously only accepted that version. > The current code accepts neither. My fix is to accept both. The documentation says it should be a 10byte string. "Alternatively the user mode program could write a hex string for the value using 10 char long format e.g. "0x12345678". This translates to a pm_qos_update_request call." When I wrote the code I was thinking that a c-string that has a zero byte end of string maker. note: a new line character is not implied anywhere. However; reading the documentation it is somewhat ambiguous the buffer is 10 bytes or 11. We should tighten up the Documentation and make the code match it. I don't like the 10 or 11 byte buffer logic. It should be one or the other and not include any new line character. What do you want the documentation to read? --mark > > Cc: Mark Gross <markgross@xxxxxxxxxxx> > Cc: Dan Carpenter <error27@xxxxxxxxx> > Cc: Rafael J. Wysocki <rjw@xxxxxxx> > Signed-off-by: Simon Horman <horms@xxxxxxxxxxxx> > > --- > This appears to have been introduced around 2.6.36-rc4. > And was an @stable patch. As such I believe this change > is stable material. > --- > kernel/pm_qos_params.c | 7 ++++--- > 1 files changed, 4 insertions(+), 3 deletions(-) > > diff --git a/kernel/pm_qos_params.c b/kernel/pm_qos_params.c > index aeaa7f8..98a34ea 100644 > --- a/kernel/pm_qos_params.c > +++ b/kernel/pm_qos_params.c > @@ -387,10 +387,11 @@ static ssize_t pm_qos_power_write(struct file *filp, const char __user *buf, > if (count == sizeof(s32)) { > if (copy_from_user(&value, buf, sizeof(s32))) > return -EFAULT; > - } else if (count == 11) { /* len('0x12345678/0') */ > - if (copy_from_user(ascii_value, buf, 11)) > + } else if (count == 11 || count == 10) { /* len('0x12345678\n') or > + * len('0x12345678') */ > + if (copy_from_user(ascii_value, buf, count)) > return -EFAULT; > - if (strlen(ascii_value) != 10) > + if (strlen(ascii_value) != count) > return -EINVAL; > x = sscanf(ascii_value, "%x", &value); > if (x != 1) > -- > 1.7.2.3 > _______________________________________________ linux-pm mailing list linux-pm@xxxxxxxxxxxxxxxxxxxxxxxxxx https://lists.linux-foundation.org/mailman/listinfo/linux-pm