From: Linus Torvalds <torvalds@linux-foundation.org>
To: Petr Mladek <pmladek@suse.com>
Cc: "Rasmus Villemoes" <linux@rasmusvillemoes.dk>,
"Kees Cook" <keescook@chromium.org>,
"Richard Weinberger" <richard@nod.at>,
linux-hardening@vger.kernel.org, netdev@vger.kernel.org,
linux-kernel@vger.kernel.org,
"Steven Rostedt" <rostedt@goodmis.org>,
"Sergey Senozhatsky" <senozhatsky@chromium.org>,
"Andy Shevchenko" <andriy.shevchenko@linux.intel.com>,
"David S. Miller" <davem@davemloft.net>,
"Eric Dumazet" <edumazet@google.com>,
"Jakub Kicinski" <kuba@kernel.org>,
"Paolo Abeni" <pabeni@redhat.com>,
"Miguel Ojeda" <ojeda@kernel.org>,
"Alex Gaynor" <alex.gaynor@gmail.com>,
"Wedson Almeida Filho" <wedsonaf@gmail.com>,
"Boqun Feng" <boqun.feng@gmail.com>,
"Gary Guo" <gary@garyguo.net>,
"Björn Roy Baron" <bjorn3_gh@protonmail.com>,
"Benno Lossin" <benno.lossin@proton.me>,
"Alexei Starovoitov" <ast@kernel.org>,
"Daniel Borkmann" <daniel@iogearbox.net>,
"Jesper Dangaard Brouer" <hawk@kernel.org>,
"John Fastabend" <john.fastabend@gmail.com>
Subject: Re: [RFC PATCH 0/1] Integer overflows while scanning for integers
Date: Fri, 9 Jun 2023 10:46:31 -0700 [thread overview]
Message-ID: <CAHk-=whW0W3YY4dHGbQoSagMnWEQbQ=Kgv48LJYN1OwVFz8qTQ@mail.gmail.com> (raw)
In-Reply-To: <ZINbQ3eyMB2OGfiN@alley>
On Fri, Jun 9, 2023 at 10:03 AM Petr Mladek <pmladek@suse.com> wrote:
>
> Added Linus into CC. Quick summary for him:
>
> 1. The original problem is best described in the cover letter, see
> https://lore.kernel.org/r/20230607223755.1610-1-richard@nod.at
Well, honestly, I'm not convinced this is a huge problem, but:
> > I can't figure out what POSIX actually says should or could happen with
> > sscanf("99999999999999", "%i", &x);
>
> It looks to me that it does not say anything about it. Even the latest
> IEEE Std 1003.1-2017 says just this:
We really don't need to language-lawyer POSIX. It's one of those
"let's follow it to minimize confusion" things, but no need to think
our internal library decisions need to actually care deeply.
For example, we did all the '%pXYZ" extensions on the printf() side,
which are very much *not* POSIX. They have been a huge success.
And even when it comes to the user space ABI, we've always prioritized
backwards compatibility over POSIX.
In some cases have actively disagreed with POSIX for being actively
wrong. For example, POSIX at one point thought that 'accept()',
'bind()' and friends should take a 'size_t' for the address length,
which was complete garbage.
It's "int", and I absolutely refused to follow broken specs. They
ended up fixing their completely broken spec to use 'socklen_t'
instead, which is still completely wrong (it's "int", and anything
else is wrong), but at least you can now be POSIX-compatible without
being broken.
In this case, I would suggest:
- absolutely do *NOT* add a WARNING() for this, because that just
allows user space to arbitrarily cause kernel warnings. Not ok.
- fail overflows by default
- to be able to deal with any compatibility issues, add a way to say
"don't fail overflows" in the format specs, maybe using '!'.
IOW, make
sscanf("999999999999", "%d", &i);
return 0 (because it couldn't parse a single field - go ahead and set
errno to ERANGE too if you care), but allow people to do
sscanf("999999999999", "%!d", &i);
which would return 1 and set 'i' to -727379969.
That makes us error out on overflow by default, but gives us an easy
way to say "in this case, I'll take the overflow value" if it turns
out that we have some situation where people gave bad input and it
"just worked".
There's a special case of "overflow" that we may need to deal with:
passing in "-1" as a way to set all bits to one in an unsigned value
is not necessarily uncommon.
So I suspect that even if the conversion is something like "%lu" (or
"%x"), which is technically meant for unsigned values, we need to
still allow negative values, and just say "it's not overflow, it's 2's
complement".
Linus
next prev parent reply other threads:[~2023-06-09 17:46 UTC|newest]
Thread overview: 15+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-06-07 22:37 [RFC PATCH 0/1] Integer overflows while scanning for integers Richard Weinberger
2023-06-07 22:37 ` [RFC PATCH 1/1] vsprintf: Warn on integer scanning overflows Richard Weinberger
2023-06-08 14:27 ` Andy Shevchenko
2023-06-08 16:14 ` Richard Weinberger
2023-06-08 16:23 ` Andy Shevchenko
2023-06-12 6:16 ` kernel test robot
2023-06-07 23:36 ` [RFC PATCH 0/1] Integer overflows while scanning for integers Kees Cook
2023-06-08 15:27 ` Petr Mladek
2023-06-08 16:12 ` Richard Weinberger
2023-06-08 16:19 ` Greg KH
2023-06-08 16:23 ` Richard Weinberger
2023-06-09 10:10 ` Rasmus Villemoes
2023-06-09 17:02 ` Petr Mladek
2023-06-09 17:46 ` Linus Torvalds [this message]
2023-06-10 19:31 ` David Laight
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAHk-=whW0W3YY4dHGbQoSagMnWEQbQ=Kgv48LJYN1OwVFz8qTQ@mail.gmail.com' \
--to=torvalds@linux-foundation.org \
--cc=alex.gaynor@gmail.com \
--cc=andriy.shevchenko@linux.intel.com \
--cc=ast@kernel.org \
--cc=benno.lossin@proton.me \
--cc=bjorn3_gh@protonmail.com \
--cc=boqun.feng@gmail.com \
--cc=daniel@iogearbox.net \
--cc=davem@davemloft.net \
--cc=edumazet@google.com \
--cc=gary@garyguo.net \
--cc=hawk@kernel.org \
--cc=john.fastabend@gmail.com \
--cc=keescook@chromium.org \
--cc=kuba@kernel.org \
--cc=linux-hardening@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux@rasmusvillemoes.dk \
--cc=netdev@vger.kernel.org \
--cc=ojeda@kernel.org \
--cc=pabeni@redhat.com \
--cc=pmladek@suse.com \
--cc=richard@nod.at \
--cc=rostedt@goodmis.org \
--cc=senozhatsky@chromium.org \
--cc=wedsonaf@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).