linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Kees Cook <keescook@chromium.org>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>,
	Ard Biesheuvel <ard.biesheuvel@linaro.org>,
	Arnd Bergmann <arnd@arndb.de>, Jean Delvare <jdelvare@suse.de>
Subject: Re: [GIT PULL] gcc-plugins updates for v4.13-rc1
Date: Wed, 5 Jul 2017 14:49:28 -0700	[thread overview]
Message-ID: <CAGXu5jLwSFp6yDbcjHmnYTTY0iM=9CQVdHnz0tYtZ++zOEvzdQ@mail.gmail.com> (raw)
In-Reply-To: <CA+55aFwku_yentgkzJ4To4oeK0HvTH6yyGVrhks6NBFK6+gS9Q@mail.gmail.com>

On Wed, Jul 5, 2017 at 12:07 PM, Linus Torvalds
<torvalds@linux-foundation.org> wrote:
> Hmm. Completely unrelated comment, and this may not be a gcc 'plugin'
> issue as much as a more general gcc question, but I suspect a plugin
> could do it.
>
> For the kernel, we already really ignore some of the more idiotic C
> standard rules that introduce pointless undefined behavior: things
> like the strict aliasing rules are just insane, and the "overflow is
> udnefined" is bad too. So we use
>
>   -fno-strict-aliasing
>   -fno-strict-overflow
>   -fno-delete-null-pointer-checks
>
> to basically say "those optimizations are fundamentally stupid and
> wrong, and only encourage compilers to generate random code that
> doesn't actually match the source code".
>
> And I suspect one other undefined behavior is the one we _try_ to warn
> about, but where the compiler is not always good enough to give valid
> warnings - uninitialized automatic variables.
>
> Maybe we could have gcc just always initialize variables to zero. Not
> just static ones, but the automatic variables too. And maybe it
> wouldn't generate much extra code, since gcc will see the real
> initialization, and the extra hardening against random behavior will
> just go away - so this might be one of those cheap things where we
> just avoid undefined behavior and avoid leaking old stack contents.
>
> Yes, yes, you'd still have the uninitialized variable warning, but
> that doesn't catch the case where you pass a structure pointer to a
> helper that is *supposed* to fill it in, but misses a field or just
> misses padding.
>
> And maybe I'm wrong, and maybe it would generate a lot of really bad
> extra zeroing and wouldn't be acceptable for most people, but I
> *think* this might be one of those things where we might get some
> extra belt and suspenders kind of hardening basically for free..
>
> Comments?

It is, unfortunately, not free. :( There has been a lot of academic
research[1] into finding ways to minimize the impact, but given some
of the Linux maintainers refusing even zeroing of APIs that pass
stack-based structures[2]. Another thing that has been worked on is
porting the stackleak gcc plugin from grsecurity to upstream[3]. This
does effective clearing of the stack, but it takes a more holistic
approach (and for added fun, it does alloca probes too). Like some of
the more comprehensive academic attempts, it sees about a 4% hit (but
it's doing more...)

I'd love to get the stackleak plugin into upstream (and the work is
on-going), but having something try a "lighter" form of this in a gcc
plugin would be interesting to experiment with.

-Kees

[1] e.g. https://www.internetsociety.org/sites/default/files/ndss2017_09-2_Lu_paper.pdf
performs only uninitialized on-stack pointer zeroing, and
http://www.cs.vu.nl/~giuffrida/papers/safeinit-ndss-2017.pdf shows <5%
performance hit with optimization for initializing everything
[2] https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git/commit/?h=getsockname&id=a4467f966f0c70fd232388c05798a84276eef1ef
[3] http://openwall.com/lists/kernel-hardening/2017/06/09/14

-- 
Kees Cook
Pixel Security

      parent reply	other threads:[~2017-07-05 21:49 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-07-05  5:05 [GIT PULL] gcc-plugins updates for v4.13-rc1 Kees Cook
2017-07-05 19:07 ` Linus Torvalds
2017-07-05 20:40   ` Ard Biesheuvel
2017-07-05 21:35     ` Linus Torvalds
2017-07-05 21:48       ` Arnd Bergmann
2017-07-05 21:52         ` Kees Cook
2017-07-05 21:56         ` Linus Torvalds
2017-07-05 22:27           ` Ard Biesheuvel
2017-07-05 22:39             ` Linus Torvalds
2017-07-05 22:41           ` Andrey Ryabinin
2017-07-05 21:12   ` Arnd Bergmann
2017-07-05 21:49   ` Kees Cook [this message]

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='CAGXu5jLwSFp6yDbcjHmnYTTY0iM=9CQVdHnz0tYtZ++zOEvzdQ@mail.gmail.com' \
    --to=keescook@chromium.org \
    --cc=ard.biesheuvel@linaro.org \
    --cc=arnd@arndb.de \
    --cc=jdelvare@suse.de \
    --cc=linux-kernel@vger.kernel.org \
    --cc=torvalds@linux-foundation.org \
    /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 NNTP newsgroup(s).