linux-kernel - Re: [PATCH v1] kernel.h: Update comment about simple

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] [day] [month] [year] [list]

Message-ID: <CANiq72nC7C4Hhc0Q2aP_oRqWGuY6pujvB5oYW7RvRjyG+V8+9w@mail.gmail.com>
Date:   Tue, 27 Feb 2018 16:21:25 +0100
From:   Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
To:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        linux-kernel <linux-kernel@...r.kernel.org>,
        Geert Uytterhoeven <geert@...ux-m68k.org>,
        Willy Tarreau <w@....eu>
Subject: Re: [PATCH v1] kernel.h: Update comment about simple_strto<foo>() functions

On Tue, Feb 27, 2018 at 12:40 PM, Andy Shevchenko
<andriy.shevchenko@...ux.intel.com> wrote:
> On Mon, 2018-02-26 at 23:31 +0100, Miguel Ojeda wrote:
>> On Mon, Feb 26, 2018 at 6:55 PM, Andy Shevchenko
>> <andriy.shevchenko@...ux.intel.com> wrote:
>> > There were discussions in the past about use cases for
>> > simple_strto<foo>() functions and in some rare cases they have a
>> > benefit
>> > on kstrto<foo>() ones.
>> >
>> > Update a comment to reduce confusing about special use cases.
>> >
>> > Suggested-by: Miguel Ojeda <miguel.ojeda.sandonis@...il.com>
>>
>> I am not sure we should just go back to the old ones, though.
>
> I didn't tell that we should.
> The niche of kstrto*() and simple_strto*() is different.

I meant that I am not sure that we should just re-allow the old ones
(i.e. I didn't mean that you meant to remove the kstrto*() ones :-).

>
>>  Maybe it
>> is better to create a new set of kstrto*_inplace() or some other name,
>> safer than the old ones and following kstrto*()'s interface regarding
>> returned errors, overflow checking, etc. There are two variations that
>> can be useful:
>>
>>   * A strict version taking a (start, end) range or (start, size) pair
>> which contains the number to be converted. If there is any problem
>> parsing it (e.g. invalid characters, extra characters, ...), fail.
>>
>>   * A less strict version taking an extra end pointer (or size
>> parameter) which is not allowed to be surpassed, and any non-digit
>> character means successful stop.
>
> Send a patch, we will discuss that for sure.
>
>> The old behavior (simple_*()) can still be achieved (almost) with the
>> second version with an "infinite" end pointer if one really needs it.
>
>> In any case, if you want to go forward with the old ones, we would
>> also have to change the comments inside lib/vsprintf.c and possibly
>> checkpatch :-)
>
> Feel free to amend.
>
> I actually didn't get your position here. You rather going to keep ugly
> code in your subsystem because of "official" comment than do it in more
> cleaner, but old fashion way.

I am not going to keep ugly code (why would you say so? It is not even
accepted yet); but I am not adding calls to deprecated functions
either. If those functions shouldn't be deprecated, fine; but *that*
is what should be discussed/changed. Adding new calls to functions
that were deprecated years ago (and currently are in HEAD) does not
help anybody.

>
> Btw, you can still weakly (based on power of base) detect an overflow by
> checking a returned pointer from simple_strto*().

Sure, but fixing that interface is precisely one of the reasons
simple_strto*() were deprecated to begin with.

Cheers,
Miguel

>
> --
> Andy Shevchenko <andriy.shevchenko@...ux.intel.com>
> Intel Finland Oy