* [PATCH] cpufreq: warn about invalid vals to scaling_max/min_freq interfaces
@ 2023-03-16 3:15 qinyu
2023-03-16 3:36 ` Viresh Kumar
` (2 more replies)
0 siblings, 3 replies; 7+ messages in thread
From: qinyu @ 2023-03-16 3:15 UTC (permalink / raw)
To: rafael, viresh.kumar; +Cc: linux-pm, zhangxiaofeng46, hewenliang4, linfeilong
When echo an invalid val to scaling_min_freq:
> echo 123abc123 > scaling_min_freq
It looks weird to have a return val of 0:
> echo $?
> 0
Sane people won't echo strings like that into these interfaces but fuzz
tests may do. Also, maybe it's better to inform people if input is invalid
or out of range.
After this:
> echo 123abc123 > scaling_min_freq
> -bash: echo: write error: Invalid argument
Signed-off-by: qinyu <qinyu32@huawei.com>
Tested-by: zhangxiaofeng <zhangxiaofeng46@huawei.com>
---
drivers/cpufreq/cpufreq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 6d8fd3b8d..d61f7308f 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -725,9 +725,9 @@ static ssize_t store_##file_name \
unsigned long val; \
int ret; \
\
- ret = sscanf(buf, "%lu", &val); \
- if (ret != 1) \
- return -EINVAL; \
+ ret = kstrtoul(buf, 0, &val); \
+ if (ret) \
+ return ret; \
\
ret = freq_qos_update_request(policy->object##_freq_req, val);\
return ret >= 0 ? count : ret; \
--
2.33.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] cpufreq: warn about invalid vals to scaling_max/min_freq interfaces
2023-03-16 3:15 [PATCH] cpufreq: warn about invalid vals to scaling_max/min_freq interfaces qinyu
@ 2023-03-16 3:36 ` Viresh Kumar
2023-03-17 11:47 ` Russell Haley
2023-03-17 15:41 ` Rafael J. Wysocki
2 siblings, 0 replies; 7+ messages in thread
From: Viresh Kumar @ 2023-03-16 3:36 UTC (permalink / raw)
To: qinyu; +Cc: rafael, linux-pm, zhangxiaofeng46, hewenliang4, linfeilong
On 16-03-23, 11:15, qinyu wrote:
> When echo an invalid val to scaling_min_freq:
> > echo 123abc123 > scaling_min_freq
> It looks weird to have a return val of 0:
> > echo $?
> > 0
>
> Sane people won't echo strings like that into these interfaces but fuzz
> tests may do. Also, maybe it's better to inform people if input is invalid
> or out of range.
>
> After this:
> > echo 123abc123 > scaling_min_freq
> > -bash: echo: write error: Invalid argument
>
> Signed-off-by: qinyu <qinyu32@huawei.com>
> Tested-by: zhangxiaofeng <zhangxiaofeng46@huawei.com>
> ---
> drivers/cpufreq/cpufreq.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 6d8fd3b8d..d61f7308f 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -725,9 +725,9 @@ static ssize_t store_##file_name \
> unsigned long val; \
> int ret; \
> \
> - ret = sscanf(buf, "%lu", &val); \
> - if (ret != 1) \
> - return -EINVAL; \
> + ret = kstrtoul(buf, 0, &val); \
> + if (ret) \
> + return ret; \
> \
> ret = freq_qos_update_request(policy->object##_freq_req, val);\
> return ret >= 0 ? count : ret; \
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
--
viresh
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cpufreq: warn about invalid vals to scaling_max/min_freq interfaces
2023-03-16 3:15 [PATCH] cpufreq: warn about invalid vals to scaling_max/min_freq interfaces qinyu
2023-03-16 3:36 ` Viresh Kumar
@ 2023-03-17 11:47 ` Russell Haley
2023-03-20 7:36 ` qinyu
2023-03-17 15:41 ` Rafael J. Wysocki
2 siblings, 1 reply; 7+ messages in thread
From: Russell Haley @ 2023-03-17 11:47 UTC (permalink / raw)
To: qinyu, rafael, viresh.kumar
Cc: linux-pm, zhangxiaofeng46, hewenliang4, linfeilong
On 3/15/23 22:15, qinyu wrote:
> When echo an invalid val to scaling_min_freq:
>> echo 123abc123 > scaling_min_freq
> It looks weird to have a return val of 0:
>> echo $?
>> 0
>
> Sane people won't echo strings like that into these interfaces but fuzz
> tests may do. Also, maybe it's better to inform people if input is invalid
> or out of range.
AFAICT, the patch doesn't actually cause it to error if the input is out
of range. So the commit message should not be worded to imply that it does.
It is good that it doesn't, I think, because someone might have a
fail-on-unhandled-error program (ex: shell script) that writes this file
deployed on hardware with different cpuinfo_max_freq. A new
unanticipated error would cause such a program to crash where it hadn't
before.
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cpufreq: warn about invalid vals to scaling_max/min_freq interfaces
2023-03-16 3:15 [PATCH] cpufreq: warn about invalid vals to scaling_max/min_freq interfaces qinyu
2023-03-16 3:36 ` Viresh Kumar
2023-03-17 11:47 ` Russell Haley
@ 2023-03-17 15:41 ` Rafael J. Wysocki
2023-03-20 8:17 ` [PATCH v2] " qinyu
2 siblings, 1 reply; 7+ messages in thread
From: Rafael J. Wysocki @ 2023-03-17 15:41 UTC (permalink / raw)
To: qinyu
Cc: rafael, viresh.kumar, linux-pm, zhangxiaofeng46, hewenliang4, linfeilong
On Thu, Mar 16, 2023 at 4:11 AM qinyu <qinyu32@huawei.com> wrote:
>
> When echo an invalid val to scaling_min_freq:
> > echo 123abc123 > scaling_min_freq
> It looks weird to have a return val of 0:
> > echo $?
> > 0
>
> Sane people won't echo strings like that into these interfaces but fuzz
> tests may do. Also, maybe it's better to inform people if input is invalid
> or out of range.
>
> After this:
> > echo 123abc123 > scaling_min_freq
> > -bash: echo: write error: Invalid argument
>
> Signed-off-by: qinyu <qinyu32@huawei.com>
> Tested-by: zhangxiaofeng <zhangxiaofeng46@huawei.com>
The changelog doesn't match the actual code changes. Please make them
match each other.
> ---
> drivers/cpufreq/cpufreq.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 6d8fd3b8d..d61f7308f 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -725,9 +725,9 @@ static ssize_t store_##file_name \
> unsigned long val; \
> int ret; \
> \
> - ret = sscanf(buf, "%lu", &val); \
> - if (ret != 1) \
> - return -EINVAL; \
> + ret = kstrtoul(buf, 0, &val); \
> + if (ret) \
> + return ret; \
> \
> ret = freq_qos_update_request(policy->object##_freq_req, val);\
> return ret >= 0 ? count : ret; \
> --
> 2.33.0
>
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] cpufreq: warn about invalid vals to scaling_max/min_freq interfaces
2023-03-17 11:47 ` Russell Haley
@ 2023-03-20 7:36 ` qinyu
0 siblings, 0 replies; 7+ messages in thread
From: qinyu @ 2023-03-20 7:36 UTC (permalink / raw)
To: Russell Haley, rafael, viresh.kumar
Cc: linux-pm, zhangxiaofeng46, hewenliang4, linfeilong
On 2023/3/17 19:47, Russell Haley wrote:
> On 3/15/23 22:15, qinyu wrote:
>> When echo an invalid val to scaling_min_freq:
>>> echo 123abc123 > scaling_min_freq
>> It looks weird to have a return val of 0:
>>> echo $?
>>> 0
>>
>> Sane people won't echo strings like that into these interfaces but fuzz
>> tests may do. Also, maybe it's better to inform people if input is invalid
>> or out of range.
>
> AFAICT, the patch doesn't actually cause it to error if the input is out
> of range. So the commit message should not be worded to imply that it does.
>
> It is good that it doesn't, I think, because someone might have a
> fail-on-unhandled-error program (ex: shell script) that writes this file
> deployed on hardware with different cpuinfo_max_freq. A new
> unanticipated error would cause such a program to crash where it hadn't
> before.
>
By saying out of range what I mean is out of range of an unsigned long var, not the frequency range. Sorry for the ambiguous description, I will delete this part and resend the patch.
^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH v2] cpufreq: warn about invalid vals to scaling_max/min_freq interfaces
2023-03-17 15:41 ` Rafael J. Wysocki
@ 2023-03-20 8:17 ` qinyu
2023-03-22 19:12 ` Rafael J. Wysocki
0 siblings, 1 reply; 7+ messages in thread
From: qinyu @ 2023-03-20 8:17 UTC (permalink / raw)
To: rafael
Cc: hewenliang4, linfeilong, linux-pm, qinyu32, viresh.kumar,
zhangxiaofeng46, yumpusamongus
When echo an invalid val to scaling_min_freq:
> echo 123abc123 > scaling_min_freq
It looks weird to have a return val of 0:
> echo $?
> 0
Sane people won't echo strings like that into these interfaces but fuzz
tests may do. Also, maybe it's better to inform people if input is
invalid.
After this:
> echo 123abc123 > scaling_min_freq
> -bash: echo: write error: Invalid argument
Signed-off-by: qinyu <qinyu32@huawei.com>
Tested-by: zhangxiaofeng <zhangxiaofeng46@huawei.com>
Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
---
v2:
- reword commit message to match the actual code changes.
drivers/cpufreq/cpufreq.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 6d8fd3b8d..d61f7308f 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -725,9 +725,9 @@ static ssize_t store_##file_name \
unsigned long val; \
int ret; \
\
- ret = sscanf(buf, "%lu", &val); \
- if (ret != 1) \
- return -EINVAL; \
+ ret = kstrtoul(buf, 0, &val); \
+ if (ret) \
+ return ret; \
\
ret = freq_qos_update_request(policy->object##_freq_req, val);\
return ret >= 0 ? count : ret; \
--
2.33.0
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2] cpufreq: warn about invalid vals to scaling_max/min_freq interfaces
2023-03-20 8:17 ` [PATCH v2] " qinyu
@ 2023-03-22 19:12 ` Rafael J. Wysocki
0 siblings, 0 replies; 7+ messages in thread
From: Rafael J. Wysocki @ 2023-03-22 19:12 UTC (permalink / raw)
To: qinyu
Cc: rafael, hewenliang4, linfeilong, linux-pm, viresh.kumar,
zhangxiaofeng46, yumpusamongus
On Mon, Mar 20, 2023 at 9:09 AM qinyu <qinyu32@huawei.com> wrote:
>
> When echo an invalid val to scaling_min_freq:
> > echo 123abc123 > scaling_min_freq
> It looks weird to have a return val of 0:
> > echo $?
> > 0
>
> Sane people won't echo strings like that into these interfaces but fuzz
> tests may do. Also, maybe it's better to inform people if input is
> invalid.
>
> After this:
> > echo 123abc123 > scaling_min_freq
> > -bash: echo: write error: Invalid argument
>
> Signed-off-by: qinyu <qinyu32@huawei.com>
> Tested-by: zhangxiaofeng <zhangxiaofeng46@huawei.com>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>
> ---
> v2:
> - reword commit message to match the actual code changes.
> drivers/cpufreq/cpufreq.c | 6 +++---
> 1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 6d8fd3b8d..d61f7308f 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -725,9 +725,9 @@ static ssize_t store_##file_name \
> unsigned long val; \
> int ret; \
> \
> - ret = sscanf(buf, "%lu", &val); \
> - if (ret != 1) \
> - return -EINVAL; \
> + ret = kstrtoul(buf, 0, &val); \
> + if (ret) \
> + return ret; \
> \
> ret = freq_qos_update_request(policy->object##_freq_req, val);\
> return ret >= 0 ? count : ret; \
> --
Applied as 6.4 material, thanks!
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2023-03-22 19:12 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-03-16 3:15 [PATCH] cpufreq: warn about invalid vals to scaling_max/min_freq interfaces qinyu
2023-03-16 3:36 ` Viresh Kumar
2023-03-17 11:47 ` Russell Haley
2023-03-20 7:36 ` qinyu
2023-03-17 15:41 ` Rafael J. Wysocki
2023-03-20 8:17 ` [PATCH v2] " qinyu
2023-03-22 19:12 ` Rafael J. Wysocki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).