Re: [PATCH v2 21/37] kasan: introduce CONFIG_KASAN_HW_TAGS

[Andrey Konovalov]

On Fri, Sep 18, 2020 at 2:32 PM Marco Elver <elver@xxxxxxxxxx> wrote:
>
> On Tue, Sep 15, 2020 at 11:16PM +0200, Andrey Konovalov wrote:
> > This patch adds a configuration option for a new KASAN mode called
> > hardware tag-based KASAN. This mode uses the memory tagging approach
> > like the software tag-based mode, but relies on arm64 Memory Tagging
> > Extension feature for tag management and access checking.
> >
> > Signed-off-by: Andrey Konovalov <andreyknvl@xxxxxxxxxx>
> > Signed-off-by: Vincenzo Frascino <vincenzo.frascino@xxxxxxx>
> > ---
> > Change-Id: I246c2def9fffa6563278db1bddfbe742ca7bdefe
> > ---
> >  lib/Kconfig.kasan | 56 +++++++++++++++++++++++++++++++++--------------
> >  1 file changed, 39 insertions(+), 17 deletions(-)
> >
> > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> > index b4cf6c519d71..17c9ecfaecb9 100644
> > --- a/lib/Kconfig.kasan
> > +++ b/lib/Kconfig.kasan
> > @@ -6,7 +6,10 @@ config HAVE_ARCH_KASAN
> >  config HAVE_ARCH_KASAN_SW_TAGS
> >       bool
> >
> > -config       HAVE_ARCH_KASAN_VMALLOC
> > +config HAVE_ARCH_KASAN_HW_TAGS
> > +     bool
> > +
> > +config HAVE_ARCH_KASAN_VMALLOC
> >       bool
> >
> >  config CC_HAS_KASAN_GENERIC
> > @@ -20,10 +23,11 @@ config CC_HAS_WORKING_NOSANITIZE_ADDRESS
> >
> >  menuconfig KASAN
> >       bool "KASAN: runtime memory debugger"
> > -     depends on (HAVE_ARCH_KASAN && CC_HAS_KASAN_GENERIC) || \
> > -                (HAVE_ARCH_KASAN_SW_TAGS && CC_HAS_KASAN_SW_TAGS)
> > +     depends on (((HAVE_ARCH_KASAN && CC_HAS_KASAN_GENERIC) || \
> > +                  (HAVE_ARCH_KASAN_SW_TAGS && CC_HAS_KASAN_SW_TAGS)) && \
> > +                 CC_HAS_WORKING_NOSANITIZE_ADDRESS) || \
> > +                HAVE_ARCH_KASAN_HW_TAGS
> >       depends on (SLUB && SYSFS) || (SLAB && !DEBUG_SLAB)
> > -     depends on CC_HAS_WORKING_NOSANITIZE_ADDRESS
> >       select SLUB_DEBUG if SLUB
>
> Is SLUB_DEBUG necessary with HW_TAGS?

I'll check and drop it if it's unnecessary.

> >       select CONSTRUCTORS
> >       select STACKDEPOT
> > @@ -38,13 +42,18 @@ choice
> >       prompt "KASAN mode"
> >       default KASAN_GENERIC
> >       help
> > -       KASAN has two modes: generic KASAN (similar to userspace ASan,
> > -       x86_64/arm64/xtensa, enabled with CONFIG_KASAN_GENERIC) and
> > -       software tag-based KASAN (a version based on software memory
> > -       tagging, arm64 only, similar to userspace HWASan, enabled with
> > -       CONFIG_KASAN_SW_TAGS).
> > +       KASAN has three modes:
> > +       1. generic KASAN (similar to userspace ASan,
> > +          x86_64/arm64/xtensa, enabled with CONFIG_KASAN_GENERIC),
> > +       2. software tag-based KASAN (arm64 only, based on software
> > +          memory tagging (similar to userspace HWASan), enabled with
> > +          CONFIG_KASAN_SW_TAGS), and
> > +       3. hardware tag-based KASAN (arm64 only, based on hardware
> > +          memory tagging, enabled with CONFIG_KASAN_HW_TAGS).
> >
> > -       Both generic and tag-based KASAN are strictly debugging features.
> > +       All KASAN modes are strictly debugging features.
> > +
> > +       For better error detection enable CONFIG_STACKTRACE.
>
> I don't think CONFIG_STACKTRACE improves error detection, right? It only
> makes the reports more readable

Yes, will fix.

> >
> >  config KASAN_GENERIC
> >       bool "Generic mode"
> > @@ -61,8 +70,6 @@ config KASAN_GENERIC
> >         and introduces an overhead of ~x1.5 for the rest of the allocations.
> >         The performance slowdown is ~x3.
> >
> > -       For better error detection enable CONFIG_STACKTRACE.
> > -
> >         Currently CONFIG_KASAN_GENERIC doesn't work with CONFIG_DEBUG_SLAB
> >         (the resulting kernel does not boot).
> >
> > @@ -72,9 +79,11 @@ config KASAN_SW_TAGS
> >       help
> >         Enables software tag-based KASAN mode.
> >
> > -       This mode requires Top Byte Ignore support by the CPU and therefore
> > -       is only supported for arm64. This mode requires Clang version 7.0.0
> > -       or later.
> > +       This mode require software memory tagging support in the form of
> > +       HWASan-like compiler instrumentation.
> > +
> > +       Currently this mode is only implemented for arm64 CPUs and relies on
> > +       Top Byte Ignore. This mode requires Clang version 7.0.0 or later.
> >
> >         This mode consumes about 1/16th of available memory at kernel start
> >         and introduces an overhead of ~20% for the rest of the allocations.
> > @@ -82,15 +91,27 @@ config KASAN_SW_TAGS
> >         casting and comparison, as it embeds tags into the top byte of each
> >         pointer.
> >
> > -       For better error detection enable CONFIG_STACKTRACE.
> > -
> >         Currently CONFIG_KASAN_SW_TAGS doesn't work with CONFIG_DEBUG_SLAB
> >         (the resulting kernel does not boot).
> >
> > +config KASAN_HW_TAGS
> > +     bool "Hardware tag-based mode"
> > +     depends on HAVE_ARCH_KASAN_HW_TAGS
> > +     depends on SLUB
> > +     help
> > +       Enables hardware tag-based KASAN mode.
> > +
> > +       This mode requires hardware memory tagging support, and can be used
> > +       by any architecture that provides it.
> > +
> > +       Currently this mode is only implemented for arm64 CPUs starting from
> > +       ARMv8.5 and relies on Memory Tagging Extension and Top Byte Ignore.
> > +
> >  endchoice
> >
> >  choice
> >       prompt "Instrumentation type"
> > +     depends on KASAN_GENERIC || KASAN_SW_TAGS
> >       default KASAN_OUTLINE
> >
> >  config KASAN_OUTLINE
> > @@ -114,6 +135,7 @@ endchoice
> >
> >  config KASAN_STACK_ENABLE
> >       bool "Enable stack instrumentation (unsafe)" if CC_IS_CLANG && !COMPILE_TEST
> > +     depends on KASAN_GENERIC || KASAN_SW_TAGS
> >       help
> >         The LLVM stack address sanitizer has a know problem that
> >         causes excessive stack usage in a lot of functions, see
>
> How about something like the below change (introduce KASAN_INSTRUMENTED
> Kconfig var) to avoid the repeated "KASAN_GENERIC || KASAN_SW_TAGS".
> This could then also be used in the various .c/.h files (and make some
> of the code more readable hopefully).

I tried doing that initially, but it didn't really look good. The
reason is that we actually have two properties that are currently
common for the software modes, but aren't actually tied to each other:
instrumentation and shadow memory. Therefore we will end up with two
new configs: KASAN_INSTRUMENTED and KASAN_USES_SHADOW (or something),
and things get quite confusing. I think it's better to keep
KASAN_GENERIC || KASAN_SW_TAGS everywhere.