[GIT PULL] meminit fix for v5.3-rc2

[GIT PULL] meminit fix for v5.3-rc2

[Kees Cook]

Hi Linus,

Please pull this meminit fix for v5.3-rc2. This is late in the -rc2
window because I got confused about whether I or akpm was taking this
patch. I noticed it still wasn't in -mm, so here it is. It's a small
Kconfig change that fixes a bunch of build warnings under KASAN and the
gcc-plugin-based stack auto-initialization features (which are arguably
redundant, so better to let KASAN control this).

Thanks!

-Kees

The following changes since commit cd6c84d8f0cdc911df435bb075ba22ce3c605b07:

  Linux 5.2-rc2 (2019-05-26 16:49:19 -0700)

are available in the Git repository at:

  https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git tags/meminit-v5.3-rc2

for you to fetch changes up to 173e6ee21e2b3f477f07548a79c43b8d9cfbb37d:

  structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK (2019-07-25 16:16:12 -0700)

----------------------------------------------------------------
meminit fix

- Disable gcc-based stack variable auto-init under KASAN (Arnd Bergmann)

----------------------------------------------------------------
Arnd Bergmann (1):
      structleak: disable STRUCTLEAK_BYREF in combination with KASAN_STACK

 security/Kconfig.hardening | 7 +++++++
 1 file changed, 7 insertions(+)

-- 
Kees Cook

Re: [GIT PULL] meminit fix for v5.3-rc2

[Linus Torvalds]

On Sun, Jul 28, 2019 at 12:21 PM Kees Cook <keescook@chromium.org> wrote:
>
> Please pull this meminit fix for v5.3-rc2.

Side noe: I find "meminit" a confusing description for the structleak
thing. When I hear it, it sounds like some generic memory
initialization thing in the VM layer (which we obviously do also
have), not the stack variable initialization.

Also, have you guys talked to gcc people about just making it a real
feature, like I think it is for clang? In particular, I still suspect
that we could/should  just make zero-filling the *default* in the long
run, and say "our C standard is that local variables are initialized
to zero, exactly the same way static variables are".

I know you posted some numbers somewhere (well, I'm pretty sure you
did) and the full stack initialization really was pretty cheap,
wasn't it?

               Linus

Re: [GIT PULL] meminit fix for v5.3-rc2

[pr-tracker-bot]

The pull request you sent on Sun, 28 Jul 2019 12:21:34 -0700:

> https://git.kernel.org/pub/scm/linux/kernel/git/kees/linux.git tags/meminit-v5.3-rc2

has been merged into torvalds/linux.git:
https://git.kernel.org/torvalds/c/c622fc5f54cb0c7ea2e6fedba27ba533b97657d8

Thank you!

-- 
Deet-doot-dot, I am a bot.
https://korg.wiki.kernel.org/userdoc/prtracker

Re: [GIT PULL] meminit fix for v5.3-rc2

[Linus Torvalds]

On Sun, Jul 28, 2019 at 12:43 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> Side note: I find "meminit" a confusing description for the structleak
> thing.

Not that 'structleak' is all that much better, because it's not
necessarily just about structs either.

'stack variable initialization' is too long. I dunno. But it's more
about "variables" (and in the case of kmalloc etc - perhaps
'allocations') than "memory", I feel.

The point is that in the kernel we do memory management and memory
initialization, and that's something very different.

                     Linus

Re: [GIT PULL] meminit fix for v5.3-rc2

[Kees Cook]

On Sun, Jul 28, 2019 at 12:43:15PM -0700, Linus Torvalds wrote:
> On Sun, Jul 28, 2019 at 12:21 PM Kees Cook <keescook@chromium.org> wrote:
> >
> > Please pull this meminit fix for v5.3-rc2.
> 
> Side noe: I find "meminit" a confusing description for the structleak
> thing. When I hear it, it sounds like some generic memory
> initialization thing in the VM layer (which we obviously do also
> have), not the stack variable initialization.

I will find a better name. :) We dreamed up "meminit" as finding a name
for the umbrella of both stack and heap auto-initialization. But I
agree, it's confusing.

> Also, have you guys talked to gcc people about just making it a real
> feature, like I think it is for clang? In particular, I still suspect
> that we could/should  just make zero-filling the *default* in the long
> run, and say "our C standard is that local variables are initialized
> to zero, exactly the same way static variables are".

Yes, this is on the list for discussion at Plumber's. Having gcc do
auto-init is the first part. Convincing Clang that _zero_ init isn't
a language-breaking change is the second part. :P That's been a whole
other issue.

> I know you posted some numbers somewhere (well, I'm pretty sure you
> did) and the full stack initialization really was pretty cheap,
> wasn't it?

Yes, Clang's initialization (which is 0xAA not 0x00 in most cases) is
cheap. There are rumors(?) of some pathological workloads, though. I
haven't seen real numbers for that though.

I'll try to find the Clang numbers (maybe Alexander has them?) but I
remember it being the same as (or maybe better than) the gcc-plugin
version, which I measured here:

https://git.kernel.org/linus/81a56f6dcd20

-- 
Kees Cook

Re: [GIT PULL] meminit fix for v5.3-rc2

[Alexander Potapenko]

Hi Kees, Linus,

On Mon, Jul 29, 2019 at 12:16 AM Kees Cook <keescook@chromium.org> wrote:
>
> On Sun, Jul 28, 2019 at 12:43:15PM -0700, Linus Torvalds wrote:
> > On Sun, Jul 28, 2019 at 12:21 PM Kees Cook <keescook@chromium.org> wrote:
> > >
> > > Please pull this meminit fix for v5.3-rc2.
> >
> > Side noe: I find "meminit" a confusing description for the structleak
> > thing. When I hear it, it sounds like some generic memory
> > initialization thing in the VM layer (which we obviously do also
> > have), not the stack variable initialization.
>
> I will find a better name. :) We dreamed up "meminit" as finding a name
> for the umbrella of both stack and heap auto-initialization. But I
> agree, it's confusing.
>
> > Also, have you guys talked to gcc people about just making it a real
> > feature, like I think it is for clang? In particular, I still suspect
> > that we could/should  just make zero-filling the *default* in the long
> > run, and say "our C standard is that local variables are initialized
> > to zero, exactly the same way static variables are".
I wonder how hard it should be to make a zero-filling GCC plugin?
I'm not a big fan of hacking GCC, but it shouldn't differ much from
the existing GCC plugins that initialize locals.

> Yes, this is on the list for discussion at Plumber's. Having gcc do
> auto-init is the first part. Convincing Clang that _zero_ init isn't
> a language-breaking change is the second part. :P That's been a whole
> other issue.
>
> > I know you posted some numbers somewhere (well, I'm pretty sure you
> > did) and the full stack initialization really was pretty cheap,
> > wasn't it?
>
> Yes, Clang's initialization (which is 0xAA not 0x00 in most cases) is
> cheap. There are rumors(?) of some pathological workloads, though. I
> haven't seen real numbers for that though.
>
> I'll try to find the Clang numbers (maybe Alexander has them?) but I
> remember it being the same as (or maybe better than) the gcc-plugin
> version, which I measured here:

I've some stale data collected on an x86 QEMU instance.
For 0x00 stack initialization:
 - hackbench, netperf and parallel Linux build were virtually free
(slowdown within stdev)
 - for af_inet_loopback the slowdown was ~4%
For 0xAA stack initialization:
 - netperf and parallel Linux build were free
 - for hackbench the slowdown was ~1.5%
 - for af_inet_loopback the slowdown was ~7%

Since then we've made some improvements to Clang (and more are coming,
e.g. cross-function DSE in https://reviews.llvm.org/D61879), so I
expect the performance numbers to be better now.
A big chunk of slowdown had been caused by DSE not working well with
inline assembly on x86, need to check what's the current status there.

Lately I've been mostly benchmarking Android system performance using
the hwuimacro benchmark suite (an end-to-end benchmark for various UI
features).
Most of the benchmarks are unaffected by stack initialization, however
there are cases in which the slowdown is up to 5% for no obvious
reason (the hottest functions don't have initialization code in them,
slowdown could be related to increased icache pressure).

The biggest problem is that there's no single slowdown number to report.
Benchmarks like netperf may show big slowdowns, but many systems don't
run under netperf-like load 24/7
For graphical apps it's critical that the user doesn't notice the UI
glitches introduced by the instrumentation, so benchmarks exploiting
the full graphical stack are a lot more interesting.
Those often have big variance though, and are very specific to the
particular system.

Alex

> https://git.kernel.org/linus/81a56f6dcd20
>
> --
> Kees Cook


--
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg

Re: [GIT PULL] meminit fix for v5.3-rc2

[Linus Torvalds]

On Tue, Jul 30, 2019 at 6:53 AM Alexander Potapenko <glider@google.com> wrote:
>
> I wonder how hard it should be to make a zero-filling GCC plugin?
> I'm not a big fan of hacking GCC, but it shouldn't differ much from
> the existing GCC plugins that initialize locals.

The thing is, as long as it's a plugin, I don't think we can rely on
it. The gcc people will rightly just laugh at us if we were to report
a bug with some kernel plugin.

So I'd like the zeroing of local variables to be a native compiler
option, so that we can (_eventually_ - these things take a long time)
just start saying "ok, we simply consider stack variables to be always
initialized".

> I've some stale data collected on an x86 QEMU instance.
> For 0x00 stack initialization:
>  - hackbench, netperf and parallel Linux build were virtually free
> (slowdown within stdev)
>  - for af_inet_loopback the slowdown was ~4%
> For 0xAA stack initialization:
>  - netperf and parallel Linux build were free
>  - for hackbench the slowdown was ~1.5%
>  - for af_inet_loopback the slowdown was ~7%

So I would expect that we have some special cases where we end up
having arrays (or big structures) on the stack that end up being
critical, and where initializing them is clearly  abad idea.

Then we can verify manually are very much initialized, and that we
could then mark and say "this is uninitialized".

So when a compiler has an option to initialize stack variables, it
would probably _also_ be a very good idea for that compiler to then
support a variable attribute that says "don't initialize _this_
variable, I will do that manually".

But if we in ten years had a kernel model where only allocations and
variables that were _explicitly_ uninitialized, that would be lovely.

Then you can grep for those and verify that "yes, this is safe".

We've historically had the reverse model - things are uninitialized by
default, and you have to explicitly initialize them. Turning that on
its head is what I would like to do long-term.

(For normal allocations that wouldn't be too bad: get rid of
__GFP_ZERO and friends, and instead do __GFP_UNINITIALIZED).

Again - I don't think we want a world where everything is
force-initialized. There _are_ going to be situations where that just
hurts too much. But if we get to a place where we are zero-initialized
by default, and have to explicitly mark the unsafe things (and we'll
have comments not just about how they get initialized, but also about
why that particular thing is so performance-critical), that would be a
good place to be.

This, btw, is why I also think that the "initialize with poison" is
pointless and wrong. Yes, it can find bugs, but it doesn't really help
improve the general situation, and people see it as a debugging tool,
not a "improve code quality and improve the life of kernel developers"
tool.

                Linus

Re: [GIT PULL] meminit fix for v5.3-rc2

[Alexander Potapenko]

On Tue, Jul 30, 2019 at 9:54 PM Linus Torvalds
<torvalds@linux-foundation.org> wrote:
>
> On Tue, Jul 30, 2019 at 6:53 AM Alexander Potapenko <glider@google.com> wrote:
> >
> > I wonder how hard it should be to make a zero-filling GCC plugin?
> > I'm not a big fan of hacking GCC, but it shouldn't differ much from
> > the existing GCC plugins that initialize locals.
>
> The thing is, as long as it's a plugin, I don't think we can rely on
> it. The gcc people will rightly just laugh at us if we were to report
> a bug with some kernel plugin.
>
> So I'd like the zeroing of local variables to be a native compiler
> option, so that we can (_eventually_ - these things take a long time)
> just start saying "ok, we simply consider stack variables to be always
> initialized".
>
> > I've some stale data collected on an x86 QEMU instance.
> > For 0x00 stack initialization:
> >  - hackbench, netperf and parallel Linux build were virtually free
> > (slowdown within stdev)
> >  - for af_inet_loopback the slowdown was ~4%
> > For 0xAA stack initialization:
> >  - netperf and parallel Linux build were free
> >  - for hackbench the slowdown was ~1.5%
> >  - for af_inet_loopback the slowdown was ~7%
>
> So I would expect that we have some special cases where we end up
> having arrays (or big structures) on the stack that end up being
> critical, and where initializing them is clearly  abad idea.
>
> Then we can verify manually are very much initialized, and that we
> could then mark and say "this is uninitialized".
>
> So when a compiler has an option to initialize stack variables, it
> would probably _also_ be a very good idea for that compiler to then
> support a variable attribute that says "don't initialize _this_
> variable, I will do that manually".
FWIW Clang has __attribute((uninitialized)) already:
https://reviews.llvm.org/rL349442
I agree that making it in GCC is easier if initialization itself is
implemented as part of GCC.

> But if we in ten years had a kernel model where only allocations and
> variables that were _explicitly_ uninitialized, that would be lovely.
>
> Then you can grep for those and verify that "yes, this is safe".
>
> We've historically had the reverse model - things are uninitialized by
> default, and you have to explicitly initialize them. Turning that on
> its head is what I would like to do long-term.
>
> (For normal allocations that wouldn't be too bad: get rid of
> __GFP_ZERO and friends, and instead do __GFP_UNINITIALIZED).
There've been concerns about such flags easily going out of control
(my original proposal for heap initialization contained such a flag).
To some extent their spread can be prevented by build-time checks, but
a simple grep can be insufficient, as people will start creating
helper functions to allocate with __GFP_UNINITIALIZED.

> Again - I don't think we want a world where everything is
> force-initialized. There _are_ going to be situations where that just
> hurts too much. But if we get to a place where we are zero-initialized
> by default, and have to explicitly mark the unsafe things (and we'll
> have comments not just about how they get initialized, but also about
> why that particular thing is so performance-critical), that would be a
> good place to be.
>
> This, btw, is why I also think that the "initialize with poison" is
> pointless and wrong. Yes, it can find bugs, but it doesn't really help
> improve the general situation, and people see it as a debugging tool,
> not a "improve code quality and improve the life of kernel developers"
> tool.
This sure makes sense. If this policy is adopted kernel-wide, we won't
need any debugging tools for uninit variables, so it's natural to
initialize them to zero.
>                 Linus



-- 
Alexander Potapenko
Software Engineer

Google Germany GmbH
Erika-Mann-Straße, 33
80636 München

Geschäftsführer: Paul Manicle, Halimah DeLaine Prado
Registergericht und -nummer: Hamburg, HRB 86891
Sitz der Gesellschaft: Hamburg