Re: [PATCH -tip v4 2/3] kasan: Treat meminstrinsic as builtins in uninstrumented files

[Andrey Konovalov]

On Fri, Feb 17, 2023 at 12:45 AM Marco Elver <elver@xxxxxxxxxx> wrote:
>
> Where the compiler instruments meminstrinsics by generating calls to
> __asan/__hwasan_ prefixed functions, let the compiler consider
> memintrinsics as builtin again.
>
> To do so, never override memset/memmove/memcpy if the compiler does the
> correct instrumentation - even on !GENERIC_ENTRY architectures.
>
> Fixes: 69d4c0d32186 ("entry, kasan, x86: Disallow overriding mem*() functions")
> Signed-off-by: Marco Elver <elver@xxxxxxxxxx>
> ---
> v4:
> * New patch.
> ---
>  lib/Kconfig.kasan      | 9 +++++++++
>  mm/kasan/shadow.c      | 5 ++++-
>  scripts/Makefile.kasan | 9 +++++++++
>  3 files changed, 22 insertions(+), 1 deletion(-)
>
> diff --git a/lib/Kconfig.kasan b/lib/Kconfig.kasan
> index be6ee6020290..fdca89c05745 100644
> --- a/lib/Kconfig.kasan
> +++ b/lib/Kconfig.kasan
> @@ -49,6 +49,15 @@ menuconfig KASAN
>
>  if KASAN
>
> +config CC_HAS_KASAN_MEMINTRINSIC_PREFIX
> +       def_bool (CC_IS_CLANG && $(cc-option,-fsanitize=kernel-address -mllvm -asan-kernel-mem-intrinsic-prefix=1)) || \
> +                (CC_IS_GCC && $(cc-option,-fsanitize=kernel-address --param asan-kernel-mem-intrinsic-prefix=1))
> +       # Don't define it if we don't need it: compilation of the test uses
> +       # this variable to decide how the compiler should treat builtins.
> +       depends on !KASAN_HW_TAGS
> +       help
> +         The compiler is able to prefix memintrinsics with __asan or __hwasan.
> +
>  choice
>         prompt "KASAN mode"
>         default KASAN_GENERIC
> diff --git a/mm/kasan/shadow.c b/mm/kasan/shadow.c
> index f8a47cb299cb..43b6a59c8b54 100644
> --- a/mm/kasan/shadow.c
> +++ b/mm/kasan/shadow.c
> @@ -38,11 +38,14 @@ bool __kasan_check_write(const volatile void *p, unsigned int size)
>  }
>  EXPORT_SYMBOL(__kasan_check_write);
>
> -#ifndef CONFIG_GENERIC_ENTRY
> +#if !defined(CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX) && !defined(CONFIG_GENERIC_ENTRY)
>  /*
>   * CONFIG_GENERIC_ENTRY relies on compiler emitted mem*() calls to not be
>   * instrumented. KASAN enabled toolchains should emit __asan_mem*() functions
>   * for the sites they want to instrument.
> + *
> + * If we have a compiler that can instrument meminstrinsics, never override
> + * these, so that non-instrumented files can safely consider them as builtins.
>   */
>  #undef memset
>  void *memset(void *addr, int c, size_t len)
> diff --git a/scripts/Makefile.kasan b/scripts/Makefile.kasan
> index fa9f836f8039..c186110ffa20 100644
> --- a/scripts/Makefile.kasan
> +++ b/scripts/Makefile.kasan
> @@ -1,5 +1,14 @@
>  # SPDX-License-Identifier: GPL-2.0
> +
> +ifdef CONFIG_CC_HAS_KASAN_MEMINTRINSIC_PREFIX
> +# Safe for compiler to generate meminstrinsic calls in uninstrumented files.
> +CFLAGS_KASAN_NOSANITIZE :=
> +else
> +# Don't let compiler generate memintrinsic calls in uninstrumented files
> +# because they are instrumented.
>  CFLAGS_KASAN_NOSANITIZE := -fno-builtin
> +endif
> +
>  KASAN_SHADOW_OFFSET ?= $(CONFIG_KASAN_SHADOW_OFFSET)
>
>  cc-param = $(call cc-option, -mllvm -$(1), $(call cc-option, --param $(1)))
> --
> 2.39.2.637.g21b0678d19-goog
>

Reviewed-by: Andrey Konovalov <andreyknvl@xxxxxxxxx>

Is it also safe to remove custom mem* definitions from
arch/x86/include/asm/string_64.h now?

https://elixir.bootlin.com/linux/v6.2-rc8/source/arch/x86/include/asm/string_64.h#L88