[PATCH v5 0/5] lib and lib/cmdline enhancements

[PATCH v5 0/5] lib and lib/cmdline enhancements

[Dmitry Antipov]

Adjust '_parse_integer_limit()' and 'memparse()' to not ignore
overflows, extend string to 64-bit integer conversion tests, add
KUnit-based test for 'memparse()' and fix kernel-doc glitches
found in lib/cmdline.c.

Dmitry Antipov (5):
  lib: fix _parse_integer_limit() to handle overflow
  lib: fix memparse() to handle overflow
  lib: add more string to 64-bit integer conversion overflow tests
  lib/cmdline_kunit: add test case for memparse()
  lib/cmdline: adjust a few comments to fix kernel-doc -Wreturn warnings

 lib/cmdline.c             | 29 +++++++++++++++------
 lib/kstrtox.c             | 38 ++++++++++++++++++----------
 lib/test-kstrtox.c        |  6 +++++
 lib/tests/cmdline_kunit.c | 53 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 105 insertions(+), 21 deletions(-)

-- 
2.52.0

[PATCH v5 1/5] lib: fix _parse_integer_limit() to handle overflow

[Dmitry Antipov]

In '_parse_integer_limit()', adjust native integer arithmetic
with near-to-overflow branch where 'check_mul_overflow()' and
'check_add_overflow()' are used to check whether an intermediate
result goes out of range, and denote such a case with ULLONG_MAX,
thus making the function more similar to standard C library's
'strtoull()'. Adjust comment to kernel-doc style as well.

Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: minor brace style adjustment
v4: restore plain integer arithmetic and use check_xxx_overflow()
    on near-to-overflow branch only
v3: adjust commit message and comments as suggested by Andy
v2: initial version to join the series
---
 lib/kstrtox.c | 39 ++++++++++++++++++++++++++-------------
 1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index bdde40cd69d7..8691f85cf2ce 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -39,20 +39,26 @@ const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
 	return s;
 }
 
-/*
- * Convert non-negative integer string representation in explicitly given radix
- * to an integer. A maximum of max_chars characters will be converted.
+/**
+ * _parse_integer_limit - Convert integer string representation to an integer
+ * @s: Integer string representation
+ * @base: Radix
+ * @p: Where to store result
+ * @max_chars: Maximum amount of characters to convert
+ *
+ * Convert non-negative integer string representation in explicitly given
+ * radix to an integer. If overflow occurs, value at @p is set to ULLONG_MAX.
  *
- * Return number of characters consumed maybe or-ed with overflow bit.
- * If overflow occurs, result integer (incorrect) is still returned.
+ * This function is the workhorse of other string conversion functions and it
+ * is discouraged to use it explicitly. Consider kstrto*() family instead.
  *
- * Don't you dare use this function.
+ * Return: Number of characters consumed, maybe ORed with overflow bit
  */
 noinline
 unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned long long *p,
 				  size_t max_chars)
 {
-	unsigned long long res;
+	unsigned long long tmp, res;
 	unsigned int rv;
 
 	res = 0;
@@ -72,14 +78,21 @@ unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned lon
 		if (val >= base)
 			break;
 		/*
-		 * Check for overflow only if we are within range of
-		 * it in the max base we support (16)
+		 * Accumulate result if no overflow detected.
+		 * Otherwise just consume valid characters.
 		 */
-		if (unlikely(res & (~0ull << 60))) {
-			if (res > div_u64(ULLONG_MAX - val, base))
-				rv |= KSTRTOX_OVERFLOW;
+		if (likely(res != ULLONG_MAX)) {
+			if (unlikely(res & (~0ull << 60))) {
+				/* We're close to possible overflow. */
+				if (check_mul_overflow(res, base, &tmp) ||
+				    check_add_overflow(tmp, val, &res)) {
+					res = ULLONG_MAX;
+					rv |= KSTRTOX_OVERFLOW;
+				}
+			} else {
+				res = res * base + val;
+			}
 		}
-		res = res * base + val;
 		rv++;
 		s++;
 	}
-- 
2.52.0

[PATCH v5 2/5] lib: fix memparse() to handle overflow

[Dmitry Antipov]

Since '_parse_integer_limit()' (and so 'simple_strtoull()') is now
capable to handle overflow, adjust 'memparse()' to handle overflow
(denoted by ULLONG_MAX) returned from 'simple_strtoull()'. Also
use 'check_shl_overflow()' to catch an overflow possibly caused
by processing size suffix and denote it with ULLONG_MAX as well.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: initial version to join the series
---
 lib/cmdline.c | 22 ++++++++++++++++------
 1 file changed, 16 insertions(+), 6 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 90ed997d9570..e18fdfb80ba5 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -151,38 +151,48 @@ unsigned long long memparse(const char *ptr, char **retptr)
 {
 	char *endptr;	/* local pointer to end of parsed string */
 
+	unsigned int shl = 0;
 	unsigned long long ret = simple_strtoull(ptr, &endptr, 0);
 
+	/* Consume valid suffix even in case of overflow. */
 	switch (*endptr) {
 	case 'E':
 	case 'e':
-		ret <<= 10;
+		shl += 10;
 		fallthrough;
 	case 'P':
 	case 'p':
-		ret <<= 10;
+		shl += 10;
 		fallthrough;
 	case 'T':
 	case 't':
-		ret <<= 10;
+		shl += 10;
 		fallthrough;
 	case 'G':
 	case 'g':
-		ret <<= 10;
+		shl += 10;
 		fallthrough;
 	case 'M':
 	case 'm':
-		ret <<= 10;
+		shl += 10;
 		fallthrough;
 	case 'K':
 	case 'k':
-		ret <<= 10;
+		shl += 10;
 		endptr++;
 		fallthrough;
 	default:
 		break;
 	}
 
+	/* If no overflow, apply suffix if any. */
+	if (likely(ret != ULLONG_MAX) && shl) {
+		unsigned long long val;
+
+		ret = (unlikely(check_shl_overflow(ret, shl, &val))
+		       ? ULLONG_MAX : val);
+	}
+
 	if (retptr)
 		*retptr = endptr;
 
-- 
2.52.0

[PATCH v5 3/5] lib: add more string to 64-bit integer conversion overflow tests

[Dmitry Antipov]

Add a few more string to 64-bit integer conversion tests to
check whether 'kstrtoull()', 'kstrtoll()', 'kstrtou64()' and
'kstrtos64()' can handle overflows reported by
'_parse_integer_limit()'.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: bump version to match the series
v4: initial version to join the series
---
 lib/test-kstrtox.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/lib/test-kstrtox.c b/lib/test-kstrtox.c
index ee87fef66cb5..811128d0df16 100644
--- a/lib/test-kstrtox.c
+++ b/lib/test-kstrtox.c
@@ -198,6 +198,7 @@ static void __init test_kstrtoull_fail(void)
 		{"10000000000000000000000000000000000000000000000000000000000000000",	2},
 		{"2000000000000000000000",	8},
 		{"18446744073709551616",	10},
+		{"569202370375329612767",	10},
 		{"10000000000000000",	16},
 		/* negative */
 		{"-0", 0},
@@ -275,9 +276,11 @@ static void __init test_kstrtoll_fail(void)
 		{"9223372036854775809",	10},
 		{"18446744073709551614",	10},
 		{"18446744073709551615",	10},
+		{"569202370375329612767",	10},
 		{"-9223372036854775809",	10},
 		{"-18446744073709551614",	10},
 		{"-18446744073709551615",	10},
+		{"-569202370375329612767",	10},
 		/* sign is first character if any */
 		{"-+1", 0},
 		{"-+1", 8},
@@ -334,6 +337,7 @@ static void __init test_kstrtou64_fail(void)
 		{"-1",	10},
 		{"18446744073709551616",	10},
 		{"18446744073709551617",	10},
+		{"569202370375329612767",	10},
 	};
 	TEST_FAIL(kstrtou64, u64, "%llu", test_u64_fail);
 }
@@ -386,6 +390,8 @@ static void __init test_kstrtos64_fail(void)
 		{"18446744073709551615",	10},
 		{"18446744073709551616",	10},
 		{"18446744073709551617",	10},
+		{"569202370375329612767",	10},
+		{"-569202370375329612767",	10},
 	};
 	TEST_FAIL(kstrtos64, s64, "%lld", test_s64_fail);
 }
-- 
2.52.0

[PATCH v5 4/5] lib/cmdline_kunit: add test case for memparse()

[Dmitry Antipov]

Better late than never, now there is a long-awaited basic
test for 'memparse()' which is provided by cmdline.c.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: even more tests to trigger overflow with size suffix
v4: move actual overflow tests to test-kstrtox.c
v3: adjust style as suggested by Andy
v2: few more test cases to trigger overflows
---
 lib/tests/cmdline_kunit.c | 53 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 53 insertions(+)

diff --git a/lib/tests/cmdline_kunit.c b/lib/tests/cmdline_kunit.c
index c1602f797637..41bf4fecd97e 100644
--- a/lib/tests/cmdline_kunit.c
+++ b/lib/tests/cmdline_kunit.c
@@ -6,6 +6,7 @@
 #include <kunit/test.h>
 #include <linux/kernel.h>
 #include <linux/random.h>
+#include <linux/sizes.h>
 #include <linux/string.h>
 
 static const char *cmdline_test_strings[] = {
@@ -139,11 +140,63 @@ static void cmdline_test_range(struct kunit *test)
 	} while (++i < ARRAY_SIZE(cmdline_test_range_strings));
 }
 
+struct cmdline_test_memparse_entry {
+	const char *input;
+	const char *unrecognized;
+	unsigned long long result;
+};
+
+static const struct cmdline_test_memparse_entry testdata[] = {
+	{ "0",				"",	0ULL },
+	{ "1",				"",	1ULL },
+	{ "a",				"a",	0ULL },
+	{ "0xb",			"",	11ULL },
+	{ "0xz",			"x",	0ULL },
+	{ "1234",			"",	1234ULL },
+	{ "04567",			"",	2423ULL },
+	{ "0x9876",			"",	39030LL },
+	{ "05678",			"8",	375ULL },
+	{ "0xabcdefz",			"z",	11259375ULL },
+	{ "0cdba",			"c",	0ULL },
+	{ "4K",				"",	SZ_4K },
+	{ "0x10k@0xaaaabbbb",		"@",	SZ_16K },
+	{ "32M",			"",	SZ_32M },
+	{ "067m:foo",			":",	55 * SZ_1M },
+	{ "2G;bar=baz",			";",	SZ_2G },
+	{ "07gz",			"z",	7ULL * SZ_1G },
+	{ "3T+data",			"+",	3 * SZ_1T },
+	{ "04t,ro",			",",	SZ_4T },
+	{ "012p",			"",	11258999068426240ULL },
+	{ "7P,sync",			",",	7881299347898368ULL },
+	{ "0x2e",			"",	46ULL },
+	{ "2E and more",		" ",	2305843009213693952ULL },
+	{ "18446744073709551615",	"",	ULLONG_MAX },
+	{ "1111111111111111111T",	"",	ULLONG_MAX },
+	{ "222222222222222222222G",	"",	ULLONG_MAX },
+	{ "3333333333333333333333M",	"",	ULLONG_MAX },
+};
+
+static void cmdline_test_memparse(struct kunit *test)
+{
+	const struct cmdline_test_memparse_entry *e;
+	unsigned long long ret;
+	char *retptr;
+
+	for (e = testdata; e < testdata + ARRAY_SIZE(testdata); e++) {
+		ret = memparse(e->input, &retptr);
+		KUNIT_EXPECT_EQ_MSG(test, ret, e->result,
+				    "    when parsing '%s'", e->input);
+		KUNIT_EXPECT_EQ_MSG(test, *retptr, *e->unrecognized,
+				    "    when parsing '%s'", e->input);
+	}
+}
+
 static struct kunit_case cmdline_test_cases[] = {
 	KUNIT_CASE(cmdline_test_noint),
 	KUNIT_CASE(cmdline_test_lead_int),
 	KUNIT_CASE(cmdline_test_tail_int),
 	KUNIT_CASE(cmdline_test_range),
+	KUNIT_CASE(cmdline_test_memparse),
 	{}
 };
 
-- 
2.52.0

[PATCH v5 5/5] lib/cmdline: adjust a few comments to fix kernel-doc -Wreturn warnings

[Dmitry Antipov]

Fix 'get_option()', 'memparse()' and 'parse_option_str()' comments
to match the commonly used style as suggested by kernel-doc -Wreturn.

Suggested-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v5: likewise
v4: likewise
v3: likewise
v2: bump version to match the series
---
 lib/cmdline.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index e18fdfb80ba5..c6f7aa5abe77 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -43,7 +43,7 @@ static int get_range(char **str, int *pint, int n)
  *	When @pint is NULL the function can be used as a validator of
  *	the current option in the string.
  *
- *	Return values:
+ *	Return:
  *	0 - no int in string
  *	1 - int found, no subsequent comma
  *	2 - int found including a subsequent comma
@@ -145,6 +145,9 @@ EXPORT_SYMBOL(get_options);
  *
  *	Parses a string into a number.  The number stored at @ptr is
  *	potentially suffixed with K, M, G, T, P, E.
+ *
+ *	Return: The value as recognized by simple_strtoull() multiplied
+ *	by the value as specified by suffix, if any.
  */
 
 unsigned long long memparse(const char *ptr, char **retptr)
@@ -208,7 +211,7 @@ EXPORT_SYMBOL(memparse);
  *	This function parses a string containing a comma-separated list of
  *	strings like a=b,c.
  *
- *	Return true if there's such option in the string, or return false.
+ *	Return: True if there's such option in the string or false otherwise.
  */
 bool parse_option_str(const char *str, const char *option)
 {
-- 
2.52.0

Re: [PATCH v5 1/5] lib: fix _parse_integer_limit() to handle overflow

[Andy Shevchenko]

On Wed, Feb 04, 2026 at 04:57:13PM +0300, Dmitry Antipov wrote:
> In '_parse_integer_limit()', adjust native integer arithmetic
> with near-to-overflow branch where 'check_mul_overflow()' and
> 'check_add_overflow()' are used to check whether an intermediate
> result goes out of range, and denote such a case with ULLONG_MAX,
> thus making the function more similar to standard C library's
> 'strtoull()'. Adjust comment to kernel-doc style as well.

...

>  		/*
> -		 * Check for overflow only if we are within range of
> -		 * it in the max base we support (16)
> +		 * Accumulate result if no overflow detected.
> +		 * Otherwise just consume valid characters.
>  		 */
> -		if (unlikely(res & (~0ull << 60))) {
> -			if (res > div_u64(ULLONG_MAX - val, base))
> -				rv |= KSTRTOX_OVERFLOW;
> +		if (likely(res != ULLONG_MAX)) {
> +			if (unlikely(res & (~0ull << 60))) {
> +				/* We're close to possible overflow. */
> +				if (check_mul_overflow(res, base, &tmp) ||
> +				    check_add_overflow(tmp, val, &res)) {
> +					res = ULLONG_MAX;
> +					rv |= KSTRTOX_OVERFLOW;
> +				}
> +			} else {
> +				res = res * base + val;
> +			}
>  		}
> -		res = res * base + val;
>  		rv++;
>  		s++;

In case you would need a v6, we can leave some of the lines untouched if we
switch to for-loop instead of while, but it might make the for-loop quite long.

I'm okay with the current version, up to you to experiment and choose.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 2/5] lib: fix memparse() to handle overflow

[Andy Shevchenko]

On Wed, Feb 04, 2026 at 04:57:14PM +0300, Dmitry Antipov wrote:
> Since '_parse_integer_limit()' (and so 'simple_strtoull()') is now
> capable to handle overflow, adjust 'memparse()' to handle overflow
> (denoted by ULLONG_MAX) returned from 'simple_strtoull()'. Also
> use 'check_shl_overflow()' to catch an overflow possibly caused
> by processing size suffix and denote it with ULLONG_MAX as well.

Do we have already test cases to cover this?

...

> unsigned long long memparse(const char *ptr, char **retptr)
>  {
>  	char *endptr;	/* local pointer to end of parsed string */

>  

Shouldn't be an empty line in the definition block.

> +	unsigned int shl = 0;
>  	unsigned long long ret = simple_strtoull(ptr, &endptr, 0);

and it would be better to preserve reversed xmas tree order (to some extent).

	char *endptr;	/* local pointer to end of parsed string */
	unsigned long long ret = simple_strtoull(ptr, &endptr, 0);
	unsigned int shl = 0;

> +	/* Consume valid suffix even in case of overflow. */
>  	switch (*endptr) {
>  	case 'E':
>  	case 'e':
> -		ret <<= 10;
> +		shl += 10;
>  		fallthrough;
>  	case 'P':
>  	case 'p':
> -		ret <<= 10;
> +		shl += 10;
>  		fallthrough;
>  	case 'T':
>  	case 't':
> -		ret <<= 10;
> +		shl += 10;
>  		fallthrough;
>  	case 'G':
>  	case 'g':
> -		ret <<= 10;
> +		shl += 10;
>  		fallthrough;
>  	case 'M':
>  	case 'm':
> -		ret <<= 10;
> +		shl += 10;
>  		fallthrough;
>  	case 'K':
>  	case 'k':
> -		ret <<= 10;
> +		shl += 10;
>  		endptr++;
>  		fallthrough;
>  	default:
>  		break;
>  	}

> +	/* If no overflow, apply suffix if any. */
> +	if (likely(ret != ULLONG_MAX) && shl) {

Do we need to check for shl? Yes, it will be an additional check below,
but do we care?

> +		unsigned long long val;

> +		ret = (unlikely(check_shl_overflow(ret, shl, &val))
> +		       ? ULLONG_MAX : val);

Unneeded parentheses, and ? should be on the previous line.
With that said, I prefer to see the regular conditional instead:

		ret = check_shl_overflow(...);
		if (unlikely(ret))
			ret = ...
		else
			ret = val;

> +	}
> +
>  	if (retptr)
>  		*retptr = endptr;

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 1/5] lib: fix _parse_integer_limit() to handle overflow

[Dmitry Antipov]

On Wed, 2026-02-04 at 16:31 +0200, Andy Shevchenko wrote:

> In case you would need a v6, we can leave some of the lines untouched if we
> switch to for-loop instead of while, but it might make the for-loop quite long.

Hmm...the for-loop might be:

for (res = 0, rv = 0; max_chars--; rv++, s++) {
        ...
}

and it makes _parse_integer_limit() a few lines shorter.

Dmitry

Re: [PATCH v5 2/5] lib: fix memparse() to handle overflow

[Dmitry Antipov]

On Wed, 2026-02-04 at 16:42 +0200, Andy Shevchenko wrote:

> On Wed, Feb 04, 2026 at 04:57:14PM +0300, Dmitry Antipov wrote:
> > Since '_parse_integer_limit()' (and so 'simple_strtoull()') is now
> > capable to handle overflow, adjust 'memparse()' to handle overflow
> > (denoted by ULLONG_MAX) returned from 'simple_strtoull()'. Also
> > use 'check_shl_overflow()' to catch an overflow possibly caused
> > by processing size suffix and denote it with ULLONG_MAX as well.
> 
> Do we have already test cases to cover this?

In

static const struct cmdline_test_memparse_entry testdata[] = {
        ...
        { "1111111111111111111T",       "",     ULLONG_MAX },
        ...
};

the whole string is valid and so should be recognized by memparse(). Next,
1111111111111111111 fits unsigned long long but 1111111111111111111 << 40
is too large and should be catched by check_shl_overflow().

Dmitry

Re: [PATCH v5 1/5] lib: fix _parse_integer_limit() to handle overflow

[Andy Shevchenko]

On Thu, Feb 05, 2026 at 12:04:14PM +0300, Dmitry Antipov wrote:
> On Wed, 2026-02-04 at 16:31 +0200, Andy Shevchenko wrote:
> 
> > In case you would need a v6, we can leave some of the lines untouched if we
> > switch to for-loop instead of while, but it might make the for-loop quite long.
> 
> Hmm...the for-loop might be:
> 
> for (res = 0, rv = 0; max_chars--; rv++, s++) {

res = 0 should be left outside, it's not part of the for-loop iterators.

>         ...
> }
> 
> and it makes _parse_integer_limit() a few lines shorter.

Yes, but more disruption on the code, so there are pros and cons,
but if you decide to go with it in v6, I won't object.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 2/5] lib: fix memparse() to handle overflow

[Andy Shevchenko]

On Thu, Feb 05, 2026 at 12:17:16PM +0300, Dmitry Antipov wrote:
> On Wed, 2026-02-04 at 16:42 +0200, Andy Shevchenko wrote:
> > On Wed, Feb 04, 2026 at 04:57:14PM +0300, Dmitry Antipov wrote:
> > > Since '_parse_integer_limit()' (and so 'simple_strtoull()') is now
> > > capable to handle overflow, adjust 'memparse()' to handle overflow
> > > (denoted by ULLONG_MAX) returned from 'simple_strtoull()'. Also
> > > use 'check_shl_overflow()' to catch an overflow possibly caused
> > > by processing size suffix and denote it with ULLONG_MAX as well.
> > 
> > Do we have already test cases to cover this?
> 
> In
> 
> static const struct cmdline_test_memparse_entry testdata[] = {
>         ...
>         { "1111111111111111111T",       "",     ULLONG_MAX },
>         ...
> };
> 
> the whole string is valid and so should be recognized by memparse(). Next,
> 1111111111111111111 fits unsigned long long but 1111111111111111111 << 40
> is too large and should be catched by check_shl_overflow().

But if there is a case already, how does it pass?

My understanding is that if we modify the code behaviour the test cases should
be amended at the same time. I guess I missed something here.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 1/5] lib: fix _parse_integer_limit() to handle overflow

[David Laight]

On Wed,  4 Feb 2026 16:57:13 +0300
Dmitry Antipov <dmantipov@yandex.ru> wrote:

> In '_parse_integer_limit()', adjust native integer arithmetic
> with near-to-overflow branch where 'check_mul_overflow()' and
> 'check_add_overflow()' are used to check whether an intermediate
> result goes out of range, and denote such a case with ULLONG_MAX,
> thus making the function more similar to standard C library's
> 'strtoull()'. Adjust comment to kernel-doc style as well.
> 
> Reviewed-by: Andy Shevchenko <andriy.shevchenko@intel.com>
> Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
> ---
> v5: minor brace style adjustment
> v4: restore plain integer arithmetic and use check_xxx_overflow()
>     on near-to-overflow branch only
> v3: adjust commit message and comments as suggested by Andy
> v2: initial version to join the series
> ---
>  lib/kstrtox.c | 39 ++++++++++++++++++++++++++-------------
>  1 file changed, 26 insertions(+), 13 deletions(-)
> 
> diff --git a/lib/kstrtox.c b/lib/kstrtox.c
> index bdde40cd69d7..8691f85cf2ce 100644
> --- a/lib/kstrtox.c
> +++ b/lib/kstrtox.c
> @@ -39,20 +39,26 @@ const char *_parse_integer_fixup_radix(const char *s, unsigned int *base)
>  	return s;
>  }
>  
> -/*
> - * Convert non-negative integer string representation in explicitly given radix
> - * to an integer. A maximum of max_chars characters will be converted.
> +/**
> + * _parse_integer_limit - Convert integer string representation to an integer
> + * @s: Integer string representation
> + * @base: Radix
> + * @p: Where to store result
> + * @max_chars: Maximum amount of characters to convert
> + *
> + * Convert non-negative integer string representation in explicitly given
> + * radix to an integer. If overflow occurs, value at @p is set to ULLONG_MAX.
>   *
> - * Return number of characters consumed maybe or-ed with overflow bit.
> - * If overflow occurs, result integer (incorrect) is still returned.
> + * This function is the workhorse of other string conversion functions and it
> + * is discouraged to use it explicitly. Consider kstrto*() family instead.
>   *
> - * Don't you dare use this function.
> + * Return: Number of characters consumed, maybe ORed with overflow bit
>   */
>  noinline
>  unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned long long *p,
>  				  size_t max_chars)
>  {
> -	unsigned long long res;
> +	unsigned long long tmp, res;
>  	unsigned int rv;
>  
>  	res = 0;
> @@ -72,14 +78,21 @@ unsigned int _parse_integer_limit(const char *s, unsigned int base, unsigned lon
>  		if (val >= base)
>  			break;
>  		/*
> -		 * Check for overflow only if we are within range of
> -		 * it in the max base we support (16)
> +		 * Accumulate result if no overflow detected.
> +		 * Otherwise just consume valid characters.
>  		 */
> -		if (unlikely(res & (~0ull << 60))) {
> -			if (res > div_u64(ULLONG_MAX - val, base))
> -				rv |= KSTRTOX_OVERFLOW;
> +		if (likely(res != ULLONG_MAX)) {
> +			if (unlikely(res & (~0ull << 60))) {

Aren't those two checks in the wrong order?
The likely/unlikely really don't make that much difference
you want the main test first.

In any case what is the first check for?
I think it just stops 0xffffffffffffffff0 being treated as an error.
If you are trying to skip the rest of the digits after an overflow
you need to check 'rv'.

Although I wonder whether strtoul() (etc) should stop 'eating' input
when the value would overflow and return a pointer to the digit that
caused the error.
Code looking at the terminating character wont be expecting a digit
and will treat it as a syntax error - which is what you are trying to do.

That is a much easier API to use, and a 'drop-in' for existing code.

	David

> +				/* We're close to possible overflow. */
> +				if (check_mul_overflow(res, base, &tmp) ||
> +				    check_add_overflow(tmp, val, &res)) {
> +					res = ULLONG_MAX;
> +					rv |= KSTRTOX_OVERFLOW;
> +				}
> +			} else {
> +				res = res * base + val;
> +			}
>  		}
> -		res = res * base + val;
>  		rv++;
>  		s++;
>  	}

Re: [PATCH v5 1/5] lib: fix _parse_integer_limit() to handle overflow

[Andy Shevchenko]

On Thu, Feb 05, 2026 at 10:15:37PM +0000, David Laight wrote:
> On Wed,  4 Feb 2026 16:57:13 +0300
> Dmitry Antipov <dmantipov@yandex.ru> wrote:

...

> Although I wonder whether strtoul() (etc) should stop 'eating' input
> when the value would overflow

Definitely no stop condition. The idea behind simple_strto*() in the kernel
is that they will help to parse combined strings (several fields in one
*constant* string), not eating the extra "valid" characters (digits) will
be a disaster in a couple of aspects.

> and return a pointer to the digit that caused the error.

No.

> Code looking at the terminating character wont be expecting a digit
> and will treat it as a syntax error - which is what you are trying to do.
> 
> That is a much easier API to use, and a 'drop-in' for existing code.

Maybe, but problematic from the usage point of view as I described above.

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v5 1/5] lib: fix _parse_integer_limit() to handle overflow

[Dmitry Antipov]

On Fri, 2026-02-06 at 09:42 +0200, Andy Shevchenko wrote:

> > Code looking at the terminating character wont be expecting a digit
> > and will treat it as a syntax error - which is what you are trying to do.
> > 
> > That is a much easier API to use, and a 'drop-in' for existing code.
> 
> Maybe, but problematic from the usage point of view as I described above.

Note that was an idea behihd memvalue(), see https://lists.openwall.net/linux-hardening/2026/01/07/23.
IOW since "mem=64K" is expected to be used more often than, say, "mem=64K@0xaaaaa" or "mem=64K,sync",
it may be useful to have a wrapper which is enough to parse "64K" but treat everything else as error.

Dmitry

Re: [PATCH v5 1/5] lib: fix _parse_integer_limit() to handle overflow

[Andy Shevchenko]

On Fri, Feb 06, 2026 at 12:53:50PM +0300, Dmitry Antipov wrote:
> On Fri, 2026-02-06 at 09:42 +0200, Andy Shevchenko wrote:
> 
> > > Code looking at the terminating character wont be expecting a digit
> > > and will treat it as a syntax error - which is what you are trying to do.
> > > 
> > > That is a much easier API to use, and a 'drop-in' for existing code.
> > 
> > Maybe, but problematic from the usage point of view as I described above.
> 
> Note that was an idea behihd memvalue(), see https://lists.openwall.net/linux-hardening/2026/01/07/23.
> IOW since "mem=64K" is expected to be used more often than, say, "mem=64K@0xaaaaa" or "mem=64K,sync",
> it may be useful to have a wrapper which is enough to parse "64K" but treat everything else as error.

It will make it less useful.

-- 
With Best Regards,
Andy Shevchenko