[PATCH] firmware_loader: Replace simple_strtol() with kstrtoint()

[PATCH] firmware_loader: Replace simple_strtol() with kstrtoint()

[Haoyang Liu]

simple_strtol() is obsolete, use kstrtoint() instead.

Signed-off-by: Haoyang Liu <tttturtleruss@hust.edu.cn>
Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>
---
 drivers/base/firmware_loader/sysfs.c | 12 ++++++++++--
 1 file changed, 10 insertions(+), 2 deletions(-)

diff --git a/drivers/base/firmware_loader/sysfs.c b/drivers/base/firmware_loader/sysfs.c
index c9c93b47d9a5..4de1cb243bee 100644
--- a/drivers/base/firmware_loader/sysfs.c
+++ b/drivers/base/firmware_loader/sysfs.c
@@ -47,8 +47,12 @@ static ssize_t timeout_show(const struct class *class, const struct class_attrib
 static ssize_t timeout_store(const struct class *class, const struct class_attribute *attr,
 			     const char *buf, size_t count)
 {
-	int tmp_loading_timeout = simple_strtol(buf, NULL, 10);
+	int tmp_loading_timeout;
+	int res;
 
+	res = kstrtoint(buf, 10, &tmp_loading_timeout);
+	if (res < 0)
+		return res;
 	if (tmp_loading_timeout < 0)
 		tmp_loading_timeout = 0;
 
@@ -157,8 +161,12 @@ static ssize_t firmware_loading_store(struct device *dev,
 	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
 	struct fw_priv *fw_priv;
 	ssize_t written = count;
-	int loading = simple_strtol(buf, NULL, 10);
+	int loading;
+	int res;
 
+	res = kstrtoint(buf, 10, &loading);
+	if (res < 0)
+		return res;
 	mutex_lock(&fw_lock);
 	fw_priv = fw_sysfs->fw_priv;
 	if (fw_state_is_aborted(fw_priv) || fw_state_is_done(fw_priv))
-- 
2.25.1

Re: [PATCH] firmware_loader: Replace simple_strtol() with kstrtoint()

[Russ Weight]

On Sat, Apr 27, 2024 at 04:25:32AM +0800, Haoyang Liu wrote:
> simple_strtol() is obsolete, use kstrtoint() instead.
> 
> Signed-off-by: Haoyang Liu <tttturtleruss@hust.edu.cn>
> Reviewed-by: Dan Carpenter <dan.carpenter@linaro.org>

Reviewed-by: Russ Weight <russ.weight@linux.dev>

> ---
>  drivers/base/firmware_loader/sysfs.c | 12 ++++++++++--
>  1 file changed, 10 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/base/firmware_loader/sysfs.c b/drivers/base/firmware_loader/sysfs.c
> index c9c93b47d9a5..4de1cb243bee 100644
> --- a/drivers/base/firmware_loader/sysfs.c
> +++ b/drivers/base/firmware_loader/sysfs.c
> @@ -47,8 +47,12 @@ static ssize_t timeout_show(const struct class *class, const struct class_attrib
>  static ssize_t timeout_store(const struct class *class, const struct class_attribute *attr,
>  			     const char *buf, size_t count)
>  {
> -	int tmp_loading_timeout = simple_strtol(buf, NULL, 10);
> +	int tmp_loading_timeout;
> +	int res;
>  
> +	res = kstrtoint(buf, 10, &tmp_loading_timeout);
> +	if (res < 0)
> +		return res;
>  	if (tmp_loading_timeout < 0)
>  		tmp_loading_timeout = 0;
>  
> @@ -157,8 +161,12 @@ static ssize_t firmware_loading_store(struct device *dev,
>  	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
>  	struct fw_priv *fw_priv;
>  	ssize_t written = count;
> -	int loading = simple_strtol(buf, NULL, 10);
> +	int loading;
> +	int res;
>  
> +	res = kstrtoint(buf, 10, &loading);
> +	if (res < 0)
> +		return res;
>  	mutex_lock(&fw_lock);
>  	fw_priv = fw_sysfs->fw_priv;
>  	if (fw_state_is_aborted(fw_priv) || fw_state_is_done(fw_priv))
> -- 
> 2.25.1
> 

Re: [PATCH] firmware_loader: Replace simple_strtol() with kstrtoint()

[Andy Shevchenko]

Sat, Apr 27, 2024 at 04:25:32AM +0800, Haoyang Liu kirjoitti:
> simple_strtol() is obsolete, use kstrtoint() instead.

It's not, but kstrtox() is preferred.

...


While I'm in support of this move, this might break userspace by making
stricter requirement on the input.

So this is just informative message.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH] firmware_loader: Replace simple_strtol() with kstrtoint()

[Greg Kroah-Hartman]

On Sat, May 04, 2024 at 09:22:08PM +0300, Andy Shevchenko wrote:
> Sat, Apr 27, 2024 at 04:25:32AM +0800, Haoyang Liu kirjoitti:
> > simple_strtol() is obsolete, use kstrtoint() instead.
> 
> It's not, but kstrtox() is preferred.
> 
> ...
> 
> 
> While I'm in support of this move, this might break userspace by making
> stricter requirement on the input.

Good point, I've dropped this so that others can test it better first.

thanks,

greg k-h

[PATCH] firmware_loader: Replace simple_strtol() with kstrtoint()

[Kaushlendra Kumar]

Replace deprecated simple_strtol() calls with kstrtoint() in
timeout_store() and firmware_loading_store() functions to
improve input validation and error handling. The simple_strtol()
function does not provide proper error checking for invalid input,
while kstrtoint() returns an error for malformed strings.

This change adds proper validation for user input from sysfs attributes,
returning -EINVAL for invalid numeric strings instead of silently accepting
potentially malformed input. The behavior for valid numeric input remains
unchanged.

The simple_strtol() function is deprecated in favor of kstrtoint() family
functions which provide better error handling and are recommended for new
code and replacements.

Signed-off-by: Kaushlendra Kumar <kaushlendra.kumar@intel.com>
---
 drivers/base/firmware_loader/sysfs.c | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/base/firmware_loader/sysfs.c b/drivers/base/firmware_loader/sysfs.c
index add0b9b75edd..92e91050f96a 100644
--- a/drivers/base/firmware_loader/sysfs.c
+++ b/drivers/base/firmware_loader/sysfs.c
@@ -47,7 +47,10 @@ static ssize_t timeout_show(const struct class *class, const struct class_attrib
 static ssize_t timeout_store(const struct class *class, const struct class_attribute *attr,
 			     const char *buf, size_t count)
 {
-	int tmp_loading_timeout = simple_strtol(buf, NULL, 10);
+	int tmp_loading_timeout;
+
+	if (kstrtoint(buf, 10, &tmp_loading_timeout))
+		return -EINVAL;
 
 	if (tmp_loading_timeout < 0)
 		tmp_loading_timeout = 0;
@@ -157,7 +160,10 @@ static ssize_t firmware_loading_store(struct device *dev,
 	struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev);
 	struct fw_priv *fw_priv;
 	ssize_t written = count;
-	int loading = simple_strtol(buf, NULL, 10);
+	int loading;
+
+	if (kstrtoint(buf, 10, &loading))
+		return -EINVAL;
 
 	mutex_lock(&fw_lock);
 	fw_priv = fw_sysfs->fw_priv;
-- 
2.34.1