Re: [PATCH RFC 00/10] KFENCE: A low-overhead sampling-based memory safety error detector

From: Alexander Potapenko <glider@google.com>
To: Vlastimil Babka <vbabka@suse.cz>
Cc: Marco Elver <elver@google.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Catalin Marinas <catalin.marinas@arm.com>,
	Christoph Lameter <cl@linux.com>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Mark Rutland <mark.rutland@arm.com>,
	Pekka Enberg <penberg@kernel.org>,
	"H. Peter Anvin" <hpa@zytor.com>,
	paulmck@kernel.org, Andrey Konovalov <andreyknvl@google.com>,
	Andrey Ryabinin <aryabinin@virtuozzo.com>,
	Andy Lutomirski <luto@kernel.org>, Borislav Petkov <bp@alien8.de>,
	dave.hansen@linux.intel.com, Dmitriy Vyukov <dvyukov@google.com>,
	Eric Dumazet <edumazet@google.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Ingo Molnar <mingo@redhat.com>, Jann Horn <jannh@google.com>,
	Jonathan Corbet <corbet@lwn.net>,
	Kees Cook <keescook@chromium.org>,
	Peter Zijlstra <peterz@infradead.org>, Qian Cai <cai@lca.pw>,
	Thomas Gleixner <tglx@linutronix.de>,
	Will Deacon <will@kernel.org>,
	"the arch/x86 maintainers" <x86@kernel.org>,
	linux-doc@vger.kernel.org, LKML <linux-kernel@vger.kernel.org>,
	kasan-dev <kasan-dev@googlegroups.com>,
	linux-arm-kernel@lists.infradead.org,
	Linux Memory Management List <linux-mm@kvack.org>
Subject: Re: [PATCH RFC 00/10] KFENCE: A low-overhead sampling-based memory safety error detector
Date: Tue, 8 Sep 2020 14:16:09 +0200	[thread overview]
Message-ID: <CAG_fn=UdnN4EL6OtAV8RY7kuqO+VXqSsf+grx2Le64UQJOUMvQ@mail.gmail.com> (raw)
In-Reply-To: <4dc8852a-120d-0835-1dc4-1a91f8391c8a@suse.cz>

> Toggling a static branch is AFAIK quite disruptive (PeterZ will probably tell
> you better), and with the default 100ms sample interval, I'd think it's not good
> to toggle it so often? Did you measure what performance would you get, if the
> static key was only for long-term toggling the whole feature on and off (boot
> time or even runtime), but the decisions "am I in a sample interval right now?"
> would be normal tests behind this static key? Thanks.

100ms is the default that we use for testing, but for production it
should be fine to pick a longer interval (e.g. 1 second or more).
We haven't noticed any performance impact with neither 100ms nor bigger values.

Regarding using normal branches, they are quite expensive.
E.g. at some point we used to have a branch in slab_free() to check
whether the freed object belonged to KFENCE pool.
When the pool address was taken from memory, this resulted in some
non-zero performance penalty.

As for enabling the whole feature at runtime, our intention is to let
the users have it enabled by default, otherwise someone will need to
tell every machine in the fleet when the feature is to be enabled.
>
> > We have verified by running synthetic benchmarks (sysbench I/O,
> > hackbench) that a kernel with KFENCE is performance-neutral compared to
> > a non-KFENCE baseline kernel.
> >
> > KFENCE is inspired by GWP-ASan [1], a userspace tool with similar
> > properties. The name "KFENCE" is a homage to the Electric Fence Malloc
> > Debugger [2].
> >
> > For more details, see Documentation/dev-tools/kfence.rst added in the
> > series -- also viewable here:
> >
> >       https://raw.githubusercontent.com/google/kasan/kfence/Documentation/dev-tools/kfence.rst
> >
> > [1] http://llvm.org/docs/GwpAsan.html
> > [2] https://linux.die.net/man/3/efence
> >
> > Alexander Potapenko (6):
> >   mm: add Kernel Electric-Fence infrastructure
> >   x86, kfence: enable KFENCE for x86
> >   mm, kfence: insert KFENCE hooks for SLAB
> >   mm, kfence: insert KFENCE hooks for SLUB
> >   kfence, kasan: make KFENCE compatible with KASAN
> >   kfence, kmemleak: make KFENCE compatible with KMEMLEAK
> >
> > Marco Elver (4):
> >   arm64, kfence: enable KFENCE for ARM64
> >   kfence, lockdep: make KFENCE compatible with lockdep
> >   kfence, Documentation: add KFENCE documentation
> >   kfence: add test suite
> >
> >  Documentation/dev-tools/index.rst  |   1 +
> >  Documentation/dev-tools/kfence.rst | 285 +++++++++++
> >  MAINTAINERS                        |  11 +
> >  arch/arm64/Kconfig                 |   1 +
> >  arch/arm64/include/asm/kfence.h    |  39 ++
> >  arch/arm64/mm/fault.c              |   4 +
> >  arch/x86/Kconfig                   |   2 +
> >  arch/x86/include/asm/kfence.h      |  60 +++
> >  arch/x86/mm/fault.c                |   4 +
> >  include/linux/kfence.h             | 174 +++++++
> >  init/main.c                        |   2 +
> >  kernel/locking/lockdep.c           |   8 +
> >  lib/Kconfig.debug                  |   1 +
> >  lib/Kconfig.kfence                 |  70 +++
> >  mm/Makefile                        |   1 +
> >  mm/kasan/common.c                  |   7 +
> >  mm/kfence/Makefile                 |   6 +
> >  mm/kfence/core.c                   | 730 +++++++++++++++++++++++++++
> >  mm/kfence/kfence-test.c            | 777 +++++++++++++++++++++++++++++
> >  mm/kfence/kfence.h                 | 104 ++++
> >  mm/kfence/report.c                 | 201 ++++++++
> >  mm/kmemleak.c                      |  11 +
> >  mm/slab.c                          |  46 +-
> >  mm/slab_common.c                   |   6 +-
> >  mm/slub.c                          |  72 ++-
> >  25 files changed, 2591 insertions(+), 32 deletions(-)
> >  create mode 100644 Documentation/dev-tools/kfence.rst
> >  create mode 100644 arch/arm64/include/asm/kfence.h
> >  create mode 100644 arch/x86/include/asm/kfence.h
> >  create mode 100644 include/linux/kfence.h
> >  create mode 100644 lib/Kconfig.kfence
> >  create mode 100644 mm/kfence/Makefile
> >  create mode 100644 mm/kfence/core.c
> >  create mode 100644 mm/kfence/kfence-test.c
> >  create mode 100644 mm/kfence/kfence.h
> >  create mode 100644 mm/kfence/report.c
> >
>

-- 
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