[PATCH] cpufreq: conservative: Replace sscanf() with kstrtouint()

[PATCH] cpufreq: conservative: Replace sscanf() with kstrtouint()

[Kaushlendra Kumar]

Replace sscanf() with kstrtouint() in all sysfs store functions to improve
input validation and security. The kstrtouint() function provides better
error detection, overflow protection, and consistent error handling
compared to sscanf().

The kstrtouint() function provides:
- Robust error detection for invalid input strings
- Built-in overflow protection and boundary checking
- Consistent error reporting (0 for success, negative for failure)

This maintains existing functionality while improving input validation
robustness and following kernel coding best practices for string parsing.

Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
---
 drivers/cpufreq/cpufreq_conservative.c | 24 ++++++++++++------------
 1 file changed, 12 insertions(+), 12 deletions(-)

diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
index 56500b25d77c..cce6a8d113e1 100644
--- a/drivers/cpufreq/cpufreq_conservative.c
+++ b/drivers/cpufreq/cpufreq_conservative.c
@@ -152,9 +152,9 @@ static ssize_t sampling_down_factor_store(struct gov_attr_set *attr_set,
 	struct dbs_data *dbs_data = to_dbs_data(attr_set);
 	unsigned int input;
 	int ret;
-	ret = sscanf(buf, "%u", &input);
+	ret = kstrtouint(buf, 0, &input);
 
-	if (ret != 1 || input > MAX_SAMPLING_DOWN_FACTOR || input < 1)
+	if (ret || input > MAX_SAMPLING_DOWN_FACTOR || input < 1)
 		return -EINVAL;
 
 	dbs_data->sampling_down_factor = input;
@@ -168,9 +168,9 @@ static ssize_t up_threshold_store(struct gov_attr_set *attr_set,
 	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
 	unsigned int input;
 	int ret;
-	ret = sscanf(buf, "%u", &input);
+	ret = kstrtouint(buf, 0, &input);
 
-	if (ret != 1 || input > 100 || input <= cs_tuners->down_threshold)
+	if (ret || input > 100 || input <= cs_tuners->down_threshold)
 		return -EINVAL;
 
 	dbs_data->up_threshold = input;
@@ -184,10 +184,10 @@ static ssize_t down_threshold_store(struct gov_attr_set *attr_set,
 	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
 	unsigned int input;
 	int ret;
-	ret = sscanf(buf, "%u", &input);
+	ret = kstrtouint(buf, 0, &input);
 
 	/* cannot be lower than 1 otherwise freq will not fall */
-	if (ret != 1 || input < 1 || input >= dbs_data->up_threshold)
+	if (ret || input < 1 || input >= dbs_data->up_threshold)
 		return -EINVAL;
 
 	cs_tuners->down_threshold = input;
@@ -201,9 +201,9 @@ static ssize_t ignore_nice_load_store(struct gov_attr_set *attr_set,
 	unsigned int input;
 	int ret;
 
-	ret = sscanf(buf, "%u", &input);
-	if (ret != 1)
-		return -EINVAL;
+	ret = kstrtouint(buf, 0, &input);
+	if (ret)
+		return ret;
 
 	if (input > 1)
 		input = 1;
@@ -226,10 +226,10 @@ static ssize_t freq_step_store(struct gov_attr_set *attr_set, const char *buf,
 	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
 	unsigned int input;
 	int ret;
-	ret = sscanf(buf, "%u", &input);
+	ret = kstrtouint(buf, 0, &input);
 
-	if (ret != 1)
-		return -EINVAL;
+	if (ret)
+		return ret;
 
 	if (input > 100)
 		input = 100;
-- 
2.34.1

Re: [PATCH] cpufreq: conservative: Replace sscanf() with kstrtouint()

[Viresh Kumar]

On 06-09-25, 17:23, Kaushlendra Kumar wrote:
> Replace sscanf() with kstrtouint() in all sysfs store functions to improve
> input validation and security. The kstrtouint() function provides better
> error detection, overflow protection, and consistent error handling
> compared to sscanf().
> 
> The kstrtouint() function provides:
> - Robust error detection for invalid input strings
> - Built-in overflow protection and boundary checking
> - Consistent error reporting (0 for success, negative for failure)
> 
> This maintains existing functionality while improving input validation
> robustness and following kernel coding best practices for string parsing.
> 
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---
>  drivers/cpufreq/cpufreq_conservative.c | 24 ++++++++++++------------
>  1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index 56500b25d77c..cce6a8d113e1 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -152,9 +152,9 @@ static ssize_t sampling_down_factor_store(struct gov_attr_set *attr_set,
>  	struct dbs_data *dbs_data = to_dbs_data(attr_set);
>  	unsigned int input;
>  	int ret;
> -	ret = sscanf(buf, "%u", &input);
> +	ret = kstrtouint(buf, 0, &input);
>  
> -	if (ret != 1 || input > MAX_SAMPLING_DOWN_FACTOR || input < 1)
> +	if (ret || input > MAX_SAMPLING_DOWN_FACTOR || input < 1)
>  		return -EINVAL;
>  
>  	dbs_data->sampling_down_factor = input;
> @@ -168,9 +168,9 @@ static ssize_t up_threshold_store(struct gov_attr_set *attr_set,
>  	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
>  	unsigned int input;
>  	int ret;
> -	ret = sscanf(buf, "%u", &input);
> +	ret = kstrtouint(buf, 0, &input);
>  
> -	if (ret != 1 || input > 100 || input <= cs_tuners->down_threshold)
> +	if (ret || input > 100 || input <= cs_tuners->down_threshold)
>  		return -EINVAL;
>  
>  	dbs_data->up_threshold = input;
> @@ -184,10 +184,10 @@ static ssize_t down_threshold_store(struct gov_attr_set *attr_set,
>  	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
>  	unsigned int input;
>  	int ret;
> -	ret = sscanf(buf, "%u", &input);
> +	ret = kstrtouint(buf, 0, &input);
>  
>  	/* cannot be lower than 1 otherwise freq will not fall */
> -	if (ret != 1 || input < 1 || input >= dbs_data->up_threshold)
> +	if (ret || input < 1 || input >= dbs_data->up_threshold)
>  		return -EINVAL;
>  
>  	cs_tuners->down_threshold = input;
> @@ -201,9 +201,9 @@ static ssize_t ignore_nice_load_store(struct gov_attr_set *attr_set,
>  	unsigned int input;
>  	int ret;
>  
> -	ret = sscanf(buf, "%u", &input);
> -	if (ret != 1)
> -		return -EINVAL;
> +	ret = kstrtouint(buf, 0, &input);
> +	if (ret)
> +		return ret;
>  
>  	if (input > 1)
>  		input = 1;
> @@ -226,10 +226,10 @@ static ssize_t freq_step_store(struct gov_attr_set *attr_set, const char *buf,
>  	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
>  	unsigned int input;
>  	int ret;
> -	ret = sscanf(buf, "%u", &input);
> +	ret = kstrtouint(buf, 0, &input);
>  
> -	if (ret != 1)
> -		return -EINVAL;
> +	if (ret)
> +		return ret;
>  
>  	if (input > 100)
>  		input = 100;

Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

-- 
viresh

Re: [PATCH] cpufreq: conservative: Replace sscanf() with kstrtouint()

[Oliver Neukum]

Hi,

On 9/6/25 13:53, Kaushlendra Kumar wrote:
> Replace sscanf() with kstrtouint() in all sysfs store functions to improve
> input validation and security. The kstrtouint() function provides better
> error detection, overflow protection, and consistent error handling
> compared to sscanf().
> 
> The kstrtouint() function provides:
> - Robust error detection for invalid input strings
> - Built-in overflow protection and boundary checking
> - Consistent error reporting (0 for success, negative for failure)
> 
> This maintains existing functionality while improving input validation
> robustness and following kernel coding best practices for string parsing.

looking at the patch while it is a good thing, something struck me ...
> Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> ---
>   drivers/cpufreq/cpufreq_conservative.c | 24 ++++++++++++------------
>   1 file changed, 12 insertions(+), 12 deletions(-)
> 
> diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> index 56500b25d77c..cce6a8d113e1 100644
> --- a/drivers/cpufreq/cpufreq_conservative.c
> +++ b/drivers/cpufreq/cpufreq_conservative.c
> @@ -152,9 +152,9 @@ static ssize_t sampling_down_factor_store(struct gov_attr_set *attr_set,
>   	struct dbs_data *dbs_data = to_dbs_data(attr_set);
>   	unsigned int input;
>   	int ret;
> -	ret = sscanf(buf, "%u", &input);
> +	ret = kstrtouint(buf, 0, &input);
>   
> -	if (ret != 1 || input > MAX_SAMPLING_DOWN_FACTOR || input < 1)
> +	if (ret || input > MAX_SAMPLING_DOWN_FACTOR || input < 1)
>   		return -EINVAL;

... the parsing itself, followed by a check for bounds ...   
>   	dbs_data->sampling_down_factor = input;
> @@ -168,9 +168,9 @@ static ssize_t up_threshold_store(struct gov_attr_set *attr_set,
>   	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
>   	unsigned int input;
>   	int ret;
> -	ret = sscanf(buf, "%u", &input);
> +	ret = kstrtouint(buf, 0, &input);
>   
> -	if (ret != 1 || input > 100 || input <= cs_tuners->down_threshold)
> +	if (ret || input > 100 || input <= cs_tuners->down_threshold)
>   		return -EINVAL;

... and here again. It seems to me that if you always follow the parsing
with a check for bounds, then to reduce code duplication you really want
a function that takes an upper and a lower bound as parameters and checks
against them.
In that sense, I am afraid I have to say that your patch stops in the
middle of the journey.

	Regards
		Oliver

RE: [PATCH] cpufreq: conservative: Replace sscanf() with kstrtouint()

[Kumar, Kaushlendra]

> Hi,
>
> On 9/6/25 13:53, Kaushlendra Kumar wrote:
> > Replace sscanf() with kstrtouint() in all sysfs store functions to 
> > improve input validation and security. The kstrtouint() function 
> > provides better error detection, overflow protection, and consistent 
> > error handling compared to sscanf().
> > 
> > The kstrtouint() function provides:
> > - Robust error detection for invalid input strings
> > - Built-in overflow protection and boundary checking
> > - Consistent error reporting (0 for success, negative for failure)
> > 
> > This maintains existing functionality while improving input validation 
> > robustness and following kernel coding best practices for string parsing.
> 
> looking at the patch while it is a good thing, something struck me ...
> > Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> > ---
> >   drivers/cpufreq/cpufreq_conservative.c | 24 ++++++++++++------------
> >   1 file changed, 12 insertions(+), 12 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/cpufreq_conservative.c 
> > b/drivers/cpufreq/cpufreq_conservative.c
> > index 56500b25d77c..cce6a8d113e1 100644
> > --- a/drivers/cpufreq/cpufreq_conservative.c
> > +++ b/drivers/cpufreq/cpufreq_conservative.c
> > @@ -152,9 +152,9 @@ static ssize_t sampling_down_factor_store(struct gov_attr_set *attr_set,
> >   	struct dbs_data *dbs_data = to_dbs_data(attr_set);
> >   	unsigned int input;
> >   	int ret;
> > -	ret = sscanf(buf, "%u", &input);
> > +	ret = kstrtouint(buf, 0, &input);
> >   
> > -	if (ret != 1 || input > MAX_SAMPLING_DOWN_FACTOR || input < 1)
> > +	if (ret || input > MAX_SAMPLING_DOWN_FACTOR || input < 1)
> >   		return -EINVAL;
> 
> ... the parsing itself, followed by a check for bounds ...   
> >   	dbs_data->sampling_down_factor = input; @@ -168,9 +168,9 @@ static 
> > ssize_t up_threshold_store(struct gov_attr_set *attr_set,
> >   	struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
> >   	unsigned int input;
> >   	int ret;
> > -	ret = sscanf(buf, "%u", &input);
> > +	ret = kstrtouint(buf, 0, &input);
> >   
> > -	if (ret != 1 || input > 100 || input <= cs_tuners->down_threshold)
> > +	if (ret || input > 100 || input <= cs_tuners->down_threshold)
> >   		return -EINVAL;
> 
> ... and here again. It seems to me that if you always follow the parsing with a check for bounds, then to reduce code duplication you really want a function that takes an upper and a lower bound as parameters and checks against them.
> In that sense, I am afraid I have to say that your patch stops in the middle of the journey.

Thank you for the feedback! You made a valid point.

This current patch focuses specifically on replacing sscanf() with kstrtouint() 
for the security and robustness benefits, but I agree that the bounds checking 
pattern deserves its own optimization.

I'll prepare a separate follow-up patch that introduces a helper function to 
eliminate the code duplication you've identified. This would make the code 
much cleaner and more maintainable.

Can I prepare a separate patch 
where this sscanf replacement goes first, followed by the bounds 
checking consolidation?

Best regards,
Kaushlendra Kumar

Re: [PATCH] cpufreq: conservative: Replace sscanf() with kstrtouint()

[Rafael J. Wysocki]

On Mon, Sep 8, 2025 at 9:02 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 06-09-25, 17:23, Kaushlendra Kumar wrote:
> > Replace sscanf() with kstrtouint() in all sysfs store functions to improve
> > input validation and security. The kstrtouint() function provides better
> > error detection, overflow protection, and consistent error handling
> > compared to sscanf().
> >
> > The kstrtouint() function provides:
> > - Robust error detection for invalid input strings
> > - Built-in overflow protection and boundary checking
> > - Consistent error reporting (0 for success, negative for failure)
> >
> > This maintains existing functionality while improving input validation
> > robustness and following kernel coding best practices for string parsing.
> >
> > Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
> > ---
> >  drivers/cpufreq/cpufreq_conservative.c | 24 ++++++++++++------------
> >  1 file changed, 12 insertions(+), 12 deletions(-)
> >
> > diff --git a/drivers/cpufreq/cpufreq_conservative.c b/drivers/cpufreq/cpufreq_conservative.c
> > index 56500b25d77c..cce6a8d113e1 100644
> > --- a/drivers/cpufreq/cpufreq_conservative.c
> > +++ b/drivers/cpufreq/cpufreq_conservative.c
> > @@ -152,9 +152,9 @@ static ssize_t sampling_down_factor_store(struct gov_attr_set *attr_set,
> >       struct dbs_data *dbs_data = to_dbs_data(attr_set);
> >       unsigned int input;
> >       int ret;
> > -     ret = sscanf(buf, "%u", &input);
> > +     ret = kstrtouint(buf, 0, &input);
> >
> > -     if (ret != 1 || input > MAX_SAMPLING_DOWN_FACTOR || input < 1)
> > +     if (ret || input > MAX_SAMPLING_DOWN_FACTOR || input < 1)
> >               return -EINVAL;
> >
> >       dbs_data->sampling_down_factor = input;
> > @@ -168,9 +168,9 @@ static ssize_t up_threshold_store(struct gov_attr_set *attr_set,
> >       struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
> >       unsigned int input;
> >       int ret;
> > -     ret = sscanf(buf, "%u", &input);
> > +     ret = kstrtouint(buf, 0, &input);
> >
> > -     if (ret != 1 || input > 100 || input <= cs_tuners->down_threshold)
> > +     if (ret || input > 100 || input <= cs_tuners->down_threshold)
> >               return -EINVAL;
> >
> >       dbs_data->up_threshold = input;
> > @@ -184,10 +184,10 @@ static ssize_t down_threshold_store(struct gov_attr_set *attr_set,
> >       struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
> >       unsigned int input;
> >       int ret;
> > -     ret = sscanf(buf, "%u", &input);
> > +     ret = kstrtouint(buf, 0, &input);
> >
> >       /* cannot be lower than 1 otherwise freq will not fall */
> > -     if (ret != 1 || input < 1 || input >= dbs_data->up_threshold)
> > +     if (ret || input < 1 || input >= dbs_data->up_threshold)
> >               return -EINVAL;
> >
> >       cs_tuners->down_threshold = input;
> > @@ -201,9 +201,9 @@ static ssize_t ignore_nice_load_store(struct gov_attr_set *attr_set,
> >       unsigned int input;
> >       int ret;
> >
> > -     ret = sscanf(buf, "%u", &input);
> > -     if (ret != 1)
> > -             return -EINVAL;
> > +     ret = kstrtouint(buf, 0, &input);
> > +     if (ret)
> > +             return ret;
> >
> >       if (input > 1)
> >               input = 1;
> > @@ -226,10 +226,10 @@ static ssize_t freq_step_store(struct gov_attr_set *attr_set, const char *buf,
> >       struct cs_dbs_tuners *cs_tuners = dbs_data->tuners;
> >       unsigned int input;
> >       int ret;
> > -     ret = sscanf(buf, "%u", &input);
> > +     ret = kstrtouint(buf, 0, &input);
> >
> > -     if (ret != 1)
> > -             return -EINVAL;
> > +     if (ret)
> > +             return ret;
> >
> >       if (input > 100)
> >               input = 100;
>
> Acked-by: Viresh Kumar <viresh.kumar@linaro.org>

Applied as 6.18 material, thanks!