Re: [PATCH v2 1/2] lib: parser: reject out-of-range values in match_number()
From: Alex Elder
Date: Fri Oct 02 2026 - 17:26:31 EST
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@xxxxxxxx> 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@xxxxxxxxx>
---
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@xxxxxxxxx
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;
}
/**