[PATCH v3 0/3] lib and lib/cmdline enhancements

[PATCH v3 0/3] lib and lib/cmdline enhancements

[Dmitry Antipov]

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

Dmitry Antipov (3):
  lib: fix _parse_integer_limit() to handle overflow
  lib/cmdline_kunit: add test case for memparse()
  lib/cmdline: adjust a few comments to fix kernel-doc -Wreturn warnings

 lib/cmdline.c             |  7 ++++--
 lib/kstrtox.c             | 42 ++++++++++++++++++++++---------
 lib/tests/cmdline_kunit.c | 52 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 87 insertions(+), 14 deletions(-)

-- 
2.52.0

[PATCH v3 1/3] lib: fix _parse_integer_limit() to handle overflow

[Dmitry Antipov]

In '_parse_integer_limit()', replace native integer arithmetic with
'check_mul_overflow()' and 'check_add_overflow()' 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.

Signed-off-by: Dmitry Antipov <dmantipov@yandex.ru>
---
v3: adjust commit message and comments as suggested by Andy
v2: initial version to join the series
---
 lib/kstrtox.c | 42 ++++++++++++++++++++++++++++++------------
 1 file changed, 30 insertions(+), 12 deletions(-)

diff --git a/lib/kstrtox.c b/lib/kstrtox.c
index bdde40cd69d7..443f89e92fa6 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,26 @@ 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))
+		if (res != ULLONG_MAX) {
+			/*
+			 * tmp = res * base;
+			 * if (overflow)
+			 *	res = ULLONG_MAX;
+			 * else {
+			 *	res = tmp + val;
+			 *	if (overflow)
+			 *		res = ULLONG_MAX;
+			 * }
+			 */
+			if (check_mul_overflow(res, base, &tmp) ||
+			    check_add_overflow(tmp, val, &res)) {
+				res = ULLONG_MAX;
 				rv |= KSTRTOX_OVERFLOW;
+			}
 		}
-		res = res * base + val;
 		rv++;
 		s++;
 	}
-- 
2.52.0

[PATCH v3 2/3] 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>
---
v3: adjust style as suggested by Andy
v2: few more test cases to trigger overflows
---
 lib/tests/cmdline_kunit.c | 52 +++++++++++++++++++++++++++++++++++++++
 1 file changed, 52 insertions(+)

diff --git a/lib/tests/cmdline_kunit.c b/lib/tests/cmdline_kunit.c
index c1602f797637..3eda70487719 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,62 @@ 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 },
+	{ "18446744073709551616", 	"",	ULLONG_MAX },
+	{ "569202370375329612767", 	"",	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 v3 3/3] 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>
---
v3: likewise
v2: just 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 90ed997d9570..9ac23a0aecf7 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)
@@ -198,7 +201,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 v3 1/3] lib: fix _parse_integer_limit() to handle overflow

[Andy Shevchenko]

On Mon, Jan 26, 2026 at 07:20:57PM +0300, Dmitry Antipov wrote:
> In '_parse_integer_limit()', replace native integer arithmetic with
> 'check_mul_overflow()' and 'check_add_overflow()' 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.

...

> -		if (unlikely(res & (~0ull << 60))) {
> -			if (res > div_u64(ULLONG_MAX - val, base))

Interestingly, but the original check was made to improve performance. We don't
need to worry about overflow unless we close to it. It also has a hint to the
compiler to take branch as a slow path.

> +		if (res != ULLONG_MAX) {
> +			/*
> +			 * tmp = res * base;
> +			 * if (overflow)
> +			 *	res = ULLONG_MAX;
> +			 * else {
> +			 *	res = tmp + val;
> +			 *	if (overflow)
> +			 *		res = ULLONG_MAX;
> +			 * }
> +			 */

This looks like a left over. Use plain English to explain what's going on
here. But I think this should be only done for the last a couple of iterations
only.

> +			if (check_mul_overflow(res, base, &tmp) ||
> +			    check_add_overflow(tmp, val, &res)) {
> +				res = ULLONG_MAX;
>  				rv |= KSTRTOX_OVERFLOW;
> +			}
>  		}
> -		res = res * base + val;
>  		rv++;
>  		s++;
>  	}

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 2/3] lib/cmdline_kunit: add test case for memparse()

[Andy Shevchenko]

On Mon, Jan 26, 2026 at 07:20:58PM +0300, Dmitry Antipov wrote:
> Better late than never, now there is a long-awaited basic
> test for 'memparse()' which is provided by cmdline.c.

...

> +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 },
> +	{ "18446744073709551616", 	"",	ULLONG_MAX },
> +	{ "569202370375329612767", 	"",	ULLONG_MAX },

There are spaces left after first value in these three lines.

> +};

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/3] lib and lib/cmdline enhancements

[Andy Shevchenko]

On Mon, Jan 26, 2026 at 07:20:56PM +0300, Dmitry Antipov wrote:
> Adjust '_parse_integer_limit()' to not ignore overflows, add KUnit-based
> test for 'memparse()' and fix kernel-doc glitches found in lib/cmdline.c.

Shouldn't we also add test cases to lib/test-kstrtox.c?

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v3 0/3] lib and lib/cmdline enhancements

[Andy Shevchenko]

On Mon, Jan 26, 2026 at 06:43:19PM +0200, Andy Shevchenko wrote:
> On Mon, Jan 26, 2026 at 07:20:56PM +0300, Dmitry Antipov wrote:
> > Adjust '_parse_integer_limit()' to not ignore overflows, add KUnit-based
> > test for 'memparse()' and fix kernel-doc glitches found in lib/cmdline.c.
> 
> Shouldn't we also add test cases to lib/test-kstrtox.c?

Actually I think some of what you added for memparse(), should be moved to
test-kstrtox.c as they test the beneath implementation rather than memparse()
added code.

-- 
With Best Regards,
Andy Shevchenko