All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v6 0/5] lib and lib/cmdline enhancements
@ 2026-02-09 16:47 Dmitry Antipov
  2026-02-09 16:47 ` [PATCH v6 1/5] lib: fix _parse_integer_limit() to handle overflow Dmitry Antipov
                   ` (5 more replies)
  0 siblings, 6 replies; 13+ messages in thread
From: Dmitry Antipov @ 2026-02-09 16:47 UTC (permalink / raw)
  To: Andy Shevchenko, Andrew Morton
  Cc: Kees Cook, Darrick J . Wong, linux-hardening, linux-kernel,
	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             | 39 ++++++++++++++++++++-------
 lib/kstrtox.c             | 47 ++++++++++++++++++++-------------
 lib/test-kstrtox.c        |  6 +++++
 lib/tests/cmdline_kunit.c | 55 +++++++++++++++++++++++++++++++++++++++
 4 files changed, 120 insertions(+), 27 deletions(-)

-- 
2.53.0


^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH v6 1/5] lib: fix _parse_integer_limit() to handle overflow
  2026-02-09 16:47 [PATCH v6 0/5] lib and lib/cmdline enhancements Dmitry Antipov
@ 2026-02-09 16:47 ` Dmitry Antipov
  2026-02-10  7:36   ` Andy Shevchenko
  2026-02-09 16:47 ` [PATCH v6 2/5] lib: fix memparse() " Dmitry Antipov
                   ` (4 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Dmitry Antipov @ 2026-02-09 16:47 UTC (permalink / raw)
  To: Andy Shevchenko, Andrew Morton
  Cc: Kees Cook, Darrick J . Wong, linux-hardening, linux-kernel,
	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>
---
v6: more compact for-loop and minor style adjustments again
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 | 47 +++++++++++++++++++++++++++++------------------
 1 file changed, 29 insertions(+), 18 deletions(-)

diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index bdde40cd69d7..ab7ce72e36e2 100644
--- a/lib/kstrtox.c
+++ b/lib/kstrtox.c
@@ -39,25 +39,29 @@ 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
  *
- * Return number of characters consumed maybe or-ed with overflow bit.
- * If overflow occurs, result integer (incorrect) is still returned.
+ * Convert non-negative integer string representation in explicitly given
+ * radix to an integer. If overflow occurs, value at @p is set to ULLONG_MAX.
  *
- * Don't you dare use this function.
+ * This function is the workhorse of other string conversion functions and it
+ * is discouraged to use it explicitly. Consider kstrto*() family instead.
+ *
+ * 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 res = 0;
 	unsigned int rv;
 
-	res = 0;
-	rv = 0;
-	while (max_chars--) {
+	for (rv = 0; max_chars--; rv++, s++) {
 		unsigned int c = *s;
 		unsigned int lc = _tolower(c);
 		unsigned int val;
@@ -72,16 +76,23 @@ 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. */
+				unsigned long long tmp;
+
+				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++;
 	}
 	*p = res;
 	return rv;
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 2/5] lib: fix memparse() to handle overflow
  2026-02-09 16:47 [PATCH v6 0/5] lib and lib/cmdline enhancements Dmitry Antipov
  2026-02-09 16:47 ` [PATCH v6 1/5] lib: fix _parse_integer_limit() to handle overflow Dmitry Antipov
@ 2026-02-09 16:47 ` Dmitry Antipov
  2026-02-10  7:51   ` Andy Shevchenko
  2026-02-09 16:47 ` [PATCH v6 3/5] lib: add more string to 64-bit integer conversion overflow tests Dmitry Antipov
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 13+ messages in thread
From: Dmitry Antipov @ 2026-02-09 16:47 UTC (permalink / raw)
  To: Andy Shevchenko, Andrew Morton
  Cc: Kees Cook, Darrick J . Wong, linux-hardening, linux-kernel,
	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>
---
v6: handle valid-suffix-only string like "k"
    as unrecognized, minor style adjustments
v5: initial version to join the series
---
 lib/cmdline.c | 32 +++++++++++++++++++++++++-------
 1 file changed, 25 insertions(+), 7 deletions(-)

diff --git a/lib/cmdline.c b/lib/cmdline.c
index 90ed997d9570..0d8770a0fb67 100644
--- a/lib/cmdline.c
+++ b/lib/cmdline.c
@@ -150,39 +150,57 @@ EXPORT_SYMBOL(get_options);
 unsigned long long memparse(const char *ptr, char **retptr)
 {
 	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 (shl) {
+		/* Valid suffix without preceding number. */
+		if (unlikely(ptr == endptr - 1)) {
+			endptr--;
+			ret = 0;
+		}
+		/* Apply suffix if no overflow. */
+		else if (likely(ret != ULLONG_MAX)) {
+			unsigned long long val;
+
+			if (unlikely(check_shl_overflow(ret, shl, &val)))
+				ret = ULLONG_MAX;
+			else
+				ret = val;
+		}
+	}
+
 	if (retptr)
 		*retptr = endptr;
 
-- 
2.53.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 3/5] lib: add more string to 64-bit integer conversion overflow tests
  2026-02-09 16:47 [PATCH v6 0/5] lib and lib/cmdline enhancements Dmitry Antipov
  2026-02-09 16:47 ` [PATCH v6 1/5] lib: fix _parse_integer_limit() to handle overflow Dmitry Antipov
  2026-02-09 16:47 ` [PATCH v6 2/5] lib: fix memparse() " Dmitry Antipov
@ 2026-02-09 16:47 ` Dmitry Antipov
  2026-02-09 16:47 ` [PATCH v6 4/5] lib/cmdline_kunit: add test case for memparse() Dmitry Antipov
                   ` (2 subsequent siblings)
  5 siblings, 0 replies; 13+ messages in thread
From: Dmitry Antipov @ 2026-02-09 16:47 UTC (permalink / raw)
  To: Andy Shevchenko, Andrew Morton
  Cc: Kees Cook, Darrick J . Wong, linux-hardening, linux-kernel,
	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>
---
v6: likewise
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.53.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 4/5] lib/cmdline_kunit: add test case for memparse()
  2026-02-09 16:47 [PATCH v6 0/5] lib and lib/cmdline enhancements Dmitry Antipov
                   ` (2 preceding siblings ...)
  2026-02-09 16:47 ` [PATCH v6 3/5] lib: add more string to 64-bit integer conversion overflow tests Dmitry Antipov
@ 2026-02-09 16:47 ` Dmitry Antipov
  2026-02-09 16:47 ` [PATCH v6 5/5] lib/cmdline: adjust a few comments to fix kernel-doc -Wreturn warnings Dmitry Antipov
  2026-02-10  7:53 ` [PATCH v6 0/5] lib and lib/cmdline enhancements Andy Shevchenko
  5 siblings, 0 replies; 13+ messages in thread
From: Dmitry Antipov @ 2026-02-09 16:47 UTC (permalink / raw)
  To: Andy Shevchenko, Andrew Morton
  Cc: Kees Cook, Darrick J . Wong, linux-hardening, linux-kernel,
	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>
---
v6: tests to check whether valid-suffix-only string is handled as unrecognized
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 | 55 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 55 insertions(+)

diff --git a/lib/tests/cmdline_kunit.c b/lib/tests/cmdline_kunit.c
index c1602f797637..4827c4753386 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,65 @@ 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 },
+	{ "k",				"k",	0ULL },
+	{ "E",				"E",	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.53.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH v6 5/5] lib/cmdline: adjust a few comments to fix kernel-doc -Wreturn warnings
  2026-02-09 16:47 [PATCH v6 0/5] lib and lib/cmdline enhancements Dmitry Antipov
                   ` (3 preceding siblings ...)
  2026-02-09 16:47 ` [PATCH v6 4/5] lib/cmdline_kunit: add test case for memparse() Dmitry Antipov
@ 2026-02-09 16:47 ` Dmitry Antipov
  2026-02-10  7:53 ` [PATCH v6 0/5] lib and lib/cmdline enhancements Andy Shevchenko
  5 siblings, 0 replies; 13+ messages in thread
From: Dmitry Antipov @ 2026-02-09 16:47 UTC (permalink / raw)
  To: Andy Shevchenko, Andrew Morton
  Cc: Kees Cook, Darrick J . Wong, linux-hardening, linux-kernel,
	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>
---
v3 and upwards: 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 0d8770a0fb67..d21075f0c5d7 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)
@@ -216,7 +219,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.53.0


^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 1/5] lib: fix _parse_integer_limit() to handle overflow
  2026-02-09 16:47 ` [PATCH v6 1/5] lib: fix _parse_integer_limit() to handle overflow Dmitry Antipov
@ 2026-02-10  7:36   ` Andy Shevchenko
  2026-02-12 11:13     ` Dmitry Antipov
  0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2026-02-10  7:36 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Andrew Morton, Kees Cook, Darrick J . Wong, linux-hardening,
	linux-kernel

On Mon, Feb 09, 2026 at 07:47:53PM +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.

...

> -	unsigned long long res;
> +	unsigned long long res = 0;

>  
> -	res = 0;

We can leave this untouched.

...


> -	while (max_chars--) {
> +	for (rv = 0; max_chars--; rv++, s++) {

I don't see how max_chars is used. With that said, I would rather see the usual
way of expressing the condition in the for-loop:

	for (rv = 0; rv < max_chars; rv++, s++) {

...

> +		if (likely(res != ULLONG_MAX)) {

Have you seen David's question about these checks?
Maybe I missed your answer...

> +			if (unlikely(res & (~0ull << 60))) {
> +				/* We're close to possible overflow. */
> +				unsigned long long tmp;
> +
> +				if (check_mul_overflow(res, base, &tmp) ||
> +				    check_add_overflow(tmp, val, &res)) {
> +					res = ULLONG_MAX;
> +					rv |= KSTRTOX_OVERFLOW;
> +				}
> +			} else {
> +				res = res * base + val;
> +			}
>  		}

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 2/5] lib: fix memparse() to handle overflow
  2026-02-09 16:47 ` [PATCH v6 2/5] lib: fix memparse() " Dmitry Antipov
@ 2026-02-10  7:51   ` Andy Shevchenko
  2026-02-12 11:21     ` Dmitry Antipov
  0 siblings, 1 reply; 13+ messages in thread
From: Andy Shevchenko @ 2026-02-10  7:51 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Andrew Morton, Kees Cook, Darrick J . Wong, linux-hardening,
	linux-kernel

On Mon, Feb 09, 2026 at 07:47:54PM +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.

...

>  unsigned long long memparse(const char *ptr, char **retptr)
>  {
>  	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 (shl) {
> +		/* Valid suffix without preceding number. */
> +		if (unlikely(ptr == endptr - 1)) {

I believe this can be optimised with the endptr++ moved somewhere here.
I have not yet a clear picture in my mind, just gut feelings, so please
try to think about it. With that we won't need endptr--.

> +			endptr--;

> +			ret = 0;

Wouldn't ret be already 0 here?

> +		}
> +		/* Apply suffix if no overflow. */
> +		else if (likely(ret != ULLONG_MAX)) {

Should be (style)

		/* Apply suffix if no overflow. */
		} else if (likely(ret != ULLONG_MAX)) {

> +			unsigned long long val;
> +
> +			if (unlikely(check_shl_overflow(ret, shl, &val)))
> +				ret = ULLONG_MAX;
> +			else
> +				ret = val;
> +		}
> +	}

Strictly speaking this is an ABI breakage. I dunno how many (broken) strings
will stop working after this check.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 0/5] lib and lib/cmdline enhancements
  2026-02-09 16:47 [PATCH v6 0/5] lib and lib/cmdline enhancements Dmitry Antipov
                   ` (4 preceding siblings ...)
  2026-02-09 16:47 ` [PATCH v6 5/5] lib/cmdline: adjust a few comments to fix kernel-doc -Wreturn warnings Dmitry Antipov
@ 2026-02-10  7:53 ` Andy Shevchenko
  5 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2026-02-10  7:53 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Andrew Morton, Kees Cook, Darrick J . Wong, linux-hardening,
	linux-kernel

On Mon, Feb 09, 2026 at 07:47:52PM +0300, Dmitry Antipov wrote:
> 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.

Thanks for the update!

I have some minor comments per patch 1, and one main Q in patch 2 about ABI.

-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 1/5] lib: fix _parse_integer_limit() to handle overflow
  2026-02-10  7:36   ` Andy Shevchenko
@ 2026-02-12 11:13     ` Dmitry Antipov
       [not found]       ` <20260212120030.2f15caaa@pumpkin>
  0 siblings, 1 reply; 13+ messages in thread
From: Dmitry Antipov @ 2026-02-12 11:13 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andrew Morton, David Laight, Kees Cook, Darrick J . Wong,
	linux-hardening, linux-kernel

On Tue, 2026-02-10 at 09:36 +0200, Andy Shevchenko wrote:

> I don't see how max_chars is used. With that said, I would rather see the usual
> way of expressing the condition in the for-loop:
> 
> 	for (rv = 0; rv < max_chars; rv++, s++) {

This will break the loop (and so stop consuming characters) if KSTRTOX_OVERFLOW
bit is set.

> > +		if (likely(res != ULLONG_MAX)) {
> 
> Have you seen David's question about these checks?
> Maybe I missed your answer...
> 
> > +			if (unlikely(res & (~0ull << 60))) {

The first check may be dropped indeed (assuming check_mul_overflow(ULLONG_MAX, a, b)
and check_add_overflow(ULLONG_MAX, a, b) always signals an overflow).

Dmitry

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 2/5] lib: fix memparse() to handle overflow
  2026-02-10  7:51   ` Andy Shevchenko
@ 2026-02-12 11:21     ` Dmitry Antipov
  0 siblings, 0 replies; 13+ messages in thread
From: Dmitry Antipov @ 2026-02-12 11:21 UTC (permalink / raw)
  To: Andy Shevchenko
  Cc: Andrew Morton, Kees Cook, Darrick J . Wong, linux-hardening,
	linux-kernel

On Tue, 2026-02-10 at 09:51 +0200, Andy Shevchenko wrote:

> Strictly speaking this is an ABI breakage. I dunno how many (broken) strings
> will stop working after this check.

Yes, but this is for the corner case only (where ULLONG_MAX is used instead of
ignoring an overflow). And finding more bugs is better than silently hiding them.

Dmitry

^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 1/5] lib: fix _parse_integer_limit() to handle overflow
       [not found]       ` <20260212120030.2f15caaa@pumpkin>
@ 2026-02-12 13:25         ` David Laight
  2026-02-12 13:37           ` Andy Shevchenko
  0 siblings, 1 reply; 13+ messages in thread
From: David Laight @ 2026-02-12 13:25 UTC (permalink / raw)
  To: Dmitry Antipov
  Cc: Andy Shevchenko, Andrew Morton, Kees Cook, djwong,
	linux-hardening, linux-kernel

On Thu, 12 Feb 2026 12:00:30 +0000
David Laight <david.laight.linux@gmail.com> wrote:

Re-send with "..." removed from one of the addresses so my MUA (claws) won't
escape the second one and the list-servers fail to accept the mail.

> On Thu, 12 Feb 2026 14:13:16 +0300
> Dmitry Antipov <dmantipov@yandex.ru> wrote:
> 
> > On Tue, 2026-02-10 at 09:36 +0200, Andy Shevchenko wrote:
> >   
> > > I don't see how max_chars is used. With that said, I would rather see the usual
> > > way of expressing the condition in the for-loop:
> > > 
> > > 	for (rv = 0; rv < max_chars; rv++, s++) {    
> > 
> > This will break the loop (and so stop consuming characters) if KSTRTOX_OVERFLOW
> > bit is set.
> >   
> > > > +		if (likely(res != ULLONG_MAX)) {    
> > > 
> > > Have you seen David's question about these checks?
> > > Maybe I missed your answer...  
> 
> I've not seen one...
> 
> > >     
> > > > +			if (unlikely(res & (~0ull << 60))) {    
> > 
> > The first check may be dropped indeed (assuming check_mul_overflow(ULLONG_MAX, a, b)
> > and check_add_overflow(ULLONG_MAX, a, b) always signals an overflow).  
> 
> That check for the high bits may well be cheaper than the one in
> check_mul_overflow() - which is likely to need to partially generate
> the 128bit result.
> Also if the code is going to call check_mul_overflow() it ought to use the
> result in the 'non-overflow' case.
> 
> But there is nothing 'magic' about check_mul_overflow(), given the base
> is known (and the only dificult one is 10) comparing against the known
> limit will be better code.
> 
> 	David
> 
> 
> > 
> > Dmitry  
> 


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH v6 1/5] lib: fix _parse_integer_limit() to handle overflow
  2026-02-12 13:25         ` David Laight
@ 2026-02-12 13:37           ` Andy Shevchenko
  0 siblings, 0 replies; 13+ messages in thread
From: Andy Shevchenko @ 2026-02-12 13:37 UTC (permalink / raw)
  To: David Laight
  Cc: Dmitry Antipov, Andrew Morton, Kees Cook, djwong, linux-hardening,
	linux-kernel

On Thu, Feb 12, 2026 at 01:25:17PM +0000, David Laight wrote:
> On Thu, 12 Feb 2026 12:00:30 +0000
> David Laight <david.laight.linux@gmail.com> wrote:
> 
> Re-send with "..." removed from one of the addresses so my MUA (claws) won't
> escape the second one and the list-servers fail to accept the mail.

There is a v7, perhaps you can reply there?


-- 
With Best Regards,
Andy Shevchenko



^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2026-02-12 13:37 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-02-09 16:47 [PATCH v6 0/5] lib and lib/cmdline enhancements Dmitry Antipov
2026-02-09 16:47 ` [PATCH v6 1/5] lib: fix _parse_integer_limit() to handle overflow Dmitry Antipov
2026-02-10  7:36   ` Andy Shevchenko
2026-02-12 11:13     ` Dmitry Antipov
     [not found]       ` <20260212120030.2f15caaa@pumpkin>
2026-02-12 13:25         ` David Laight
2026-02-12 13:37           ` Andy Shevchenko
2026-02-09 16:47 ` [PATCH v6 2/5] lib: fix memparse() " Dmitry Antipov
2026-02-10  7:51   ` Andy Shevchenko
2026-02-12 11:21     ` Dmitry Antipov
2026-02-09 16:47 ` [PATCH v6 3/5] lib: add more string to 64-bit integer conversion overflow tests Dmitry Antipov
2026-02-09 16:47 ` [PATCH v6 4/5] lib/cmdline_kunit: add test case for memparse() Dmitry Antipov
2026-02-09 16:47 ` [PATCH v6 5/5] lib/cmdline: adjust a few comments to fix kernel-doc -Wreturn warnings Dmitry Antipov
2026-02-10  7:53 ` [PATCH v6 0/5] lib and lib/cmdline enhancements Andy Shevchenko

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.