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

[Linus Torvalds]

On Thu, Sep 10, 2015 at 4:28 AM, Alexey Dobriyan <adobriyan@xxxxxxxxx> wrote:
>>
>> Maybe I misread the code, but that's what it looks like.
>
> It does exactly that (which I think is saner default).

No. You think *WRONG*.

Nobody in the history of the world has really cared about integer
parsing overflow. Really. It's just not an issue. It's not a security
issue (the possibly truncated number that is returned *could* have
been returned by just parsing that numebr in the first place), and
it's not a practical worry either.

Seriously. When was the last time you heard somebody complain about

   "I wrote the number 9223372036854775809, and the computer thought I
meant '1' - I'm really really upset"

Yeah, outside of mathematicians and bignum people, that complaint just
simply doesn't happen.

But the example I gave was *real*. Seriously. I didn't make some
theoretical argument. People really do write

   unsigned int x = -1;

and I just checked in the kernel - a quick grep seems to indicate that
we have around a hundred cases of that.

And that code isn't wrong.

YOUR CODE IS WRONG, AND REALITY SHOWS THAT YOUR DEFAULT IS CRAP.

Really. The whole "-1" thing is just a very common and convenient way
to say "all bits set" even for unsigned values.

I'd argue that it's an even *bigger* deal when parsing numbers in the
kernel, because in source code you could write it as

    unsigned long val = ~0ul;

if you are pedantic. You can't do anything similar when you have
something simple like a number parser rather than a real language
parser.

> If you have very strict rules as default, someone who wants to implement
> more relaxed parsing can easily do that.

BS.

The only reason for your interface was that it was simpler to use. You
broke that.

And you broke that for no good reason. As mentioned, the type size
overflow has never ever been a security issue, and simply _cannot_ be
one. And your incorrectly strict range check violates all
expectations, and all other numerical parsers. I just checked glibc,
and it is perfectly happy to use "%d" with an unsigned int, and it is
also perfectly happy to take MAX_UINT rather than MAX_INT.

In fact, you can happily feed scanf("%d") in user space something like

     -9223372036854775803

and it will return the integer "5". Nobody has ever cared. And that's
still a *better* end result than erroring out, because while the above
example is insane, examples like "-1" etc are not.

So your "default" is not actually safe. It breaks real cases, and
doesn't add any security.

It's broken.

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