public inbox for linux-hardening@vger.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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox