[PATCH 1/2] lib: parser: reject out-of-range values in match_number()

[PATCH 1/2] lib: parser: reject out-of-range values in match_number()

[shashank]

match_int(), match_octal() and match_hex() store the result in an int
and return -EINVAL or -ERANGE on failure.  match_number() checks the
range by parsing into a long with simple_strtol() and comparing against
INT_MIN/INT_MAX, a check added by commit 77dd3b0bd17a ("lib/parser.c:
avoid overflow in match_number()").  That does not catch every
out-of-range input:

 - simple_strtoull() saturates to ULLONG_MAX on overflow and
   simple_strtol() simply converts its result to long, so any value of
   at least 2^64 - 2^31, and anything that overflows 64 bits, ends up
   inside the int range.  On 64-bit, match_int() returns 0 and sets
   the result to -1 for "18446744073709551615" or
   "99999999999999999999", match_hex() does the same for
   "ffffffffffffffff", and "-18446744073709551615" gives 1.

 - On 32-bit, long has the same width as int, so the range check can
   never fail: "2147483648" gives INT_MIN and "4294967295" gives -1.

On 64-bit, values from INT_MAX + 1 up to 2^64 - 2^31 - 1 are already
rejected, so for those this only makes 32-bit kernels behave like
64-bit ones.  Two callers store the result in a u32 and so accepted
such values on 32-bit only: the legacy NFSv4 idmapper upcall
(fs/nfs/nfs4idmap.c), which falls back to a numeric id when the lookup
fails, and rd_pages= in drivers/target/target_core_rd.c, which also
ignores match_int()'s return value.

These helpers parse mount options and similar user-supplied strings,
so an out-of-range number is silently accepted as a different value
instead of being rejected.

Parse the number the same way simple_strtol() does, using the same
kstrtox helpers, but keep the unsigned magnitude and the overflow
indication, and check the magnitude against the int range.  The
accepted syntax, including what counts as "no number" (-EINVAL), is
unchanged.  kstrtoint() is not used because it rejects trailing
characters ("12abc" gives 12 today) and differs in the handling of a
lone "-" or "0x".

Fixes: 77dd3b0bd17a ("lib/parser.c: avoid overflow in match_number()")
Assisted-by: LLM
Signed-off-by: shashank <jain.sm@gmail.com>
---
 lib/parser.c | 39 +++++++++++++++++++++++++++------------
 1 file changed, 27 insertions(+), 12 deletions(-)

diff --git a/lib/parser.c b/lib/parser.c
index 62da0ac0d438..30fdaefe4957 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -11,6 +11,8 @@
 #include <linux/slab.h>
 #include <linux/string.h>
 
+#include "kstrtox.h"
+
 /*
  * max size needed by different bases to express U64
  * HEX: "0xFFFFFFFFFFFFFFFF" --> 18
@@ -137,22 +139,35 @@ EXPORT_SYMBOL(match_token);
  */
 static int match_number(substring_t *s, int *result, int base)
 {
-	char *endp;
 	char buf[NUMBER_BUF_LEN];
-	int ret;
-	long val;
+	unsigned long long val;
+	unsigned int radix = base;
+	const char *cp = buf;
+	bool negative;
+	unsigned int rv;
 
 	if (match_strlcpy(buf, s, NUMBER_BUF_LEN) >= NUMBER_BUF_LEN)
 		return -ERANGE;
-	ret = 0;
-	val = simple_strtol(buf, &endp, base);
-	if (endp == buf)
-		ret = -EINVAL;
-	else if (val < (long)INT_MIN || val > (long)INT_MAX)
-		ret = -ERANGE;
-	else
-		*result = (int) val;
-	return ret;
+
+	/*
+	 * Parse like simple_strtol() does, but keep the magnitude and the
+	 * overflow indication instead of truncating to a long, so that
+	 * values outside the range of an int can be rejected.
+	 */
+	negative = *cp == '-';
+	if (negative)
+		cp++;
+	cp = _parse_integer_fixup_radix(cp, &radix);
+	rv = _parse_integer(cp, radix, &val);
+	if (cp + (rv & ~KSTRTOX_OVERFLOW) == buf)
+		return -EINVAL;
+
+	if (rv & KSTRTOX_OVERFLOW ||
+	    val > (negative ? (unsigned long long)INT_MAX + 1 : INT_MAX))
+		return -ERANGE;
+
+	*result = negative ? -(long long)val : (long long)val;
+	return 0;
 }
 
 /**
-- 
2.43.0

[PATCH 2/2] lib/tests: add KUnit test for parser number helpers

[shashank]

Add a small KUnit suite for match_int(), match_octal() and match_hex(),
covering values at and just beyond the int range, values that overflow
64 bits, and a match_token() round trip with an out-of-range argument.

Assisted-by: LLM
Signed-off-by: shashank <jain.sm@gmail.com>
---
 lib/Kconfig.debug        |  12 ++++
 lib/tests/Makefile       |   1 +
 lib/tests/parser_kunit.c | 131 +++++++++++++++++++++++++++++++++++++++
 3 files changed, 144 insertions(+)
 create mode 100644 lib/tests/parser_kunit.c

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 134b15a44625..905ccec61868 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -2918,6 +2918,18 @@ config CMDLINE_KUNIT_TEST
 
 	  If unsure, say N.
 
+config PARSER_KUNIT_TEST
+	tristate "KUnit test for parser number helpers" if !KUNIT_ALL_TESTS
+	depends on KUNIT
+	default KUNIT_ALL_TESTS
+	help
+	  This builds the unit test for the number parsing helpers in
+	  lib/parser.c (match_int(), match_octal(), match_hex()).
+	  For more information on KUnit and unit tests in general please refer
+	  to the KUnit documentation in Documentation/dev-tools/kunit/.
+
+	  If unsure, say N.
+
 config BASE64_KUNIT
 	tristate "KUnit test for base64 decoding and encoding" if !KUNIT_ALL_TESTS
 	depends on KUNIT
diff --git a/lib/tests/Makefile b/lib/tests/Makefile
index 3cac3b63a752..cbabf4939ce8 100644
--- a/lib/tests/Makefile
+++ b/lib/tests/Makefile
@@ -41,6 +41,7 @@ obj-$(CONFIG_MEMCPY_KUNIT_TEST) += memcpy_kunit.o
 obj-$(CONFIG_MIN_HEAP_KUNIT_TEST) += min_heap_kunit.o
 CFLAGS_overflow_kunit.o = $(call cc-disable-warning, tautological-constant-out-of-range-compare)
 obj-$(CONFIG_OVERFLOW_KUNIT_TEST) += overflow_kunit.o
+obj-$(CONFIG_PARSER_KUNIT_TEST) += parser_kunit.o
 obj-$(CONFIG_PRANDOM_KUNIT_TEST) += random32_kunit.o
 obj-$(CONFIG_PRINTF_KUNIT_TEST) += printf_kunit.o
 obj-$(CONFIG_RANDSTRUCT_KUNIT_TEST) += randstruct_kunit.o
diff --git a/lib/tests/parser_kunit.c b/lib/tests/parser_kunit.c
new file mode 100644
index 000000000000..b92476cf711f
--- /dev/null
+++ b/lib/tests/parser_kunit.c
@@ -0,0 +1,131 @@
+// SPDX-License-Identifier: GPL-2.0-only
+/*
+ * KUnit tests for the number helpers in lib/parser.c
+ */
+
+#include <kunit/test.h>
+#include <linux/limits.h>
+#include <linux/parser.h>
+#include <linux/string.h>
+
+struct parser_number_case {
+	const char *in;
+	int ret;
+	int val;
+};
+
+typedef int (*match_fn_t)(substring_t *s, int *result);
+
+static void parser_check(struct kunit *test, match_fn_t fn, const char *name,
+			 const struct parser_number_case *c, size_t n)
+{
+	size_t i;
+
+	for (i = 0; i < n; i++) {
+		char buf[64];
+		substring_t s;
+		int val = 0x5a5a5a5a;
+		int ret;
+
+		strscpy(buf, c[i].in, sizeof(buf));
+		s.from = buf;
+		s.to = buf + strlen(buf);
+
+		ret = fn(&s, &val);
+		KUNIT_EXPECT_EQ_MSG(test, ret, c[i].ret, "%s(\"%s\")",
+				    name, c[i].in);
+		if (!c[i].ret)
+			KUNIT_EXPECT_EQ_MSG(test, val, c[i].val, "%s(\"%s\")",
+					    name, c[i].in);
+	}
+}
+
+static const struct parser_number_case match_int_cases[] = {
+	{ "0",				0,		0 },
+	{ "42",				0,		42 },
+	{ "-42",			0,		-42 },
+	{ "0x10",			0,		16 },
+	{ "010",			0,		8 },
+	{ "2147483647",			0,		INT_MAX },
+	{ "-2147483648",		0,		INT_MIN },
+	{ "2147483648",			-ERANGE,	0 },
+	{ "-2147483649",		-ERANGE,	0 },
+	{ "4294967295",			-ERANGE,	0 },
+	{ "9223372036854775808",	-ERANGE,	0 },
+	{ "18446744073709551615",	-ERANGE,	0 },
+	{ "-18446744073709551615",	-ERANGE,	0 },
+	{ "99999999999999999999",	-ERANGE,	0 },
+	{ "0xffffffffffffffff",		-ERANGE,	0 },
+	{ "abc",			-EINVAL,	0 },
+};
+
+static const struct parser_number_case match_octal_cases[] = {
+	{ "755",			0,		0755 },
+	{ "17777777777",		0,		INT_MAX },
+	{ "20000000000",		-ERANGE,	0 },
+	{ "1777777777777777777777",	-ERANGE,	0 },
+	{ "8",				-EINVAL,	0 },
+};
+
+static const struct parser_number_case match_hex_cases[] = {
+	{ "ff",				0,		0xff },
+	{ "7fffffff",			0,		INT_MAX },
+	{ "80000000",			-ERANGE,	0 },
+	{ "ffffffffffffffff",		-ERANGE,	0 },
+	{ "10000000000000000",		-ERANGE,	0 },
+	{ "g",				-EINVAL,	0 },
+};
+
+static void parser_test_match_int(struct kunit *test)
+{
+	parser_check(test, match_int, "match_int", match_int_cases,
+		     ARRAY_SIZE(match_int_cases));
+}
+
+static void parser_test_match_octal(struct kunit *test)
+{
+	parser_check(test, match_octal, "match_octal", match_octal_cases,
+		     ARRAY_SIZE(match_octal_cases));
+}
+
+static void parser_test_match_hex(struct kunit *test)
+{
+	parser_check(test, match_hex, "match_hex", match_hex_cases,
+		     ARRAY_SIZE(match_hex_cases));
+}
+
+enum { PARSER_TEST_OPT_SIZE, PARSER_TEST_OPT_ERR };
+
+static const match_table_t parser_test_tokens = {
+	{ PARSER_TEST_OPT_SIZE, "size=%d" },
+	{ PARSER_TEST_OPT_ERR, NULL },
+};
+
+static void parser_test_match_token_int(struct kunit *test)
+{
+	char opt[] = "size=18446744073709551615";
+	substring_t args[MAX_OPT_ARGS];
+	int val = 0;
+
+	KUNIT_ASSERT_EQ(test, match_token(opt, parser_test_tokens, args),
+			PARSER_TEST_OPT_SIZE);
+	KUNIT_EXPECT_EQ(test, match_int(&args[0], &val), -ERANGE);
+}
+
+static struct kunit_case parser_test_cases[] = {
+	KUNIT_CASE(parser_test_match_int),
+	KUNIT_CASE(parser_test_match_octal),
+	KUNIT_CASE(parser_test_match_hex),
+	KUNIT_CASE(parser_test_match_token_int),
+	{}
+};
+
+static struct kunit_suite parser_test_suite = {
+	.name = "parser",
+	.test_cases = parser_test_cases,
+};
+
+kunit_test_suite(parser_test_suite);
+
+MODULE_DESCRIPTION("KUnit tests for lib/parser.c number helpers");
+MODULE_LICENSE("GPL");
-- 
2.43.0