linux-kernel - Re: Writing to a const pointer: is this supposed to happen?

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Sat, 4 Jul 2020 23:54:22 +0300
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Kars Mulder <kerneldev@...smulder.nl>
Cc:     David Laight <David.Laight@...lab.com>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Pavel Machek <pavel@....cz>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Kai-Heng Feng <kai.heng.feng@...onical.com>
Subject: Re: Writing to a const pointer: is this supposed to happen?

On Sat, Jul 4, 2020 at 11:32 PM Kars Mulder <kerneldev@...smulder.nl> wrote:
> On Saturday, July 04, 2020 16:39 CEST, Andy Shevchenko wrote:
> > > I've searched for a function that parses an int from a string and
> > > stores a pointer to the end; I can find some function simple_strtoul
> > > that matches this criterion, but it's documented as
> > >
> > >     "This function has caveats. Please use kstrtoul instead."
> > >
> > > ... and kstrtoul does not store a pointer to the end. The documentation
> > > of kstrtoul describes simple_strtoul as obsolete as well.
> >
> >
> >  Where? We need to fix that, because using simple_strto*() is fairly legal
> > in cases like this, but "has caveats".
>
> In lib/vsprintf.c, the comments before the function's implementation say:
>
>     /**
>      * simple_strtoul - convert a string to an unsigned long
>      * @cp: The start of the string
>      * @endp: A pointer to the end of the parsed string will be placed here
>      * @base: The number base to use
>      *
>      * This function has caveats. Please use kstrtoul instead.
>      */

This is correct.

> Many variants upon kstrtoul, such as kstrtoull, defined in lib/kstrtox.c,
> describe the simple_strtoull function as obsolete:
>
>     /**
>      * kstrtoull - convert a string to an unsigned long long
>      * [...]
>      *
>      * 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.
>      */

This and similar are not correct. 1/ They are not replacement per se
(because of different behaviour). 2/ They simple_strto*() are not
obsoleted.

Can you correct all places you found and make it consistent?

> I seem to have been slightly inaccurate about my claim that "kstrtoul"
> describes simple_strtoul as "obsolete" though, because in the specific
> case of kstrtoul, include/linux/kernel.h only says:
>
>     /**
>      * kstrtoul - convert a string to an unsigned long
>      * [...]
>      *
>      * Returns 0 on success, -ERANGE on overflow and -EINVAL on parsing error.
>      * Used as a replacement for the simple_strtoull. Return code must be checked.
>     */

> (Also, there may be a documentation error here: all kstrto* functions in
> kstrtox.c and kernel.h describe themselves as replacements of simple_strtoull.
> E.g. kstrtol and kstrtoul also describe themselves as replacements of
> simple_strtoull rather than as a replacements of simple_strtol and
> simple_strtoul respectively.)
>
> By the way, I found the documentation of the caveat somewhere in
> include/linux/kernel.h:

Yes, that's what has been added lately to clarify the usage of
simple_strto*() vs. kstrto*().

>     /*
>      * Use kstrto<foo> instead.
>      *
>      * NOTE: simple_strto<foo> does not check for the range overflow and,
>      *   depending on the input, may give interesting results.
>      *
>      * Use these functions if and only if you cannot use kstrto<foo>, because
>      * the conversion ends on the first non-digit character, which may be far
>      * beyond the supported range. It might be useful to parse the strings like
>      * 10x50 or 12:21 without altering original string or temporary buffer in use.
>      * Keep in mind above caveat.
>      */


-- 
With Best Regards,
Andy Shevchenko