On Fri, 18 Sep 2020 at 17:06, 'Andrey Konovalov' via kasan-dev
<[email protected]> wrote:
>
> On Fri, Sep 18, 2020 at 2:32 PM Marco Elver <[email protected]> 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 <[email protected]>
> > > Signed-off-by: Vincenzo Frascino <[email protected]>
> > > ---
> > > 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
On Fri, Sep 18, 2020 at 5:36 PM Marco Elver <[email protected]> wrote:
>
> > > 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?).
Correct.
> 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.
Let's leave it as is then. I don't think the code will get much better
in terms of readability if we add KASAN_SOFTWARE, but we'll get
another "indirect" config option, which makes things a bit more
confusing.