mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
* [PATCH v3 1/2] lib: parser: reject out-of-range values in match_number()
@ 2026-10-05  1:44 Shashank Mohan Jain
  2026-10-05  1:44 ` [PATCH v3 2/2] lib/tests: add KUnit test for parser number helpers Shashank Mohan Jain
  2026-10-05 15:10 ` [PATCH v3 1/2] lib: parser: reject out-of-range values in match_number() Alex Elder
  0 siblings, 2 replies; 4+ messages in thread
From: Shashank Mohan Jain @ 2026-10-05  1:44 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Alex Elder, David Gow, linux-kernel

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.

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.

Use kstrtol(), as the comment above simple_strtol() recommends.  It
returns -ERANGE for values that don't fit in a long, which on 32-bit
is the whole int range check, and the INT_MIN/INT_MAX check covers
64-bit.  The substrings passed in come from match_one(), which ends a
%d, %o or %x argument where simple_strtol()/simple_strtoul() stops,
so kstrtol() sees the same characters and accepts the same values in
the int range as before.

Fixes: 77dd3b0bd17a ("lib/parser.c: avoid overflow in match_number()")
Suggested-by: Alex Elder <elder@kernel.org>
Assisted-by: LLM
Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
---
These patches were prepared with Claude Code (Anthropic), model Claude Opus 5.5
(claude-opus-5-5): the analysis, the Lean models used to find and check the bugs,
the fix, the tests, and the check of the in-tree callers for v3.

Changes in v3:
- Use kstrtol() as suggested by Alex Elder, instead of open-coding the
  parse with _parse_integer(); the changelog says why it accepts the same
  values for the callers. The existing "int ret" and "long val"
  declarations are kept, so the diff only replaces the parse.
  Patch 2/2 (the KUnit test) is unchanged.

v2: https://lore.kernel.org/r/20260926012718.15675-1-jain.sm@gmail.com
v1: https://lore.kernel.org/r/20260925102334.49693-1-jain.sm@gmail.com

 lib/parser.c | 17 +++++++----------
 1 file changed, 7 insertions(+), 10 deletions(-)

diff --git a/lib/parser.c b/lib/parser.c
index 62da0ac..5e3abf0 100644
--- a/lib/parser.c
+++ b/lib/parser.c
@@ -137,22 +137,19 @@ 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;
 
 	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;
+	ret = kstrtol(buf, base, &val);
+	if (ret)
+		return ret;
+	if (val < (long)INT_MIN || val > (long)INT_MAX)
+		return -ERANGE;
+	*result = (int) val;
+	return 0;
 }
 
 /**
-- 
2.54.0 (Apple Git-157)


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

* [PATCH v3 2/2] lib/tests: add KUnit test for parser number helpers
  2026-10-05  1:44 [PATCH v3 1/2] lib: parser: reject out-of-range values in match_number() Shashank Mohan Jain
@ 2026-10-05  1:44 ` Shashank Mohan Jain
  2026-10-05 15:10 ` [PATCH v3 1/2] lib: parser: reject out-of-range values in match_number() Alex Elder
  1 sibling, 0 replies; 4+ messages in thread
From: Shashank Mohan Jain @ 2026-10-05  1:44 UTC (permalink / raw)
  To: Andrew Morton; +Cc: Alex Elder, David Gow, linux-kernel

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 Mohan Jain <jain.sm@gmail.com>
---
Changes in v3:
- None (the test passes with the kstrtol() based match_number() of patch 1/2).

 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


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

* Re: [PATCH v3 1/2] lib: parser: reject out-of-range values in match_number()
  2026-10-05  1:44 [PATCH v3 1/2] lib: parser: reject out-of-range values in match_number() Shashank Mohan Jain
  2026-10-05  1:44 ` [PATCH v3 2/2] lib/tests: add KUnit test for parser number helpers Shashank Mohan Jain
@ 2026-10-05 15:10 ` Alex Elder
  2026-10-06  4:42   ` shashank Jain
  1 sibling, 1 reply; 4+ messages in thread
From: Alex Elder @ 2026-10-05 15:10 UTC (permalink / raw)
  To: Shashank Mohan Jain, Andrew Morton; +Cc: Alex Elder, David Gow, linux-kernel

On 10/4/26 8:44 PM, Shashank Mohan Jain wrote:
> 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.
> 
> 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.
> 
> Use kstrtol(), as the comment above simple_strtol() recommends.  It
> returns -ERANGE for values that don't fit in a long, which on 32-bit
> is the whole int range check, and the INT_MIN/INT_MAX check covers
> 64-bit.  The substrings passed in come from match_one(), which ends a
> %d, %o or %x argument where simple_strtol()/simple_strtoul() stops,
> so kstrtol() sees the same characters and accepts the same values in
> the int range as before.
> 
> Fixes: 77dd3b0bd17a ("lib/parser.c: avoid overflow in match_number()")
> Suggested-by: Alex Elder <elder@kernel.org>
> Assisted-by: LLM
> Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
> ---
> These patches were prepared with Claude Code (Anthropic), model Claude Opus 5.5
> (claude-opus-5-5): the analysis, the Lean models used to find and check the bugs,
> the fix, the tests, and the check of the in-tree callers for v3.
> 
> Changes in v3:
> - Use kstrtol() as suggested by Alex Elder, instead of open-coding the
>    parse with _parse_integer(); the changelog says why it accepts the same
>    values for the callers. The existing "int ret" and "long val"
>    declarations are kept, so the diff only replaces the parse.
>    Patch 2/2 (the KUnit test) is unchanged.
> 
> v2: https://lore.kernel.org/r/20260926012718.15675-1-jain.sm@gmail.com
> v1: https://lore.kernel.org/r/20260925102334.49693-1-jain.sm@gmail.com
> 
>   lib/parser.c | 17 +++++++----------
>   1 file changed, 7 insertions(+), 10 deletions(-)
> 
> diff --git a/lib/parser.c b/lib/parser.c
> index 62da0ac..5e3abf0 100644
> --- a/lib/parser.c
> +++ b/lib/parser.c
> @@ -137,22 +137,19 @@ 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;
>   
>   	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;
> +	ret = kstrtol(buf, base, &val);
> +	if (ret)
> +		return ret;
> +	if (val < (long)INT_MIN || val > (long)INT_MAX)
> +		return -ERANGE;
> +	*result = (int) val;

For consistency, there should be no space between the right
parenthesis in the cast and val.  This could probably be
fixed by the maintainer if this gets accepted.

					-Alex

> +	return 0;
>   }
>   
>   /**


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

* Re: [PATCH v3 1/2] lib: parser: reject out-of-range values in match_number()
  2026-10-05 15:10 ` [PATCH v3 1/2] lib: parser: reject out-of-range values in match_number() Alex Elder
@ 2026-10-06  4:42   ` shashank Jain
  0 siblings, 0 replies; 4+ messages in thread
From: shashank Jain @ 2026-10-06  4:42 UTC (permalink / raw)
  To: Alex Elder; +Cc: Andrew Morton, Alex Elder, David Gow, linux-kernel

On 10/5/26, Alex Elder wrote:
> For consistency, there should be no space between the right
> parenthesis in the cast and val.  This could probably be
> fixed by the maintainer if this gets accepted.

Thanks, Alex. That line was carried over unchanged from the old code,
but you're right that it should be "(int)val".

Andrew, if you apply this, could you drop the space when you do?
Otherwise I'm happy to send a v4 with just that change.

Thanks,
Shashank

On Mon, Oct 5, 2026 at 8:40 PM Alex Elder <elder@ieee.org> wrote:
>
> On 10/4/26 8:44 PM, Shashank Mohan Jain wrote:
> > 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.
> >
> > 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.
> >
> > Use kstrtol(), as the comment above simple_strtol() recommends.  It
> > returns -ERANGE for values that don't fit in a long, which on 32-bit
> > is the whole int range check, and the INT_MIN/INT_MAX check covers
> > 64-bit.  The substrings passed in come from match_one(), which ends a
> > %d, %o or %x argument where simple_strtol()/simple_strtoul() stops,
> > so kstrtol() sees the same characters and accepts the same values in
> > the int range as before.
> >
> > Fixes: 77dd3b0bd17a ("lib/parser.c: avoid overflow in match_number()")
> > Suggested-by: Alex Elder <elder@kernel.org>
> > Assisted-by: LLM
> > Signed-off-by: Shashank Mohan Jain <jain.sm@gmail.com>
> > ---
> > These patches were prepared with Claude Code (Anthropic), model Claude Opus 5.5
> > (claude-opus-5-5): the analysis, the Lean models used to find and check the bugs,
> > the fix, the tests, and the check of the in-tree callers for v3.
> >
> > Changes in v3:
> > - Use kstrtol() as suggested by Alex Elder, instead of open-coding the
> >    parse with _parse_integer(); the changelog says why it accepts the same
> >    values for the callers. The existing "int ret" and "long val"
> >    declarations are kept, so the diff only replaces the parse.
> >    Patch 2/2 (the KUnit test) is unchanged.
> >
> > v2: https://lore.kernel.org/r/20260926012718.15675-1-jain.sm@gmail.com
> > v1: https://lore.kernel.org/r/20260925102334.49693-1-jain.sm@gmail.com
> >
> >   lib/parser.c | 17 +++++++----------
> >   1 file changed, 7 insertions(+), 10 deletions(-)
> >
> > diff --git a/lib/parser.c b/lib/parser.c
> > index 62da0ac..5e3abf0 100644
> > --- a/lib/parser.c
> > +++ b/lib/parser.c
> > @@ -137,22 +137,19 @@ 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;
> >
> >       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;
> > +     ret = kstrtol(buf, base, &val);
> > +     if (ret)
> > +             return ret;
> > +     if (val < (long)INT_MIN || val > (long)INT_MAX)
> > +             return -ERANGE;
> > +     *result = (int) val;
>
> For consistency, there should be no space between the right
> parenthesis in the cast and val.  This could probably be
> fixed by the maintainer if this gets accepted.
>
>                                         -Alex
>
> > +     return 0;
> >   }
> >
> >   /**
>

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

end of thread, other threads:[~2026-10-06  4:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2026-10-05  1:44 [PATCH v3 1/2] lib: parser: reject out-of-range values in match_number() Shashank Mohan Jain
2026-10-05  1:44 ` [PATCH v3 2/2] lib/tests: add KUnit test for parser number helpers Shashank Mohan Jain
2026-10-05 15:10 ` [PATCH v3 1/2] lib: parser: reject out-of-range values in match_number() Alex Elder
2026-10-06  4:42   ` shashank Jain

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

all inboxes | Powered by JetHome®