linux-kernel - Re: [patch 27/95] scanf: fix type range overflow

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]

Date:	Fri, 11 Sep 2015 14:19:18 -0700
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Alexey Dobriyan <adobriyan@...il.com>
Cc:	Andrew Morton <akpm@...ux-foundation.org>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [patch 27/95] scanf: fix type range overflow

On Fri, Sep 11, 2015 at 1:52 PM, Alexey Dobriyan <adobriyan@...il.com> wrote:
>
>    Returning to parse_integer(), whole thing is a matter of general
>    robustness (no "be liberal in what you accept" rubbish) and
>    hopefully nicer interface which is pleasant to use (1 function
>    instead of 4, etc).

The thing is, that whole argument is totally invalidated by the fact
that you then convert existing users that have different semantics
than the ones you are introducing.

If this was a *new* interface with purely *new* users, then
"robustness" would be a possible argument for it.

But as it is, it is documented (and used) as a replacement interface.

Which means that you can't just make up BS about "it's more robust".
It has clearly different semantics, and those clearly different
semantics are

 (a) bad, as explained by my example of "unsigned int x = -1".

      I don't understand how you can just dismiss this very simple
case. It's not theoretical.

 (b) incompatible and leading to actual bugs, as exemplified by the
broken conversion.

The thing is, your very own conversion proved me right. That whole

    if (isdigit(*str))
        str += parse_integer(str ...)

would actually have been ok if it wasn't for the wrongheaded range
checking. The range checking directly led to bugs, because it
introduced a subtle error case that you clearly didn't think about.

Your arguments all are entirely irrelevant to the fundamental issue.

And then when I suggest a *sane* interface that doesn't have this
problem, your arguments are crap - again. Here:

> 5. The slight problem with your kitchen-sink proposal that C doesn't have
>   optional/named macro and function arguments neither it does have real
>   preprocessor. So people will have to specify INT_MIN/INT_MAX/...
>   every invocation even if they don't care or if type information by itself
>   is enough:

Bullshit. You clearly didn't even read my proposal. My proposal was
that the fundamental unit of conversion would be the *sane* one that
didn't check ranges at all. So if you don't care about a particular
range, you just do "parse_integer()".

And if you actually care about a particular range (which is almost
*never* the range of the type itself, but can be things like "the size
of an array" etc), you do "parse_integer_range()", and give the actual
range you care about.

So no. People would never ever specify INT_MIN/INT_MAX. Because if
that's the range they care about, then they clearly don't care about
the range at all.

Anyway, I'm not discussing this. You are clearly unwilling to just
admit that your patch-series was broken, and that assigning "-1" to an
unsigned entity is perfectly normal and common-place. As such, why
bother arguing?

                  Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/