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

[Marco Elver]

On Fri, 18 Sep 2020 at 17:06, 'Andrey Konovalov' via kasan-dev
<kasan-dev@xxxxxxxxxxxxxxxx> wrote:
>
> 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.

Ah, I see. So in some cases the reason the #ifdef exists is because of
instrumentation, in other cases because there is some shadow memory
(right?).

The only other option I see is to call it what it is ("KASAN_SW" or
"KASAN_SOFTWARE"), but other than that, I don't mind if it stays
as-is.

Thanks,
-- Marco