[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date: Tue, 16 Apr 2019 14:21:59 +0200
From: Alexander Potapenko <glider@...gle.com>
To: Andrew Morton <akpm@...ux-foundation.org>
Cc: linux-security-module <linux-security-module@...r.kernel.org>,
Linux Memory Management List <linux-mm@...ck.org>, Nick Desaulniers <ndesaulniers@...gle.com>,
Kostya Serebryany <kcc@...gle.com>, Dmitriy Vyukov <dvyukov@...gle.com>, Kees Cook <keescook@...omium.org>,
Sandeep Patil <sspatil@...roid.com>, Laura Abbott <labbott@...hat.com>,
Kernel Hardening <kernel-hardening@...ts.openwall.com>
Subject: Re: [PATCH] mm: security: introduce CONFIG_INIT_HEAP_ALL
On Tue, Apr 16, 2019 at 4:02 AM Andrew Morton <akpm@...ux-foundation.org> wrote:
>
> On Fri, 12 Apr 2019 14:45:01 +0200 Alexander Potapenko <glider@...gle.com> wrote:
>
> > This config option adds the possibility to initialize newly allocated
> > pages and heap objects with zeroes.
>
> At what cost? Some performance test results would help this along.
I'll make more measurements for the new implementation, but the
preliminary results are:
~0.17% sys time slowdown (~0% wall time slowdown) on hackbench (1 CPU);
1.3% sys time slowdown (0.2% wall time slowdown) when building Linux with -j12;
4% sys time slowdown (2.6% wall time slowdown) on af_inet_loopback benchmark;
up to 100% slowdown on netperf (caused by sk buffers being initialized
multiple times; also netperf is too fast to perform any precise
measurements)
Are there any benchmarks you can recommend?
> > This is needed to prevent possible
> > information leaks and make the control-flow bugs that depend on
> > uninitialized values more deterministic.
> >
> > Initialization is done at allocation time at the places where checks for
> > __GFP_ZERO are performed. We don't initialize slab caches with
> > constructors or SLAB_TYPESAFE_BY_RCU to preserve their semantics.
> >
> > For kernel testing purposes filling allocations with a nonzero pattern
> > would be more suitable, but may require platform-specific code. To have
> > a simple baseline we've decided to start with zero-initialization.
> >
> > No performance optimizations are done at the moment to reduce double
> > initialization of memory regions.
>
> Requiring a kernel rebuild is rather user-hostile.
This is intended to be used together with other hardening measures,
like CONFIG_INIT_STACK_ALL (see a patchset by Kees).
All of those require a kernel rebuild, but we assume users don't push
and pull that lever back and forth often.
> A boot option
> (early_param()) would be much more useful and I expect that the loss in
> coverage would be small and acceptable? Could possibly use the
> static_branch infrastructure.
I'll try that out and see if there's a notable performance difference.
> > --- a/mm/slab.h
> > +++ b/mm/slab.h
> > @@ -167,6 +167,16 @@ static inline slab_flags_t kmem_cache_flags(unsigned int object_size,
> > SLAB_TEMPORARY | \
> > SLAB_ACCOUNT)
> >
> > +/*
> > + * Do we need to initialize this allocation?
> > + * Always true for __GFP_ZERO, CONFIG_INIT_HEAP_ALL enforces initialization
> > + * of caches without constructors and RCU.
> > + */
> > +#define SLAB_WANT_INIT(cache, gfp_flags) \
> > + ((GFP_INIT_ALWAYS_ON && !(cache)->ctor && \
> > + !((cache)->flags & SLAB_TYPESAFE_BY_RCU)) || \
> > + (gfp_flags & __GFP_ZERO))
>
> Is there any reason why this *must* be implemented as a macro? If not,
> it should be written in C please.
Agreed. Even in the case we want GFP_INIT_ALWAYS_ON to be known at
compile time there's no reason for this to be a macro.
>
--
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
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
