On Fri, Jan 11, 2019 at 7:58 PM Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx> wrote: > > Use after scope bugs detector seems to be almost entirely useless > for the linux kernel. It exists over two years, but I've seen only > one valid bug so far [1]. And the bug was fixed before it has been > reported. There were some other use-after-scope reports, but they > were false-positives due to different reasons like incompatibility > with structleak plugin. > > This feature significantly increases stack usage, especially with > GCC < 9 version, and causes a 32K stack overflow. It probably > adds performance penalty too. > > Given all that, let's remove use-after-scope detector entirely. > > While preparing this patch I've noticed that we mistakenly enable > use-after-scope detection for clang compiler regardless of > CONFIG_KASAN_EXTRA setting. This is also fixed now. Hi Andrey, I am on a fence. On one hand removing bug detection sucks and each case of a missed memory corruption leads to a splash of assorted bug reports by syzbot. On the other hand everything you said is true. Maybe support for CONFIG_VMAP_STACK will enable stacks larger then PAGE_ALLOC_COSTLY_ORDER? > [1] http://lkml.kernel.org/r/<20171129052106.rhgbjhhis53hkgfn@xxxxxxxxxxxxxxxxxxxxxx> > > Signed-off-by: Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx> > Cc: Qian Cai <cai@xxxxxx> > Cc: Alexander Potapenko <glider@xxxxxxxxxx> > Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx> > Cc: Catalin Marinas <catalin.marinas@xxxxxxx> > Cc: Will Deacon <will.deacon@xxxxxxx> > --- > arch/arm64/include/asm/memory.h | 4 ---- > lib/Kconfig.debug | 1 - > lib/Kconfig.kasan | 10 ---------- > lib/test_kasan.c | 24 ------------------------ > mm/kasan/generic.c | 19 ------------------- > mm/kasan/generic_report.c | 3 --- > mm/kasan/kasan.h | 3 --- > scripts/Makefile.kasan | 5 ----- > scripts/gcc-plugins/Kconfig | 4 ---- > 9 files changed, 73 deletions(-) > > diff --git a/arch/arm64/include/asm/memory.h b/arch/arm64/include/asm/memory.h > index e1ec947e7c0c..0e236a99b3ef 100644 > --- a/arch/arm64/include/asm/memory.h > +++ b/arch/arm64/include/asm/memory.h > @@ -80,11 +80,7 @@ > */ > #ifdef CONFIG_KASAN > #define KASAN_SHADOW_SIZE (UL(1) << (VA_BITS - KASAN_SHADOW_SCALE_SHIFT)) > -#ifdef CONFIG_KASAN_EXTRA > -#define KASAN_THREAD_SHIFT 2 > -#else > #define KASAN_THREAD_SHIFT 1 > -#endif /* CONFIG_KASAN_EXTRA */ > #else > #define KASAN_SHADOW_SIZE (0) > #define KASAN_THREAD_SHIFT 0 > diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug > index d4df5b24d75e..a219f3488ad7 100644 > --- a/lib/Kconfig.debug > +++ b/lib/Kconfig.debug > @@ -222,7 +222,6 @@ config ENABLE_MUST_CHECK > config FRAME_WARN > int "Warn for stack frames larger than (needs gcc 4.4)" > range 0 8192 > - default 3072 if KASAN_EXTRA > default 2048 if GCC_PLUGIN_LATENT_ENTROPY > default 1280 if (!64BIT && PARISC) > default 1024 if (!64BIT && !PARISC) > diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan > index d8c474b6691e..67d7d1309c52 100644 > --- a/lib/Kconfig.kasan > +++ b/lib/Kconfig.kasan > @@ -78,16 +78,6 @@ config KASAN_SW_TAGS > > endchoice > > -config KASAN_EXTRA > - bool "KASAN: extra checks" > - depends on KASAN_GENERIC && DEBUG_KERNEL && !COMPILE_TEST > - help > - This enables further checks in generic KASAN, for now it only > - includes the address-use-after-scope check that can lead to > - excessive kernel stack usage, frame size warnings and longer > - compile time. > - See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 > - > choice > prompt "Instrumentation type" > depends on KASAN > diff --git a/lib/test_kasan.c b/lib/test_kasan.c > index 51b78405bf24..7de2702621dc 100644 > --- a/lib/test_kasan.c > +++ b/lib/test_kasan.c > @@ -480,29 +480,6 @@ static noinline void __init copy_user_test(void) > kfree(kmem); > } > > -static noinline void __init use_after_scope_test(void) > -{ > - volatile char *volatile p; > - > - pr_info("use-after-scope on int\n"); > - { > - int local = 0; > - > - p = (char *)&local; > - } > - p[0] = 1; > - p[3] = 1; > - > - pr_info("use-after-scope on array\n"); > - { > - char local[1024] = {0}; > - > - p = local; > - } > - p[0] = 1; > - p[1023] = 1; > -} > - > static noinline void __init kasan_alloca_oob_left(void) > { > volatile int i = 10; > @@ -682,7 +659,6 @@ static int __init kmalloc_tests_init(void) > kasan_alloca_oob_right(); > ksize_unpoisons_memory(); > copy_user_test(); > - use_after_scope_test(); > kmem_cache_double_free(); > kmem_cache_invalid_free(); > kasan_memchr(); > diff --git a/mm/kasan/generic.c b/mm/kasan/generic.c > index ccb6207276e3..504c79363a34 100644 > --- a/mm/kasan/generic.c > +++ b/mm/kasan/generic.c > @@ -275,25 +275,6 @@ EXPORT_SYMBOL(__asan_storeN_noabort); > void __asan_handle_no_return(void) {} > EXPORT_SYMBOL(__asan_handle_no_return); > > -/* Emitted by compiler to poison large objects when they go out of scope. */ > -void __asan_poison_stack_memory(const void *addr, size_t size) > -{ > - /* > - * Addr is KASAN_SHADOW_SCALE_SIZE-aligned and the object is surrounded > - * by redzones, so we simply round up size to simplify logic. > - */ > - kasan_poison_shadow(addr, round_up(size, KASAN_SHADOW_SCALE_SIZE), > - KASAN_USE_AFTER_SCOPE); > -} > -EXPORT_SYMBOL(__asan_poison_stack_memory); > - > -/* Emitted by compiler to unpoison large objects when they go into scope. */ > -void __asan_unpoison_stack_memory(const void *addr, size_t size) > -{ > - kasan_unpoison_shadow(addr, size); > -} > -EXPORT_SYMBOL(__asan_unpoison_stack_memory); > - > /* Emitted by compiler to poison alloca()ed objects. */ > void __asan_alloca_poison(unsigned long addr, size_t size) > { > diff --git a/mm/kasan/generic_report.c b/mm/kasan/generic_report.c > index 5e12035888f2..36c645939bc9 100644 > --- a/mm/kasan/generic_report.c > +++ b/mm/kasan/generic_report.c > @@ -82,9 +82,6 @@ static const char *get_shadow_bug_type(struct kasan_access_info *info) > case KASAN_KMALLOC_FREE: > bug_type = "use-after-free"; > break; > - case KASAN_USE_AFTER_SCOPE: > - bug_type = "use-after-scope"; > - break; > case KASAN_ALLOCA_LEFT: > case KASAN_ALLOCA_RIGHT: > bug_type = "alloca-out-of-bounds"; > diff --git a/mm/kasan/kasan.h b/mm/kasan/kasan.h > index ea51b2d898ec..3e0c11f7d7a1 100644 > --- a/mm/kasan/kasan.h > +++ b/mm/kasan/kasan.h > @@ -34,7 +34,6 @@ > #define KASAN_STACK_MID 0xF2 > #define KASAN_STACK_RIGHT 0xF3 > #define KASAN_STACK_PARTIAL 0xF4 > -#define KASAN_USE_AFTER_SCOPE 0xF8 > > /* > * alloca redzone shadow values > @@ -187,8 +186,6 @@ void __asan_unregister_globals(struct kasan_global *globals, size_t size); > void __asan_loadN(unsigned long addr, size_t size); > void __asan_storeN(unsigned long addr, size_t size); > void __asan_handle_no_return(void); > -void __asan_poison_stack_memory(const void *addr, size_t size); > -void __asan_unpoison_stack_memory(const void *addr, size_t size); > void __asan_alloca_poison(unsigned long addr, size_t size); > void __asan_allocas_unpoison(const void *stack_top, const void *stack_bottom); > > diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan > index 25c259df8ffa..f1fb8e502657 100644 > --- a/scripts/Makefile.kasan > +++ b/scripts/Makefile.kasan > @@ -27,14 +27,9 @@ else > $(call cc-param,asan-globals=1) \ > $(call cc-param,asan-instrumentation-with-call-threshold=$(call_threshold)) \ > $(call cc-param,asan-stack=1) \ > - $(call cc-param,asan-use-after-scope=1) \ > $(call cc-param,asan-instrument-allocas=1) > endif > > -ifdef CONFIG_KASAN_EXTRA > -CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope) > -endif > - > endif # CONFIG_KASAN_GENERIC > > ifdef CONFIG_KASAN_SW_TAGS > diff --git a/scripts/gcc-plugins/Kconfig b/scripts/gcc-plugins/Kconfig > index d45f7f36b859..d9fd9988ef27 100644 > --- a/scripts/gcc-plugins/Kconfig > +++ b/scripts/gcc-plugins/Kconfig > @@ -68,10 +68,6 @@ config GCC_PLUGIN_LATENT_ENTROPY > > config GCC_PLUGIN_STRUCTLEAK > bool "Force initialization of variables containing userspace addresses" > - # Currently STRUCTLEAK inserts initialization out of live scope of > - # variables from KASAN point of view. This leads to KASAN false > - # positive reports. Prohibit this combination for now. > - depends on !KASAN_EXTRA > help > This plugin zero-initializes any structures containing a > __user attribute. This can prevent some classes of information > -- > 2.19.2 > > -- > You received this message because you are subscribed to the Google Groups "kasan-dev" group. > To unsubscribe from this group and stop receiving emails from it, send an email to kasan-dev+unsubscribe@xxxxxxxxxxxxxxxx. > To post to this group, send email to kasan-dev@xxxxxxxxxxxxxxxx. > To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20190111185842.13978-1-aryabinin%40virtuozzo.com. > For more options, visit https://groups.google.com/d/optout.
