Re: [PATCH v2] kasan: don't emit builtin calls when sanitization is off

[Andrey Konovalov]

On Tue, Jan 23, 2018 at 3:20 AM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> On Fri, 19 Jan 2018 18:58:12 +0100 Andrey Konovalov <andreyknvl@xxxxxxxxxx> wrote:
>
>> With KASAN enabled the kernel has two different memset() functions, one
>> with KASAN checks (memset) and one without (__memset). KASAN uses some
>> macro tricks to use the proper version where required. For example memset()
>> calls in mm/slub.c are without KASAN checks, since they operate on poisoned
>> slab object metadata.
>>
>> The issue is that clang emits memset() calls even when there is no memset()
>> in the source code. They get linked with improper memset() implementation
>> and the kernel fails to boot due to a huge amount of KASAN reports during
>> early boot stages.
>>
>> The solution is to add -fno-builtin flag for files with KASAN_SANITIZE := n
>> marker.
>
> This clashes somewhat with Arnd's asan-rework-kconfig-settings.patch.
> Could you two please put heads together and decide what we want for a
> final result?
>
> Meanwhile I'll "fix" the reject with
>
> +ifdef CONFIG_KASAN_EXTRA
>  CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
> +endif

With both patches applied scripts/Makefile.kasan should end like this:

"""
ifdef CONFIG_KASAN_EXTRA
CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
endif

CFLAGS_KASAN_NOSANITIZE := -fno-builtin

endif
"""

If that is what you mean, then the "fix" is correct.

Let me know if you need me to send v3 that can be applied on top of
Arnd's patches.

>
>
> From: Arnd Bergmann <arnd@xxxxxxxx>
> Subject: kasan: rework Kconfig settings
>
> We get a lot of very large stack frames using gcc-7.0.1 with the default
> -fsanitize-address-use-after-scope --param asan-stack=1 options, which can
> easily cause an overflow of the kernel stack, e.g.
>
> drivers/gpu/drm/i915/gvt/handlers.c:2434:1: warning: the frame size of 46176 bytes is larger than 3072 bytes
> drivers/net/wireless/ralink/rt2x00/rt2800lib.c:5650:1: warning: the frame size of 23632 bytes is larger than 3072 bytes
> lib/atomic64_test.c:250:1: warning: the frame size of 11200 bytes is larger than 3072 bytes
> drivers/gpu/drm/i915/gvt/handlers.c:2621:1: warning: the frame size of 9208 bytes is larger than 3072 bytes
> drivers/media/dvb-frontends/stv090x.c:3431:1: warning: the frame size of 6816 bytes is larger than 3072 bytes
> fs/fscache/stats.c:287:1: warning: the frame size of 6536 bytes is larger than 3072 bytes
>
> To reduce this risk, -fsanitize-address-use-after-scope is now split out
> into a separate CONFIG_KASAN_EXTRA Kconfig option, leading to stack frames
> that are smaller than 2 kilobytes most of the time on x86_64.  An earlier
> version of this patch also prevented combining KASAN_EXTRA with
> KASAN_INLINE, but that is no longer necessary with gcc-7.0.1.
>
> All patches to get the frame size below 2048 bytes with CONFIG_KASAN=y and
> CONFIG_KASAN_EXTRA=n have been merged by maintainers now, so we can bring
> back that default now.  KASAN_EXTRA=y still causes lots of warnings but
> now defaults to !COMPILE_TEST to disable it in allmodconfig, and it
> remains disabled in all other defconfigs since it is a new option.  I
> arbitrarily raise the warning limit for KASAN_EXTRA to 3072 to reduce the
> noise, but an allmodconfig kernel still has around 50 warnings on gcc-7.
>
> I experimented a bit more with smaller stack frames and have another
> follow-up series that reduces the warning limit for 64-bit architectures
> to 1280 bytes (without CONFIG_KASAN).
>
> With earlier versions of this patch series, I also had patches to address
> the warnings we get with KASAN and/or KASAN_EXTRA, using a
> "noinline_if_stackbloat" annotation.  That annotation now got replaced
> with a gcc-8 bugfix (see
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715) and a workaround for
> older compilers, which means that KASAN_EXTRA is now just as bad as before
> and will lead to an instant stack overflow in a few extreme cases.
>
> This reverts parts of commit commit 3f181b4 ("lib/Kconfig.debug: disable
> -Wframe-larger-than warnings with KASAN=y").  Two patches in linux-next
> should be merged first to avoid introducing warnings in an allmodconfig
> build:
> 3cd890dbe2a4 ("media: dvb-frontends: fix i2c access helpers for KASAN")
> 16c3ada89cff ("media: r820t: fix r820t_write_reg for KASAN")
>
>
> Do we really need to backport this?
>
> I think we do: without this patch, enabling KASAN will lead to
> unavoidable kernel stack overflow in certain device drivers when built
> with gcc-7 or higher on linux-4.10+ or any version that contains a
> backport of commit c5caf21ab0cf8.  Most people are probably still on
> older compilers, but it will get worse over time as they upgrade their
> distros.
>
> The warnings we get on kernels older than this should all be for code
> that uses dangerously large stack frames, though most of them do not
> cause an actual stack overflow by themselves.The asan-stack option was
> added in linux-4.0, and commit 3f181b4d8652 ("lib/Kconfig.debug:
> disable -Wframe-larger-than warnings with KASAN=y") effectively turned
> off the warning for allmodconfig kernels, so I would like to see this
> fix backported to any kernels later than 4.0.
>
> I have done dozens of fixes for individual functions with stack frames
> larger than 2048 bytes with asan-stack, and I plan to make sure that
> all those fixes make it into the stable kernels as well (most are
> already there).
>
> Part of the complication here is that asan-stack (from 4.0) was
> originally assumed to always require much larger stacks, but that
> turned out to be a combination of multiple gcc bugs that we have now
> worked around and fixed, but sanitize-address-use-after-scope (from
> v4.10) has a much higher inherent stack usage and also suffers from at
> least three other problems that we have analyzed but not yet fixed
> upstream, each of them makes the stack usage more severe than it should
> be.
>
> Link: http://lkml.kernel.org/r/20171221134744.2295529-1-arnd@xxxxxxxx
> Signed-off-by: Arnd Bergmann <arnd@xxxxxxxx>
> Acked-by: Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx>
> Cc: Mauro Carvalho Chehab <mchehab@xxxxxxxxxx>
> Cc: Andrey Ryabinin <aryabinin@xxxxxxxxxxxxx>
> Cc: Alexander Potapenko <glider@xxxxxxxxxx>
> Cc: Dmitry Vyukov <dvyukov@xxxxxxxxxx>
> Cc: <stable@xxxxxxxxxxxxxxx>
>
> Signed-off-by: Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx>
> ---
>
>  lib/Kconfig.debug      |    2 +-
>  lib/Kconfig.kasan      |   11 +++++++++++
>  scripts/Makefile.kasan |    3 +++
>  3 files changed, 15 insertions(+), 1 deletion(-)
>
> diff -puN lib/Kconfig.debug~kasan-rework-kconfig-settings lib/Kconfig.debug
> --- a/lib/Kconfig.debug~kasan-rework-kconfig-settings
> +++ a/lib/Kconfig.debug
> @@ -217,7 +217,7 @@ config ENABLE_MUST_CHECK
>  config FRAME_WARN
>         int "Warn for stack frames larger than (needs gcc 4.4)"
>         range 0 8192
> -       default 0 if KASAN
> +       default 3072 if KASAN_EXTRA
>         default 2048 if GCC_PLUGIN_LATENT_ENTROPY
>         default 1280 if (!64BIT && PARISC)
>         default 1024 if (!64BIT && !PARISC)
> diff -puN lib/Kconfig.kasan~kasan-rework-kconfig-settings lib/Kconfig.kasan
> --- a/lib/Kconfig.kasan~kasan-rework-kconfig-settings
> +++ a/lib/Kconfig.kasan
> @@ -20,6 +20,17 @@ config KASAN
>           Currently CONFIG_KASAN doesn't work with CONFIG_DEBUG_SLAB
>           (the resulting kernel does not boot).
>
> +config KASAN_EXTRA
> +       bool "KAsan: extra checks"
> +       depends on KASAN && DEBUG_KERNEL && !COMPILE_TEST
> +       help
> +         This enables further checks in the kernel address sanitizer, 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.
> +         https://gcc.gnu.org/bugzilla/show_bug.cgi?id=81715 has more
> +
> +
>  choice
>         prompt "Instrumentation type"
>         depends on KASAN
> diff -puN scripts/Makefile.kasan~kasan-rework-kconfig-settings scripts/Makefile.kasan
> --- a/scripts/Makefile.kasan~kasan-rework-kconfig-settings
> +++ a/scripts/Makefile.kasan
> @@ -38,5 +38,8 @@ else
>
>  endif
>
> +ifdef CONFIG_KASAN_EXTRA
>  CFLAGS_KASAN += $(call cc-option, -fsanitize-address-use-after-scope)
>  endif
> +
> +endif
> _
>
> --
> 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@xxxxxxxxxxxxxxxxx
> To post to this group, send email to kasan-dev@xxxxxxxxxxxxxxxxx
> To view this discussion on the web visit https://groups.google.com/d/msgid/kasan-dev/20180122182047.1da66312c5d94cb4632efabd%40linux-foundation.org.
> For more options, visit https://groups.google.com/d/optout.