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

[shashank Jain]

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@xxxxxxxx> 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@xxxxxxxxxx>
> > 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 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@xxxxxxxxx
> > v1: https://lore.kernel.org/r/20260925102334.49693-1-jain.sm@xxxxxxxxx
> >
> >   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;
> >   }
> >
> >   /**
>