Re: [PATCH v3 1/2] kernel.h: Update comment about simple_strto<foo>() functions

[Geert Uytterhoeven]

Hi Petr,

On Mon, Jul 8, 2019 at 3:02 PM Petr Mladek <pmladek@xxxxxxxx> wrote:
> On Thu 2019-07-04 14:55:31, Andy Shevchenko wrote:
> > There were discussions in the past about use cases for
> > simple_strto<foo>() functions and, in some rare cases,
> > they have a benefit over kstrto<foo>() ones.
> >
> > Update a comment to reduce confusion about special use cases.
> >
> > Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@xxxxxxxxx>
> > Signed-off-by: Andy Shevchenko <andriy.shevchenko@xxxxxxxxxxxxxxx>
> > ---
> > - update comment based on Geert's input
> >  include/linux/kernel.h | 17 ++++++++++++-----
> >  1 file changed, 12 insertions(+), 5 deletions(-)
> >
> > diff --git a/include/linux/kernel.h b/include/linux/kernel.h
> > index 0c9bc231107f..63663c44933d 100644
> > --- a/include/linux/kernel.h
> > +++ b/include/linux/kernel.h
> > @@ -332,8 +332,7 @@ int __must_check kstrtoll(const char *s, unsigned int base, long long *res);
> >   * @res: Where to write the result of the conversion on success.
> >   *
> >   * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
> > - * Used as a replacement for the obsolete simple_strtoull. Return code must
> > - * be checked.
> > + * Used as a replacement for the simple_strtoull. Return code must be checked.
> >  */
> >  static inline int __must_check kstrtoul(const char *s, unsigned int base, unsigned long *res)
> >  {
> > @@ -361,8 +360,7 @@ static inline int __must_check kstrtoul(const char *s, unsigned int base, unsign
> >   * @res: Where to write the result of the conversion on success.
> >   *
> >   * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
> > - * Used as a replacement for the obsolete simple_strtoull. Return code must
> > - * be checked.
> > + * Used as a replacement for the simple_strtoull. Return code must be checked.
> >   */
> >  static inline int __must_check kstrtol(const char *s, unsigned int base, long *res)
> >  {
> > @@ -438,7 +436,16 @@ static inline int __must_check kstrtos32_from_user(const char __user *s, size_t
> >       return kstrtoint_from_user(s, count, base, res);
> >  }
> >
> > -/* Obsolete, do not use.  Use kstrto<foo> instead */
> > +/*
> > + * Use kstrto<foo> instead.
> > + *
> > + * NOTE: The simple_strto<foo> does not check for overflow and,
> > + *    depending on the input, may give interesting results.
>
> I am a bit confused whether the interesting results are caused
> by the buffer overflow or if there is another reason.

Which buffer overflow?
> If it is because of the overflow, I would remove the 2nd line. I guess
> that anyone knows what a buffer overflow might cause.

AFAIK, the overflow is a numerical overflow.

The "interesting result" is that the function keeps parsing until it finds
a character that doesn't fit in the range of expected characters, according
to the specified numerical base, but further ignoring character class.
But that's really what you want, when you want to parse things like
10x50 or 10:50.

Gr{oetje,eeting}s,

                        Geert

-- 
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@xxxxxxxxxxxxxx

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
                                -- Linus Torvalds