[PATCH 1/2] ubi: build: replace simple_strtoul with kstrtouint in open_mtd_device()

[PATCH 1/2] ubi: build: replace simple_strtoul with kstrtouint in open_mtd_device()

[haoyu.lu]

From: Haoyu Lu <hechushiguitu666@gmail.com>

Replace the deprecated simple_strtoul() with kstrtouint() for parsing
the MTD device number string. kstrtouint() provides stricter validation
and better error handling.

In open_mtd_device(), a string that can be parsed as a pure number is
treated as an MTD device index; otherwise it is treated as a device
name. kstrtouint() returns 0 on success and -EINVAL if the string
contains non-numeric characters, which aligns with this logic and
eliminates the need for manual endp checks.

For compatibility with kstrtouint(), mtd_num is changed from int to
unsigned int, consistent with the existing simple_strtoul ->
kstrtouint conversions in mtdoops.c and mtdsuper.c.

The multi-line comment is simplified to a concise single-line comment
since the logic is self-explanatory with kstrtouint().

Signed-off-by: Haoyu Lu <hechushiguitu666@gmail.com>
---
 drivers/mtd/ubi/build.c | 15 +++++----------
 1 file changed, 5 insertions(+), 10 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 674ad87809df..03ef195dc25c 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -1210,21 +1210,16 @@ static struct mtd_info * __init open_mtd_by_chdev(const char *mtd_dev)
 static struct mtd_info * __init open_mtd_device(const char *mtd_dev)
 {
 	struct mtd_info *mtd;
-	int mtd_num;
-	char *endp;
+	unsigned int mtd_num;
 
-	mtd_num = simple_strtoul(mtd_dev, &endp, 0);
-	if (*endp != '\0' || mtd_dev == endp) {
-		/*
-		 * This does not look like an ASCII integer, probably this is
-		 * MTD device name.
-		 */
+	if (kstrtouint(mtd_dev, 0, &mtd_num) != 0) {
+		/* Not a plain number, treat as MTD device name */
 		mtd = get_mtd_device_nm(mtd_dev);
 		if (PTR_ERR(mtd) == -ENODEV)
-			/* Probably this is an MTD character device node path */
 			mtd = open_mtd_by_chdev(mtd_dev);
-	} else
+	} else {
 		mtd = get_mtd_device(NULL, mtd_num);
+	}
 
 	return mtd;
 }
-- 
2.17.1

[PATCH 2/2] ubi: build: replace simple_strtoul with kstrtoul in bytes_str_to_int()

[haoyu.lu]

From: Haoyu Lu <hechushiguitu666@gmail.com>

Replace the deprecated simple_strtoul() with kstrtoul() in the
bytes_str_to_int() helper function. Since kstrtoul() rejects trailing
non-numeric characters (such as the G/M/K suffixes), the numeric prefix
is first extracted with strspn() and then parsed separately before
handling the suffix.

This provides proper error handling through the kstrto* family while
preserving the existing suffix semantics for byte count parameters.

Note: the original simple_strtoul() with base=0 accepted hexadecimal
(0x prefix) and octal (0 prefix) formats, while kstrtoul() with base=10
only supports decimal. This is not a practical concern since MTD byte
count parameters are always specified as decimal values in boot
parameters.

Signed-off-by: Haoyu Lu <hechushiguitu666@gmail.com>
---
 drivers/mtd/ubi/build.c | 19 +++++++++++++++----
 1 file changed, 15 insertions(+), 4 deletions(-)

diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
index 03ef195dc25c..7cb6ba4a3840 100644
--- a/drivers/mtd/ubi/build.c
+++ b/drivers/mtd/ubi/build.c
@@ -1426,16 +1426,27 @@ module_exit(ubi_exit);
  */
 static int bytes_str_to_int(const char *str)
 {
-	char *endp;
 	unsigned long result;
+	unsigned int num_len;
+	char num_buf[32];
 
-	result = simple_strtoul(str, &endp, 0);
-	if (str == endp || result >= INT_MAX) {
+	/* Find the length of the numeric prefix */
+	num_len = strspn(str, "0123456789");
+	if (num_len == 0 || num_len >= sizeof(num_buf)) {
 		pr_err("UBI error: incorrect bytes count: \"%s\"\n", str);
 		return -EINVAL;
 	}
 
-	switch (*endp) {
+	/* Parse the numeric part */
+	memcpy(num_buf, str, num_len);
+	num_buf[num_len] = '\0';
+	if (kstrtoul(num_buf, 10, &result) < 0 || result >= INT_MAX) {
+		pr_err("UBI error: incorrect bytes count: \"%s\"\n", str);
+		return -EINVAL;
+	}
+
+	/* Handle suffix */
+	switch (str[num_len]) {
 	case 'G':
 		result *= 1024;
 		fallthrough;
-- 
2.17.1

Re: [PATCH 2/2] ubi: build: replace simple_strtoul with kstrtoul in bytes_str_to_int()

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "haoyu.lu" <hechushiguitu666@gmail.com>
> An: "Miquel Raynal" <miquel.raynal@bootlin.com>, "richard" <richard@nod.at>, "Vignesh Raghavendra" <vigneshr@ti.com>
> CC: "chengzhihao1" <chengzhihao1@huawei.com>, "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel"
> <linux-kernel@vger.kernel.org>, "Haoyu Lu" <hechushiguitu666@gmail.com>
> Gesendet: Donnerstag, 7. Mai 2026 05:23:08
> Betreff: [PATCH 2/2] ubi: build: replace simple_strtoul with kstrtoul in bytes_str_to_int()

> From: Haoyu Lu <hechushiguitu666@gmail.com>
> 
> Replace the deprecated simple_strtoul() with kstrtoul() in the
>
> bytes_str_to_int() helper function. Since kstrtoul() rejects trailing
> non-numeric characters (such as the G/M/K suffixes), the numeric prefix
> is first extracted with strspn() and then parsed separately before
> handling the suffix.
> 
> This provides proper error handling through the kstrto* family while
> preserving the existing suffix semantics for byte count parameters.
> 
> Note: the original simple_strtoul() with base=0 accepted hexadecimal
> (0x prefix) and octal (0 prefix) formats, while kstrtoul() with base=10
> only supports decimal. This is not a practical concern since MTD byte
> count parameters are always specified as decimal values in boot
> parameters.
> 
> Signed-off-by: Haoyu Lu <hechushiguitu666@gmail.com>
> ---
> drivers/mtd/ubi/build.c | 19 +++++++++++++++----
> 1 file changed, 15 insertions(+), 4 deletions(-)


Your patch does not explain why this change is needed.
From what I can tell, it just makes the existing code more complex and
fixes no real issues.

Is simple_strtoul() really deprecated? If so, where?
AFAICT, the documentation just mentions:
"This function has caveats. Please use kstrtol instead."

Do these these caveats hurt the current code?

Thanks,
//richard

Re: [PATCH 2/2] ubi: build: replace simple_strtoul with kstrtoul in bytes_str_to_int()

[Zhihao Cheng]

在 2026/5/7 11:23, haoyu.lu 写道:
> From: Haoyu Lu <hechushiguitu666@gmail.com>
> 
> Replace the deprecated simple_strtoul() with kstrtoul() in the
> bytes_str_to_int() helper function. Since kstrtoul() rejects trailing
> non-numeric characters (such as the G/M/K suffixes), the numeric prefix
> is first extracted with strspn() and then parsed separately before
> handling the suffix.
> 
> This provides proper error handling through the kstrto* family while
> preserving the existing suffix semantics for byte count parameters.
> 
> Note: the original simple_strtoul() with base=0 accepted hexadecimal
> (0x prefix) and octal (0 prefix) formats, while kstrtoul() with base=10
> only supports decimal. This is not a practical concern since MTD byte
> count parameters are always specified as decimal values in boot
> parameters.
The ubi could be loaded by module insertion, many vendors set param 
'vid_hdr_offs' to save space. We cannot make the assumption that all 
users use the decimal system.
> 
> Signed-off-by: Haoyu Lu <hechushiguitu666@gmail.com>
> ---
>   drivers/mtd/ubi/build.c | 19 +++++++++++++++----
>   1 file changed, 15 insertions(+), 4 deletions(-)
> 
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 03ef195dc25c..7cb6ba4a3840 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -1426,16 +1426,27 @@ module_exit(ubi_exit);
>    */
>   static int bytes_str_to_int(const char *str)
>   {
> -	char *endp;
>   	unsigned long result;
> +	unsigned int num_len;
> +	char num_buf[32];
>   
> -	result = simple_strtoul(str, &endp, 0);
> -	if (str == endp || result >= INT_MAX) {
> +	/* Find the length of the numeric prefix */
> +	num_len = strspn(str, "0123456789");
> +	if (num_len == 0 || num_len >= sizeof(num_buf)) {
>   		pr_err("UBI error: incorrect bytes count: \"%s\"\n", str);
>   		return -EINVAL;
>   	}
>   
> -	switch (*endp) {
> +	/* Parse the numeric part */
> +	memcpy(num_buf, str, num_len);
> +	num_buf[num_len] = '\0';
> +	if (kstrtoul(num_buf, 10, &result) < 0 || result >= INT_MAX) {
> +		pr_err("UBI error: incorrect bytes count: \"%s\"\n", str);
> +		return -EINVAL;
> +	}
> +
> +	/* Handle suffix */
> +	switch (str[num_len]) {
>   	case 'G':
>   		result *= 1024;
>   		fallthrough;
> 

Re: [PATCH 2/2] ubi: build: replace simple_strtoul with kstrtoul in bytes_str_to_int()

[Richard Weinberger]

----- Ursprüngliche Mail -----
> Von: "chengzhihao1" <chengzhihao1@huawei.com>
> An: "haoyu.lu" <hechushiguitu666@gmail.com>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "richard" <richard@nod.at>,
> "Vignesh Raghavendra" <vigneshr@ti.com>
> CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>
> Gesendet: Donnerstag, 7. Mai 2026 09:45:24
> Betreff: Re: [PATCH 2/2] ubi: build: replace simple_strtoul with kstrtoul in bytes_str_to_int()

> 在 2026/5/7 11:23, haoyu.lu 写道:
>> From: Haoyu Lu <hechushiguitu666@gmail.com>
>> 
>> Replace the deprecated simple_strtoul() with kstrtoul() in the
>> bytes_str_to_int() helper function. Since kstrtoul() rejects trailing
>> non-numeric characters (such as the G/M/K suffixes), the numeric prefix
>> is first extracted with strspn() and then parsed separately before
>> handling the suffix.
>> 
>> This provides proper error handling through the kstrto* family while
>> preserving the existing suffix semantics for byte count parameters.
>> 
>> Note: the original simple_strtoul() with base=0 accepted hexadecimal
>> (0x prefix) and octal (0 prefix) formats, while kstrtoul() with base=10
>> only supports decimal. This is not a practical concern since MTD byte
>> count parameters are always specified as decimal values in boot
>> parameters.
> The ubi could be loaded by module insertion, many vendors set param
> 'vid_hdr_offs' to save space. We cannot make the assumption that all
> users use the decimal system.

That's a good point!

In general I'm not fond of such changes. They change code for the sake
of changing code without fixing actual issues.
I'll happily accept patches that point out real issues with the current
usage of various string functions, though.

Thanks,
//richard

Re: [PATCH 2/2] ubi: build: replace simple_strtoul with kstrtoul in bytes_str_to_int()

[Haoyu Lu]

Hi Richard, Zhihao,

Thanks for the feedback. I will drop this patch.

Thanks,
Haoyu

On Thu, May 7, 2026 at 3:50 PM Richard Weinberger <richard@nod.at> wrote:
>
> ----- Ursprüngliche Mail -----
> > Von: "chengzhihao1" <chengzhihao1@huawei.com>
> > An: "haoyu.lu" <hechushiguitu666@gmail.com>, "Miquel Raynal" <miquel.raynal@bootlin.com>, "richard" <richard@nod.at>,
> > "Vignesh Raghavendra" <vigneshr@ti.com>
> > CC: "linux-mtd" <linux-mtd@lists.infradead.org>, "linux-kernel" <linux-kernel@vger.kernel.org>
> > Gesendet: Donnerstag, 7. Mai 2026 09:45:24
> > Betreff: Re: [PATCH 2/2] ubi: build: replace simple_strtoul with kstrtoul in bytes_str_to_int()
>
> > 在 2026/5/7 11:23, haoyu.lu 写道:
> >> From: Haoyu Lu <hechushiguitu666@gmail.com>
> >>
> >> Replace the deprecated simple_strtoul() with kstrtoul() in the
> >> bytes_str_to_int() helper function. Since kstrtoul() rejects trailing
> >> non-numeric characters (such as the G/M/K suffixes), the numeric prefix
> >> is first extracted with strspn() and then parsed separately before
> >> handling the suffix.
> >>
> >> This provides proper error handling through the kstrto* family while
> >> preserving the existing suffix semantics for byte count parameters.
> >>
> >> Note: the original simple_strtoul() with base=0 accepted hexadecimal
> >> (0x prefix) and octal (0 prefix) formats, while kstrtoul() with base=10
> >> only supports decimal. This is not a practical concern since MTD byte
> >> count parameters are always specified as decimal values in boot
> >> parameters.
> > The ubi could be loaded by module insertion, many vendors set param
> > 'vid_hdr_offs' to save space. We cannot make the assumption that all
> > users use the decimal system.
>
> That's a good point!
>
> In general I'm not fond of such changes. They change code for the sake
> of changing code without fixing actual issues.
> I'll happily accept patches that point out real issues with the current
> usage of various string functions, though.
>
> Thanks,
> //richard

Re: [PATCH 1/2] ubi: build: replace simple_strtoul with kstrtouint in open_mtd_device()

[Zhihao Cheng]

在 2026/5/7 11:23, haoyu.lu 写道:
> From: Haoyu Lu <hechushiguitu666@gmail.com>
> 
> Replace the deprecated simple_strtoul() with kstrtouint() for parsing
> the MTD device number string. kstrtouint() provides stricter validation
> and better error handling.
> 
> In open_mtd_device(), a string that can be parsed as a pure number is
> treated as an MTD device index; otherwise it is treated as a device
> name. kstrtouint() returns 0 on success and -EINVAL if the string
> contains non-numeric characters, which aligns with this logic and
> eliminates the need for manual endp checks.
> 
> For compatibility with kstrtouint(), mtd_num is changed from int to
> unsigned int, consistent with the existing simple_strtoul ->
> kstrtouint conversions in mtdoops.c and mtdsuper.c.
> 
> The multi-line comment is simplified to a concise single-line comment
> since the logic is self-explanatory with kstrtouint().
> 
> Signed-off-by: Haoyu Lu <hechushiguitu666@gmail.com>
> ---
>   drivers/mtd/ubi/build.c | 15 +++++----------
>   1 file changed, 5 insertions(+), 10 deletions(-)
> 

Reviewed-by: Zhihao Cheng <chengzhihao1@huawei.com>
> diff --git a/drivers/mtd/ubi/build.c b/drivers/mtd/ubi/build.c
> index 674ad87809df..03ef195dc25c 100644
> --- a/drivers/mtd/ubi/build.c
> +++ b/drivers/mtd/ubi/build.c
> @@ -1210,21 +1210,16 @@ static struct mtd_info * __init open_mtd_by_chdev(const char *mtd_dev)
>   static struct mtd_info * __init open_mtd_device(const char *mtd_dev)
>   {
>   	struct mtd_info *mtd;
> -	int mtd_num;
> -	char *endp;
> +	unsigned int mtd_num;
>   
> -	mtd_num = simple_strtoul(mtd_dev, &endp, 0);
> -	if (*endp != '\0' || mtd_dev == endp) {
> -		/*
> -		 * This does not look like an ASCII integer, probably this is
> -		 * MTD device name.
> -		 */
> +	if (kstrtouint(mtd_dev, 0, &mtd_num) != 0) {
> +		/* Not a plain number, treat as MTD device name */
>   		mtd = get_mtd_device_nm(mtd_dev);
>   		if (PTR_ERR(mtd) == -ENODEV)
> -			/* Probably this is an MTD character device node path */
>   			mtd = open_mtd_by_chdev(mtd_dev);
> -	} else
> +	} else {
>   		mtd = get_mtd_device(NULL, mtd_num);
> +	}
>   
>   	return mtd;
>   }
>