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

[Alex Elder <elder@ieee.org>]

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;
>   }
>   
>   /**