The Linux Kernel Mailing List
 help / color / mirror / Atom feed
* [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number()
@ 2026-09-26  1:27 Shashank Mohan Jain
  2026-09-26  1:27 ` [PATCH v2 2/2] lib/tests: add KUnit test for parser number helpers Shashank Mohan Jain
  2026-10-01 21:37 ` [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number() Alex Elder
  0 siblings, 2 replies; 7+ messages in thread
From: Shashank Mohan Jain @ 2026-09-26  1:27 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.

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 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 fixes and the tests. I reviewed them and take responsibility for them. The
trailer only says "Assisted-by: LLM", as Documentation/process/coding-assistants.rst
requires since commit 816d9992d9ed ("coding-assistants: simplify attribution").

Changes in v2:
- Resent with my full name; no code changes.

v1: https://lore.kernel.org/r/20260925102334.49693-1-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


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

* [PATCH v2 2/2] lib/tests: add KUnit test for parser number helpers
  2026-09-26  1:27 [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number() Shashank Mohan Jain
@ 2026-09-26  1:27 ` Shashank Mohan Jain
  2026-10-01 21:37 ` [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number() Alex Elder
  1 sibling, 0 replies; 7+ messages in thread
From: Shashank Mohan Jain @ 2026-09-26  1:27 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>
---
 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 related	[flat|nested] 7+ messages in thread

* Re: [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number()
  2026-09-26  1:27 [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number() Shashank Mohan Jain
  2026-09-26  1:27 ` [PATCH v2 2/2] lib/tests: add KUnit test for parser number helpers Shashank Mohan Jain
@ 2026-10-01 21:37 ` Alex Elder
  2026-10-02  1:34   ` shashank Jain
  1 sibling, 1 reply; 7+ messages in thread
From: Alex Elder @ 2026-10-01 21:37 UTC (permalink / raw)
  To: Shashank Mohan Jain, Andrew Morton; +Cc: Alex Elder, David Gow, linux-kernel

On 9/25/26 8:27 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:

The result is still placed in memory referred to by an
integer pointer, right?  Not a long long?

It might be better to define a new function that expects to
produce (up to) 64 bit result values (signed, or maybe not),
rather than changing this function.

Or... maybe there's a simpler way to catch a too-long
input string.
> 
>   - simple_strtoull() saturates to ULLONG_MAX on overflow and

This matters because match_number() calls simple_strtol(),
which is implemented using simple_strtoul(), and that is
implemented using simple_strtoull().  So if a very long
numeric string is passed to match_number(), it will be
misinterpreted as something that overflows 32 bits,
which will end up having a misleading (but essentially
valid) int value.

Is that correct?

At this point I'm just trying to make sure I understand what
your intent is, because I'm unsure this is the right way to
achieve what you're after.

I guess I'm stuck on match_number() expecting to produce
an int result, and your discussion talks a lot about 64-bit
values.

					-Alex


>     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 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 fixes and the tests. I reviewed them and take responsibility for them. The
> trailer only says "Assisted-by: LLM", as Documentation/process/coding-assistants.rst
> requires since commit 816d9992d9ed ("coding-assistants: simplify attribution").
> 
> Changes in v2:
> - Resent with my full name; no code changes.
> 
> v1: https://lore.kernel.org/r/20260925102334.49693-1-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;
>   }
>   
>   /**


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

* Re: [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number()
  2026-10-01 21:37 ` [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number() Alex Elder
@ 2026-10-02  1:34   ` shashank Jain
  2026-10-02 21:21     ` Alex Elder
  0 siblings, 1 reply; 7+ messages in thread
From: shashank Jain @ 2026-10-02  1:34 UTC (permalink / raw)
  To: Alex Elder; +Cc: Andrew Morton, Alex Elder, David Gow, linux-kernel

Hi Alex,

Thanks for looking at this.

> The result is still placed in memory referred to by an
> integer pointer, right?  Not a long long?

Yes. The interface doesn't change: match_int(), match_octal() and
match_hex() still return an int through an int *, and the accepted
range is still INT_MIN..INT_MAX. The unsigned long long is only used
while parsing, so that a value outside the int range can be detected
and rejected with -ERANGE instead of being stored as a different int.
By the time *result is written, the value is known to fit.

> It might be better to define a new function that expects to
> produce (up to) 64 bit result values (signed, or maybe not),
> rather than changing this function.

For callers that need 64-bit values there is already match_u64()
(match_u64int() with kstrtoull()), so I don't think a new function is
needed. This patch is only about the int helpers accepting
out-of-range input.

> Or... maybe there's a simpler way to catch a too-long
> input string.

I looked at that, but the length of the string doesn't decide whether
the value fits: "4294967295" is only 10 characters and doesn't fit in
an int, while leading zeros make a long string that does
("00000000000000000001"). What is needed is the overflow indication,
which simple_strtoull() drops (there is a FIXME at that spot in
simple_strntoull()). _parse_integer() returns it, so the patch parses
the same way simple_strtol() does, keeps that flag, and compares the
magnitude with the int range.

> This matters because match_number() calls simple_strtol(),
> which is implemented using simple_strtoul(), and that is
> implemented using simple_strtoull().  So if a very long
> numeric string is passed to match_number(), it will be
> misinterpreted as something that overflows 32 bits,
> which will end up having a misleading (but essentially
> valid) int value.
>
> Is that correct?

Yes, that is the problem, in two forms:

- On 64-bit, a value of 2^64 or more saturates to ULLONG_MAX in
  simple_strtoull(), and simple_strtol() turns that into -1L, which
  passes the INT_MIN/INT_MAX check. Values from 2^64 - 2^31 to
  2^64 - 1 wrap to small negative longs the same way. So match_int()
  returns 0 with *result = -1 for "18446744073709551615" or
  "99999999999999999999".

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

In both cases the caller gets success and a different number from the
one that was written, for example in a mount option. With the patch
these return -ERANGE. Values that fit in an int are accepted exactly
as before, with the same result and the same syntax.

Sorry the changelog made it sound as if the result were becoming
64-bit; only the intermediate magnitude is. I can reword that in a v3
if it helps.

Thanks,
Shashank

On Fri, Oct 2, 2026 at 3:07 AM Alex Elder <elder@ieee.org> wrote:
>
> On 9/25/26 8:27 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:
>
> The result is still placed in memory referred to by an
> integer pointer, right?  Not a long long?
>
> It might be better to define a new function that expects to
> produce (up to) 64 bit result values (signed, or maybe not),
> rather than changing this function.
>
> Or... maybe there's a simpler way to catch a too-long
> input string.
> >
> >   - simple_strtoull() saturates to ULLONG_MAX on overflow and
>
> This matters because match_number() calls simple_strtol(),
> which is implemented using simple_strtoul(), and that is
> implemented using simple_strtoull().  So if a very long
> numeric string is passed to match_number(), it will be
> misinterpreted as something that overflows 32 bits,
> which will end up having a misleading (but essentially
> valid) int value.
>
> Is that correct?
>
> At this point I'm just trying to make sure I understand what
> your intent is, because I'm unsure this is the right way to
> achieve what you're after.
>
> I guess I'm stuck on match_number() expecting to produce
> an int result, and your discussion talks a lot about 64-bit
> values.
>
>                                         -Alex
>
>
> >     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 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 fixes and the tests. I reviewed them and take responsibility for them. The
> > trailer only says "Assisted-by: LLM", as Documentation/process/coding-assistants.rst
> > requires since commit 816d9992d9ed ("coding-assistants: simplify attribution").
> >
> > Changes in v2:
> > - Resent with my full name; no code changes.
> >
> > v1: https://lore.kernel.org/r/20260925102334.49693-1-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;
> >   }
> >
> >   /**
>

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

* Re: [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number()
  2026-10-02  1:34   ` shashank Jain
@ 2026-10-02 21:21     ` Alex Elder
  2026-10-03  2:12       ` shashank Jain
  0 siblings, 1 reply; 7+ messages in thread
From: Alex Elder @ 2026-10-02 21:21 UTC (permalink / raw)
  To: shashank Jain; +Cc: Andrew Morton, Alex Elder, David Gow, linux-kernel

On 10/1/26 8:34 PM, shashank Jain wrote:
> Hi Alex,
> 
> Thanks for looking at this.
> 
>> The result is still placed in memory referred to by an
>> integer pointer, right?  Not a long long?
> 
> Yes. The interface doesn't change: match_int(), match_octal() and
> match_hex() still return an int through an int *, and the accepted
> range is still INT_MIN..INT_MAX. The unsigned long long is only used
> while parsing, so that a value outside the int range can be detected
> and rejected with -ERANGE instead of being stored as a different int.
> By the time *result is written, the value is known to fit.

So your goal is simply to return an int as before, but make
the function robustly handle all possible inputs.  It should
return a valid value in *result when the input substring
contains a value INT_MIN..INT_MAX inclusive, and return an
error otherwise.

Note that the comment above simple_strtol() says:

   This function has caveats. Please use kstrtol instead.

I started reviewing what exactly kstrtol() would do, to
see if using that might be simpler.  It looks to me like
it does some of the same things you do.

I'm haven't run the unit test you provided, and I tried
working through what would happen with kstrtol(), but
gave up because it was taking too long.

Would this work instead?

static int match_number(substring_t *s, int *result, int base)
{
     char buf[NUMBER_BUF_LEN];
     long res;
     int res;

     if (match_strlcpy(buf, s, NUMBER_BUF_LEN) >= NUMBER_BUF_LEN)
         return -ERANGE;

     ret = kstrtol(buf, base, &res);
     if (ret)
         return ret;

     if (res < (long)INT_MIN || res > (long)INT_MAX)
         return -ERANGE;

     *result = (int)res;

     return 0;
}

					-Alex

> 
>> It might be better to define a new function that expects to
>> produce (up to) 64 bit result values (signed, or maybe not),
>> rather than changing this function.
> 
> For callers that need 64-bit values there is already match_u64()
> (match_u64int() with kstrtoull()), so I don't think a new function is
> needed. This patch is only about the int helpers accepting
> out-of-range input.
> 
>> Or... maybe there's a simpler way to catch a too-long
>> input string.
> 
> I looked at that, but the length of the string doesn't decide whether
> the value fits: "4294967295" is only 10 characters and doesn't fit in
> an int, while leading zeros make a long string that does
> ("00000000000000000001"). What is needed is the overflow indication,
> which simple_strtoull() drops (there is a FIXME at that spot in
> simple_strntoull()). _parse_integer() returns it, so the patch parses
> the same way simple_strtol() does, keeps that flag, and compares the
> magnitude with the int range.
> 
>> This matters because match_number() calls simple_strtol(),
>> which is implemented using simple_strtoul(), and that is
>> implemented using simple_strtoull().  So if a very long
>> numeric string is passed to match_number(), it will be
>> misinterpreted as something that overflows 32 bits,
>> which will end up having a misleading (but essentially
>> valid) int value.
>>
>> Is that correct?
> 
> Yes, that is the problem, in two forms:
> 
> - On 64-bit, a value of 2^64 or more saturates to ULLONG_MAX in
>    simple_strtoull(), and simple_strtol() turns that into -1L, which
>    passes the INT_MIN/INT_MAX check. Values from 2^64 - 2^31 to
>    2^64 - 1 wrap to small negative longs the same way. So match_int()
>    returns 0 with *result = -1 for "18446744073709551615" or
>    "99999999999999999999".
> 
> - On 32-bit, long is as wide as int, so the range check can never
>    fail: "4294967295" also gives -1, and "2147483648" gives INT_MIN.
> 
> In both cases the caller gets success and a different number from the
> one that was written, for example in a mount option. With the patch
> these return -ERANGE. Values that fit in an int are accepted exactly
> as before, with the same result and the same syntax.
> 
> Sorry the changelog made it sound as if the result were becoming
> 64-bit; only the intermediate magnitude is. I can reword that in a v3
> if it helps.
> 
> Thanks,
> Shashank
> 
> On Fri, Oct 2, 2026 at 3:07 AM Alex Elder <elder@ieee.org> wrote:
>>
>> On 9/25/26 8:27 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:
>>
>> The result is still placed in memory referred to by an
>> integer pointer, right?  Not a long long?
>>
>> It might be better to define a new function that expects to
>> produce (up to) 64 bit result values (signed, or maybe not),
>> rather than changing this function.
>>
>> Or... maybe there's a simpler way to catch a too-long
>> input string.
>>>
>>>    - simple_strtoull() saturates to ULLONG_MAX on overflow and
>>
>> This matters because match_number() calls simple_strtol(),
>> which is implemented using simple_strtoul(), and that is
>> implemented using simple_strtoull().  So if a very long
>> numeric string is passed to match_number(), it will be
>> misinterpreted as something that overflows 32 bits,
>> which will end up having a misleading (but essentially
>> valid) int value.
>>
>> Is that correct?
>>
>> At this point I'm just trying to make sure I understand what
>> your intent is, because I'm unsure this is the right way to
>> achieve what you're after.
>>
>> I guess I'm stuck on match_number() expecting to produce
>> an int result, and your discussion talks a lot about 64-bit
>> values.
>>
>>                                          -Alex
>>
>>
>>>      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 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 fixes and the tests. I reviewed them and take responsibility for them. The
>>> trailer only says "Assisted-by: LLM", as Documentation/process/coding-assistants.rst
>>> requires since commit 816d9992d9ed ("coding-assistants: simplify attribution").
>>>
>>> Changes in v2:
>>> - Resent with my full name; no code changes.
>>>
>>> v1: https://lore.kernel.org/r/20260925102334.49693-1-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;
>>>    }
>>>
>>>    /**
>>


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

* Re: [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number()
  2026-10-02 21:21     ` Alex Elder
@ 2026-10-03  2:12       ` shashank Jain
  2026-10-05  0:46         ` Alex Elder
  0 siblings, 1 reply; 7+ messages in thread
From: shashank Jain @ 2026-10-03  2:12 UTC (permalink / raw)
  To: Alex Elder; +Cc: Andrew Morton, Alex Elder, David Gow, linux-kernel

Hi Alex,

> So your goal is simply to return an int as before, but make
> the function robustly handle all possible inputs.  It should
> return a valid value in *result when the input substring
> contains a value INT_MIN..INT_MAX inclusive, and return an
> error otherwise.

Yes, exactly.

> Would this work instead?

Yes, and it is simpler. I checked it against the callers and the
test:

- For the callers, the substring comes from match_one(), which ends a
  %d, %o or %x argument where simple_strtol()/simple_strtoul() stops
  parsing. So kstrtol() sees the same characters, and the only
  difference is that it rejects values that don't fit. I went through
  the in-tree match_int(), match_octal() and match_hex() callers I
  could find, and they all use %d, %o or %x tokens, none %s.

- It also covers 32-bit: there long is the same width as int, and
  kstrtol() itself returns -ERANGE for anything outside the long range,
  so the INT_MIN/INT_MAX check is only needed on 64-bit.

- The cases in the KUnit test in patch 2/2 give the same results with
  kstrtol(), so the test doesn't need to change.

kstrtol() also accepts a leading '+' and a trailing newline, but a
%d/%o/%x argument from match_token() never contains those.

I'll send a v3 of patch 1/2 with your version (with "int ret; long res;"
for the two declarations) and add a Suggested-by: for you, if that's OK.

Thanks,
Shashank

On Sat, Oct 3, 2026 at 2:51 AM Alex Elder <elder@ieee.org> wrote:
>
> On 10/1/26 8:34 PM, shashank Jain wrote:
> > Hi Alex,
> >
> > Thanks for looking at this.
> >
> >> The result is still placed in memory referred to by an
> >> integer pointer, right?  Not a long long?
> >
> > Yes. The interface doesn't change: match_int(), match_octal() and
> > match_hex() still return an int through an int *, and the accepted
> > range is still INT_MIN..INT_MAX. The unsigned long long is only used
> > while parsing, so that a value outside the int range can be detected
> > and rejected with -ERANGE instead of being stored as a different int.
> > By the time *result is written, the value is known to fit.
>
> So your goal is simply to return an int as before, but make
> the function robustly handle all possible inputs.  It should
> return a valid value in *result when the input substring
> contains a value INT_MIN..INT_MAX inclusive, and return an
> error otherwise.
>
> Note that the comment above simple_strtol() says:
>
>    This function has caveats. Please use kstrtol instead.
>
> I started reviewing what exactly kstrtol() would do, to
> see if using that might be simpler.  It looks to me like
> it does some of the same things you do.
>
> I'm haven't run the unit test you provided, and I tried
> working through what would happen with kstrtol(), but
> gave up because it was taking too long.
>
> Would this work instead?
>
> static int match_number(substring_t *s, int *result, int base)
> {
>      char buf[NUMBER_BUF_LEN];
>      long res;
>      int res;
>
>      if (match_strlcpy(buf, s, NUMBER_BUF_LEN) >= NUMBER_BUF_LEN)
>          return -ERANGE;
>
>      ret = kstrtol(buf, base, &res);
>      if (ret)
>          return ret;
>
>      if (res < (long)INT_MIN || res > (long)INT_MAX)
>          return -ERANGE;
>
>      *result = (int)res;
>
>      return 0;
> }
>
>                                         -Alex
>
> >
> >> It might be better to define a new function that expects to
> >> produce (up to) 64 bit result values (signed, or maybe not),
> >> rather than changing this function.
> >
> > For callers that need 64-bit values there is already match_u64()
> > (match_u64int() with kstrtoull()), so I don't think a new function is
> > needed. This patch is only about the int helpers accepting
> > out-of-range input.
> >
> >> Or... maybe there's a simpler way to catch a too-long
> >> input string.
> >
> > I looked at that, but the length of the string doesn't decide whether
> > the value fits: "4294967295" is only 10 characters and doesn't fit in
> > an int, while leading zeros make a long string that does
> > ("00000000000000000001"). What is needed is the overflow indication,
> > which simple_strtoull() drops (there is a FIXME at that spot in
> > simple_strntoull()). _parse_integer() returns it, so the patch parses
> > the same way simple_strtol() does, keeps that flag, and compares the
> > magnitude with the int range.
> >
> >> This matters because match_number() calls simple_strtol(),
> >> which is implemented using simple_strtoul(), and that is
> >> implemented using simple_strtoull().  So if a very long
> >> numeric string is passed to match_number(), it will be
> >> misinterpreted as something that overflows 32 bits,
> >> which will end up having a misleading (but essentially
> >> valid) int value.
> >>
> >> Is that correct?
> >
> > Yes, that is the problem, in two forms:
> >
> > - On 64-bit, a value of 2^64 or more saturates to ULLONG_MAX in
> >    simple_strtoull(), and simple_strtol() turns that into -1L, which
> >    passes the INT_MIN/INT_MAX check. Values from 2^64 - 2^31 to
> >    2^64 - 1 wrap to small negative longs the same way. So match_int()
> >    returns 0 with *result = -1 for "18446744073709551615" or
> >    "99999999999999999999".
> >
> > - On 32-bit, long is as wide as int, so the range check can never
> >    fail: "4294967295" also gives -1, and "2147483648" gives INT_MIN.
> >
> > In both cases the caller gets success and a different number from the
> > one that was written, for example in a mount option. With the patch
> > these return -ERANGE. Values that fit in an int are accepted exactly
> > as before, with the same result and the same syntax.
> >
> > Sorry the changelog made it sound as if the result were becoming
> > 64-bit; only the intermediate magnitude is. I can reword that in a v3
> > if it helps.
> >
> > Thanks,
> > Shashank
> >
> > On Fri, Oct 2, 2026 at 3:07 AM Alex Elder <elder@ieee.org> wrote:
> >>
> >> On 9/25/26 8:27 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:
> >>
> >> The result is still placed in memory referred to by an
> >> integer pointer, right?  Not a long long?
> >>
> >> It might be better to define a new function that expects to
> >> produce (up to) 64 bit result values (signed, or maybe not),
> >> rather than changing this function.
> >>
> >> Or... maybe there's a simpler way to catch a too-long
> >> input string.
> >>>
> >>>    - simple_strtoull() saturates to ULLONG_MAX on overflow and
> >>
> >> This matters because match_number() calls simple_strtol(),
> >> which is implemented using simple_strtoul(), and that is
> >> implemented using simple_strtoull().  So if a very long
> >> numeric string is passed to match_number(), it will be
> >> misinterpreted as something that overflows 32 bits,
> >> which will end up having a misleading (but essentially
> >> valid) int value.
> >>
> >> Is that correct?
> >>
> >> At this point I'm just trying to make sure I understand what
> >> your intent is, because I'm unsure this is the right way to
> >> achieve what you're after.
> >>
> >> I guess I'm stuck on match_number() expecting to produce
> >> an int result, and your discussion talks a lot about 64-bit
> >> values.
> >>
> >>                                          -Alex
> >>
> >>
> >>>      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 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 fixes and the tests. I reviewed them and take responsibility for them. The
> >>> trailer only says "Assisted-by: LLM", as Documentation/process/coding-assistants.rst
> >>> requires since commit 816d9992d9ed ("coding-assistants: simplify attribution").
> >>>
> >>> Changes in v2:
> >>> - Resent with my full name; no code changes.
> >>>
> >>> v1: https://lore.kernel.org/r/20260925102334.49693-1-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;
> >>>    }
> >>>
> >>>    /**
> >>
>

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

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

On 10/2/26 9:12 PM, shashank Jain wrote:
> I'll send a v3 of patch 1/2 with your version (with "int ret; long res;"
> for the two declarations) and add a Suggested-by: for you, if that's OK.

Fine with me.  I'm glad I could help.	-Alex

Suggested-by: Alex Elder <elder@riscstar.com>

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

end of thread, other threads:[~2026-10-05  0:46 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-09-26  1:27 [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number() Shashank Mohan Jain
2026-09-26  1:27 ` [PATCH v2 2/2] lib/tests: add KUnit test for parser number helpers Shashank Mohan Jain
2026-10-01 21:37 ` [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number() Alex Elder
2026-10-02  1:34   ` shashank Jain
2026-10-02 21:21     ` Alex Elder
2026-10-03  2:12       ` shashank Jain
2026-10-05  0:46         ` Alex Elder

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